Tony, I think the inorder's commit stage is called "graduation". Keeping the names consistent across models is most important so calling it "committedInsts" over "graduatedInsts" wouldn't be a big deal.
I dont think "executedInsts" is the right stat to change in Inorder. Executed insts is literally instructions executed by the ALU (useful for power modeling). Let me double check that on review board. On Thu, Jan 12, 2012 at 2:02 PM, Anthony Gutierrez <[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! > > > > > > > Ali Saidi wrote: > > 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; > > > > > > > > Steve Reinhardt wrote: > > OK, I missed that... for O3 that seems fine. I don't think that's > true for InOrder though. > > > > Also, don't get me wrong, I'm not defending the current names in > their entirety... I'm surprised there isn't still a "COM:" somewhere in > there too. > > > > So should the solution be use insts for instructions and ops for > operations? > > Also, the reason I don't call them "committed" is because, while they are > committed, they're not accounted for in commit. I.e., I noted that inorder > doesn't have a commit stage and chose to use executed (although I can see > why with that terminology it could be ambiguous as to whether the executed > instruction was actually committed). So, there is a stat in the commit > stage which is called committedOperations/Instructions. > > > - Anthony > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/982/#review1909 > ----------------------------------------------------------- > > > On 2012-01-12 11:01:51, Anthony Gutierrez wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://reviews.m5sim.org/r/982/ > > ----------------------------------------------------------- > > > > (Updated 2012-01-12 11:01:51) > > > > > > Review request for Default. > > > > > > Summary > > ------- > > > > Stats: Add stats for instructions and micro ops, both globally and per > CPU. > > > > Adds an instructions_executed and operations_executed stat for each CPU > model. Also creates a new global stat sim_uOps which counts all operations. > Changes sim_insts to count instructions. > > > > > > 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
