----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1474/#review3584 -----------------------------------------------------------
Overall, seems like a useful addition. The keyword should be one of the following: base, stats, sim, config, ruby, mem, arm, x86, arch, cpu, dev, scons, alpha, mips, power, sparc configs/common/Options.py <http://reviews.gem5.org/r/1474/#comment3585> Should the type not be long long as in the Ticks? configs/example/se.py <http://reviews.gem5.org/r/1474/#comment3586> 80 char? I would suggest to make the fastmem option responsible for checking that we run in atomic and skip that check here. src/cpu/simple/AtomicSimpleCPU.py <http://reviews.gem5.org/r/1474/#comment3587> Param.Tick? Alternatively even a more abstract frequency or latency for absolute time? src/cpu/simple/atomic.hh <http://reviews.gem5.org/r/1474/#comment3588> use the m5 hash_map and hide the implementation. Otherwise we have issues with the namespace and header for the map depending on gcc versions src/cpu/simple/atomic.hh <http://reviews.gem5.org/r/1474/#comment3589> see the base/hashmap for how to open and close this namespace using the defines provided src/cpu/simple/atomic.hh <http://reviews.gem5.org/r/1474/#comment3590> Doxygen please, for all the new bits :) src/cpu/simple/atomic.hh <http://reviews.gem5.org/r/1474/#comment3591> const? src/cpu/simple/atomic.hh <http://reviews.gem5.org/r/1474/#comment3592> const? src/cpu/simple/atomic.cc <http://reviews.gem5.org/r/1474/#comment3593> Please initialize all the variables that are introduced. src/cpu/simple/atomic.cc <http://reviews.gem5.org/r/1474/#comment3594> Do we really want to hardcode the file? what happens when you have many CPUs? src/cpu/simple/atomic.cc <http://reviews.gem5.org/r/1474/#comment3595> One if statement? src/cpu/simple/atomic.cc <http://reviews.gem5.org/r/1474/#comment3597> Comments please. Throughout. src/cpu/simple/atomic.cc <http://reviews.gem5.org/r/1474/#comment3596> Before we use auto anywhere everyone has to agree to drop gcc 4.3 support and _always_ use the c++0x flag. I am happy to do this, but this is the topic for an e-mail. - Andreas Hansson On Oct. 19, 2012, 3:06 p.m., Mitch Hayenga wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1474/ > ----------------------------------------------------------- > > (Updated Oct. 19, 2012, 3:06 p.m.) > > > Review request for Default. > > > Description > ------- > > Add the ability to create SimPoint basic block vector (BBV) files from gem5. > > This patch adds SimPoint profiling functionality to the atomic CPU model. > Normally one has to use another tool like valgrind or pinpoints to perform > this analysis. Using the --simpoint_profile and --simpoint_interval options, > after running simulation a simpoint.bb.gz file will be created in the gem5 > output directory. This file can then be fed to the simpoint tool to identify > program sections representative of overall program behavior. > > > Diffs > ----- > > configs/common/Options.py b4bd51f8b7a0 > configs/example/se.py b4bd51f8b7a0 > src/cpu/simple/AtomicSimpleCPU.py b4bd51f8b7a0 > src/cpu/simple/atomic.hh b4bd51f8b7a0 > src/cpu/simple/atomic.cc b4bd51f8b7a0 > > Diff: http://reviews.gem5.org/r/1474/diff/ > > > Testing > ------- > > Ran a few benchmarks, output simpoint file and selected simpoints seem ok. > > > Thanks, > > Mitch Hayenga > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
