I'm ok with "ops" as a compromise due to the vagueness (at least in my mind) of what "operations" could mean.
On Wed, Jan 11, 2012 at 6:56 PM, Ali Saidi <[email protected]> wrote: > > > > 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 > -- - Korey _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
