> On Nov. 25, 2015, 6:42 p.m., Andreas Hansson wrote: > > Traditionally, it's not reset, as it accounting for all instructions > > executed. Perhaps hostSeconds shouldn't be reset as well and then the > > calculation would be correct? sim_insts isn't supposed to be used for > > anything other than simulator level measurement. If you want instructions > > executed on a cpu you should look at cpu.numInsts. > > Lena Olson wrote: > Thanks for the response, Andreas -- I had wondered whether or not they > were intentionally not reset, but even when I asked around no one seemed to > know, so I think it's not as obvious as it could be. I would not be super > surprised if someone tried to use it instead of adding up per-core > commitedInsts, when calculating e.g. IPC. So I think in this case, I'll go > through and make hostSeconds and similar stats unresetable as well, and if > there are no objections, I'll add a note in the description for all > non-resetable stats (similar to the final_tick description, except that it's > from beginning of simulation, not restored from checkpoint). > > Lena Olson wrote: > Actually, I have a question about this. For sim_seconds and sim_ticks, > my understanding is that these currently ARE reset. Is that considered > correct behavior? I know these changes are a little nitpicky, but I think > there is benefit to being consistent here. Is there a way we can get all the > sim_* stats to have the same behavior, and if so, is there a traditional > correct behavior?
I *really* like that host_seconds resets. This allows the user to look at a set of stats dumps and decide how long it might take to simulate subsets of work (e.g. for simpoints or modifying a region of interest). Similarly, sim_ticks is nice to have reset, while final_tick (cumulative) is nice for estimating where to switch CPUs or where to start debug traces. Since there are so few stats in question, it wouldn't really hurt to add new stats. I see value in having both cumulative and non-cumulative versions of instructions, ops, and simulation time. Would it be acceptable to just add the ones that don't currently exist? - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3235/#review7647 ----------------------------------------------------------- On Nov. 24, 2015, 12:53 a.m., Lena Olson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3235/ > ----------------------------------------------------------- > > (Updated Nov. 24, 2015, 12:53 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11229:f4b15e5109f9 > --------------------------- > stats: make sim_insts and sim_ops respect stats reset > > Because sim_insts and sim_ops were being calculated using the ThreadState > variable numInst/numOp (type Counter) rather than numInsts/numOps (type > Stats::Scalar), they were not getting reset. This behavior is confusing > because > almost all other entries in the stats file do get reset (with the exception of > final_tick, which notes it is never reset in the stats file). It also leads > to > incorrect behavior with stats like host_inst_rate, which reset the host time > but > not the instructions executed. This patch resets sim_insts and sim_ops. > > > Diffs > ----- > > src/cpu/minor/cpu.cc 021524c21cbc > src/cpu/o3/cpu.cc 021524c21cbc > src/cpu/simple/base.cc 021524c21cbc > > Diff: http://reviews.gem5.org/r/3235/diff/ > > > Testing > ------- > > > Thanks, > > Lena Olson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
