> 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? > > Joel Hestness wrote: > 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?
Yea mean having both non-resetting and resetting versions of all the sim/host/... stats? Seems fine to me. - Ali ----------------------------------------------------------- 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
