----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/982/#review1909 -----------------------------------------------------------
Thanks, Tony, this is long overdue. A few thoughts: - I think "insts" and "ops" are pretty good names, with the definitions that Ali used. However, somehow even though expanding "insts" to "instructions" seems OK, expanding "ops" to "operations" paradoxically makes things more confusing in my mind. Since everyone is used to saying "micro-op" or "macro-op" (or "uop"), "op" has a clear lineage, but no one says "micro-operation" in practice, so "operation" sounds like a more generic term that could mean almost anything. I would be happy to see names like totalInstructions() and numSimulatedInstructions() replaced with totalInsts() and numSimulatedInsts() so that their new counterparts can be named totalOps() and numSimulatedOps(). I don't think replacing "insts" with "instructions" actually makes the code any more readable anyway. - I don't like losing the "committed" part of some of those stat names; on anything with speculative execution (InOrder or O3) there will be a distinction between the number of insts/ops executed and the number committed. I assume these renames were done for consistency with other CPU models (which is good!), but it's hard to know for sure since the diff doesn't show the unchanged names and I'm too lazy to look right now... - Do make sure that the commit message is correct when this finally gets committed! - Steve On 2012-01-10 08:47:41, Anthony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/982/ > ----------------------------------------------------------- > > (Updated 2012-01-10 08:47:41) > > > Review request for Default. > > > Summary > ------- > > Changeset 8650:2b39160c9a96 > --------------------------- > [mq]: bug.patch > > > Diffs > ----- > > src/arch/noisa/cpu_dummy.hh UNKNOWN > src/cpu/base.hh UNKNOWN > src/cpu/base.cc UNKNOWN > src/cpu/inorder/cpu.hh UNKNOWN > src/cpu/inorder/cpu.cc UNKNOWN > src/cpu/inorder/inorder_dyn_inst.hh UNKNOWN > src/cpu/o3/commit.hh UNKNOWN > src/cpu/o3/commit_impl.hh UNKNOWN > src/cpu/o3/cpu.hh UNKNOWN > src/cpu/o3/cpu.cc UNKNOWN > src/cpu/simple/base.hh UNKNOWN > src/cpu/simple/base.cc UNKNOWN > src/cpu/thread_state.hh UNKNOWN > src/cpu/thread_state.cc UNKNOWN > src/sim/stat_control.cc UNKNOWN > > Diff: http://reviews.m5sim.org/r/982/diff > > > Testing > ------- > > > Thanks, > > Anthony > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
