> On 2012-01-11 15:25:25, Steve Reinhardt wrote: > > 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! > >
so, we'll still get the commited part of the name. Right now it's named: testsys.switch_cpus.commit.commitCommittedInsts which is pretty ugly. Tony's change would make it testsys.switch_cpus.commit.inst_count. I think that is much better. I'd be fine with s/operations/ops/g; s/instructions/insts/g; - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/982/#review1909 ----------------------------------------------------------- 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
