-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1865/#review4342
-----------------------------------------------------------


Looks like a very useful patch. I think it just needs the predictor size 
redundancy removed.


src/cpu/pred/global.cc
<http://reviews.gem5.org/r/1865/#comment4091>

    It looks like 2^globalHistoryBits = globalPredictorSize. Since 
globalHistoryBits was recently removed from the tournament predictor, it might 
be better to remove it as a parameter here as well and determine it from 
globalPredictorSize. Alternatively, the relationship between the two parameters 
must be enforced to avoid seg faults.



src/cpu/pred/global.cc
<http://reviews.gem5.org/r/1865/#comment4092>

    This could walk off the array (see above comment).



src/cpu/pred/global.cc
<http://reviews.gem5.org/r/1865/#comment4093>

    These updates can also walk off the array (see above).


- Erik Tomusk


On May 10, 2013, 10:29 a.m., Taylor Lloyd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1865/
> -----------------------------------------------------------
> 
> (Updated May 10, 2013, 10:29 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Description
> -------
> 
> This patch adds a global-global (gAG) branch predictor, used by specifying 
> the "global" predType.
> From BranchPredictor, it uses the following variables for configuration: 
>  - globalPredictorSize
>  - globalCtrBits
>  - globalHistoryBits
> 
> As such, it may conflict with the ongoing effort to factor out the redundant 
> variable in the predictors.
> 
> 
> Diffs
> -----
> 
>   src/cpu/pred/SConscript 00dca8a9b560 
>   src/cpu/pred/bpred_unit.cc 00dca8a9b560 
>   src/cpu/pred/global.hh PRE-CREATION 
>   src/cpu/pred/global.cc PRE-CREATION 
> 
> Diff: http://reviews.gem5.org/r/1865/diff/
> 
> 
> Testing
> -------
> 
> Prior to submission, this was tested by building build/ARM/gem5.debug, and 
> run against a subset of the SPEC CPU2006 benchmarks. Statistics generated 
> seem valid, and it caused no execution errors.
> 
> 
> Thanks,
> 
> Taylor Lloyd
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to