I think it's fine like it is. Gabe is rightly concerned that some parts of SimpleCPU are not so simple anymore, particularly timing.cc, but your change doesn't impact those parts so I don't see a problem with it.
Steve On Sun, Jul 11, 2010 at 11:01 AM, Timothy M Jones <tjon...@inf.ed.ac.uk> wrote: > Yes, it's a very minor addition to the code. However, if you'd prefer, I > could create a subclass of AtomicSimpleCPU and add it in there. Maybe > that's an overkill for this particular addition, but that could be a way to > simplify things in the future - have Atomic/TimingSimpleCPUs as simple as > possible and add anything extra into some subclass of them? > > Tim > > On 10/07/2010 13:03, Steve Reinhardt wrote: >> >> If you do look at the patch, you'll see that it's a pretty minor hook >> in the BaseSimpleCPU code, and doesn't touch atomic.* or timing.* at >> all. Plus warming up state like caches, bpred, etc. in a fast >> functional mode is a key aspect of SMARTS; you spend more time in >> warmup than in detailed simulation, so you'd lose a huge chunk of the >> perf advantage if you did warmup in O3. >> >> Steve >> >> On Sat, Jul 10, 2010 at 12:53 AM, Gabe Black<gbl...@eecs.umich.edu> >> wrote: >>> >>> I haven't looked at the code yet, but I'm wary of unsimplifying the >>> simple CPU. It's been headed that way anyway, but just as much as >>> functionally correct execution required. I'd like to see us resimplify >>> it somehow instead. I'm not familiar with SMARTS, but would you be able >>> to have some short lead in/warm up period with O3 before you started >>> taking stats? That sounds very familiar so maybe you already do? >>> >>> Gabe >>> >>> Timothy Jones wrote: >>>> >>>> ----------------------------------------------------------- >>>> This is an automatically generated e-mail. To reply, visit: >>>> http://reviews.m5sim.org/r/48/ >>>> ----------------------------------------------------------- >>>> >>>> Review request for Default. >>>> >>>> >>>> Summary >>>> ------- >>>> >>>> SimpleCPU: Allow Simple CPUs to warm a branch predictor by creating a >>>> pointer >>>> to a branch predictor in their class and a warm function within the >>>> branch >>>> predictor itself. The Simple CPU branch predictor is also provided in >>>> the >>>> parameters so that it can be exposed to python. >>>> >>>> >>>> Diffs >>>> ----- >>>> >>>> src/cpu/pred/base.hh PRE-CREATION >>>> src/cpu/pred/bpred_unit.hh PRE-CREATION >>>> src/cpu/pred/bpred_unit_impl.hh PRE-CREATION >>>> src/cpu/simple/BaseSimpleCPU.py 249f174e6f37 >>>> src/cpu/simple/base.hh 249f174e6f37 >>>> src/cpu/simple/base.cc 249f174e6f37 >>>> >>>> Diff: http://reviews.m5sim.org/r/48/diff >>>> >>>> >>>> Testing >>>> ------- >>>> >>>> >>>> Thanks, >>>> >>>> Timothy >>>> >>>> _______________________________________________ >>>> m5-dev mailing list >>>> m5-dev@m5sim.org >>>> http://m5sim.org/mailman/listinfo/m5-dev >>>> >>> >>> _______________________________________________ >>> m5-dev mailing list >>> m5-dev@m5sim.org >>> http://m5sim.org/mailman/listinfo/m5-dev >>> >> _______________________________________________ >> m5-dev mailing list >> m5-dev@m5sim.org >> http://m5sim.org/mailman/listinfo/m5-dev >> > > -- > Timothy M. Jones > http://homepages.inf.ed.ac.uk/tjones1 > > The University of Edinburgh is a charitable body, registered in > Scotland, with registration number SC005336. > > _______________________________________________ > m5-dev mailing list > m5-dev@m5sim.org > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
