----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1031/#review2098 -----------------------------------------------------------
Have you measured how this affects performance? There are a few more touch points than I had imagined, but still not that many. I'd be curious to see if it makes a difference. You could check one ARM and one non-ARM workload (maybe x86, maybe twolf, since it runs relatively quickly) and see how fast they run before and after this change. Be sure to run on a system that isn't doing anything else at the time, and that won't go into weird power management modes half way through. configs/common/Options.py <http://reviews.gem5.org/r/1031/#comment2587> Stray comment? configs/common/Options.py <http://reviews.gem5.org/r/1031/#comment2588> Stray comment? configs/example/fs.py <http://reviews.gem5.org/r/1031/#comment2589> Don't comment it out, delete it. configs/example/se.py <http://reviews.gem5.org/r/1031/#comment2590> Not that there's anything necessarily wrong with doing it, but why are you moving this down past the for loop here? It's not obvious how the two would interact. src/arch/arm/isa.cc <http://reviews.gem5.org/r/1031/#comment2591> Is the dynamic_cast necessary? What type does getCheckerCpuPtr return? If you remove it here, remove it later on too. src/cpu/SConscript <http://reviews.gem5.org/r/1031/#comment2592> Delete it, don't comment it out. src/cpu/SConscript <http://reviews.gem5.org/r/1031/#comment2595> Is the dummy checker needed any more now that you don't have to have a checker installed? src/cpu/SConscript <http://reviews.gem5.org/r/1031/#comment2594> I don't think this check is all that useful any more. I think it would be reasonable to get rid of it entirely. src/cpu/checker/cpu.cc <http://reviews.gem5.org/r/1031/#comment2596> You shouldn't make assumptions like this. FS and SE aren't two separate worlds any more, and you can't assume because you have some of the characteristics of one that you'll have all of them. I even have plans specifically for making workload do something in what would have been called FS mode. src/cpu/checker/cpu.cc <http://reviews.gem5.org/r/1031/#comment2597> Does this actually need to be FullSystem only? src/cpu/checker/cpu_impl.hh <http://reviews.gem5.org/r/1031/#comment2598> Why does this need to be FullSystem only? src/cpu/simple/BaseSimpleCPU.py <http://reviews.gem5.org/r/1031/#comment2599> A checker on the simple CPU doesn't seem to make any sense. This could error out all the time, and it should set the checker to NULL since that's the inert setting. - Gabe Black On Feb. 8, 2012, 7:45 a.m., Geoffrey Blake wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1031/ > ----------------------------------------------------------- > > (Updated Feb. 8, 2012, 7:45 a.m.) > > > Review request for Default. > > > Description > ------- > > CheckerCPU: Make CheckerCPU runtime selectable instead of compile selectable > > Enables the CheckerCPU to be enabled at runtime with the --checker option > from the configs/example/fs.py and configs/example/se.py configuration > files. Also merges with the SE/FS changes. > > > Diffs > ----- > > SConstruct 8f354c5a1634 > configs/common/Options.py 8f354c5a1634 > configs/common/Simulation.py 8f354c5a1634 > configs/example/fs.py 8f354c5a1634 > configs/example/se.py 8f354c5a1634 > src/arch/SConscript 8f354c5a1634 > src/arch/arm/isa.cc 8f354c5a1634 > src/arch/arm/utility.cc 8f354c5a1634 > src/cpu/BaseCPU.py 8f354c5a1634 > src/cpu/SConscript 8f354c5a1634 > src/cpu/base.cc 8f354c5a1634 > src/cpu/base_dyn_inst.hh 8f354c5a1634 > src/cpu/base_dyn_inst_impl.hh 8f354c5a1634 > src/cpu/checker/cpu.cc 8f354c5a1634 > src/cpu/checker/cpu_impl.hh 8f354c5a1634 > src/cpu/checker/thread_context.hh 8f354c5a1634 > src/cpu/o3/O3CPU.py 8f354c5a1634 > src/cpu/o3/SConscript 8f354c5a1634 > src/cpu/o3/commit_impl.hh 8f354c5a1634 > src/cpu/o3/cpu.hh 8f354c5a1634 > src/cpu/o3/cpu.cc 8f354c5a1634 > src/cpu/o3/cpu_builder.cc 8f354c5a1634 > src/cpu/o3/dyn_inst_impl.hh 8f354c5a1634 > src/cpu/o3/fetch_impl.hh 8f354c5a1634 > src/cpu/o3/iew_impl.hh 8f354c5a1634 > src/cpu/o3/lsq_unit_impl.hh 8f354c5a1634 > src/cpu/o3/thread_context.hh 8f354c5a1634 > src/cpu/o3/thread_context_impl.hh 8f354c5a1634 > src/cpu/ozone/OzoneCPU.py 8f354c5a1634 > src/cpu/ozone/SConscript 8f354c5a1634 > src/cpu/ozone/cpu_impl.hh 8f354c5a1634 > src/cpu/ozone/front_end_impl.hh 8f354c5a1634 > src/cpu/ozone/lw_back_end_impl.hh 8f354c5a1634 > src/cpu/ozone/lw_lsq_impl.hh 8f354c5a1634 > src/cpu/simple/BaseSimpleCPU.py 8f354c5a1634 > src/cpu/simple/base.hh 8f354c5a1634 > src/cpu/simple/base.cc 8f354c5a1634 > src/cpu/simple_thread.hh 8f354c5a1634 > src/cpu/thread_context.hh 8f354c5a1634 > > Diff: http://reviews.gem5.org/r/1031/diff/diff > > > Testing > ------- > > Compiles with ARM ISA. > Boots linux with O3 model attached to Checker in FS mode. > Runs simple HelloWorld in SE mode. > > > Thanks, > > Geoffrey Blake > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
