----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/47/#review67 -----------------------------------------------------------
Overall I'm in favor of what you're doing here... I think most of my comments are more about general cleanup of the bpred object unrelated to your relocation of it. But I've never really looked closely at the code before and it seems like this is a good opportunity to clean it up. As far as I can tell, the templating is only used to provide a MaxThreads value. I'd like to get rid of the template entirely (and the _impl.hh function and the empty base class), just pass max threads in as a parameter, and dynamically allocate the couple of arrays that depend on it. src/cpu/o3/fetch_impl.hh <http://reviews.m5sim.org/r/47/#comment206> Why do we need to clear the state here? Do we need this function at all? src/cpu/o3/fetch_impl.hh <http://reviews.m5sim.org/r/47/#comment207> I don't see the point in this function either... I'd just get rid of the empty function and this call. src/cpu/pred/bpred_unit.hh <http://reviews.m5sim.org/r/47/#comment208> It doesn't look like this function or BTBLookup or BTBUpdate get called anywhere... let's get rid of them. Even if they do get called, I don't see the point of wrapping the indirection. src/cpu/pred/builder.cc <http://reviews.m5sim.org/r/47/#comment209> I think this function should go in bpred_unit.cc and this file should go away. - Steve On 2010-07-09 18:08:18, Timothy Jones wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/47/ > ----------------------------------------------------------- > > (Updated 2010-07-09 18:08:18) > > > Review request for Default. > > > Summary > ------- > > BranchPred: Take the branch predictor out of O3CPU and make it a stand-alone > SimObject. This then allows the same branch predictor to be shared amongst > several CPUs. > > This patch is unfinished. I would like to take the branch predictor out of > the inorder CPU as well, but want comments on whether this is the best > approach to take first. > > > Diffs > ----- > > src/cpu/o3/O3CPU.py 249f174e6f37 > src/cpu/o3/SConscript 249f174e6f37 > src/cpu/o3/bpred_unit.hh 249f174e6f37 > src/cpu/o3/bpred_unit.cc 249f174e6f37 > src/cpu/o3/bpred_unit_impl.hh 249f174e6f37 > src/cpu/o3/cpu_builder.cc 249f174e6f37 > src/cpu/o3/cpu_policy.hh 249f174e6f37 > src/cpu/o3/fetch.hh 249f174e6f37 > src/cpu/o3/fetch_impl.hh 249f174e6f37 > src/cpu/pred/BaseBPredUnit.py PRE-CREATION > src/cpu/pred/SConscript 249f174e6f37 > src/cpu/pred/base.hh PRE-CREATION > src/cpu/pred/base.cc PRE-CREATION > src/cpu/pred/bpred_unit.hh PRE-CREATION > src/cpu/pred/bpred_unit.cc PRE-CREATION > src/cpu/pred/bpred_unit_impl.hh PRE-CREATION > src/cpu/pred/builder.cc PRE-CREATION > > Diff: http://reviews.m5sim.org/r/47/diff > > > Testing > ------- > > > Thanks, > > Timothy > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
