> On Jan. 11, 2016, 11:52 p.m., Joel Hestness wrote: > > src/cpu/minor/cpu.cc, line 335 > > <http://reviews.gem5.org/r/3235/diff/2/?file=53010#file53010line335> > > > > Just a note that this is interesting... looks like this fixes an > > inconsistency: MinorCPU appears to collect inst stats cummulatively, while > > the other CPUs use resettable stats (another indicator that this patch will > > help with clarity!)
I think all of them used numInst (which is a counter) originally, and I changed them all to numInsts (which is a stat)? It's easy to get numInst and numInsts confused -- there being a counter called numInst and a stat called numInsts as members of the same struct is just asking for trouble IMO, but I didn't have a better idea so I left them as is. > On Jan. 11, 2016, 11:52 p.m., Joel Hestness wrote: > > src/sim/stat_control.cc, line 161 > > <http://reviews.gem5.org/r/3235/diff/2/?file=53016#file53016line161> > > > > I'm hoping that we could use stat names that are a little more > > self-evident for these. Maybe instead of 'simulatorXxxx'/'simulator_xxxx' > > we could use something like 'cummulativeSimXxxx'/'cumm_sim_xxxx'? That way > > the names more clearly indicate that the stats are accumulated throughout > > the whole simulation time rather than just the current stats period. > > > > I do like that the descriptions say "not reset" also. I like cumulativeSim (and cumulative_sim, though it's long), as long as it's clear that it's different from final_tick -- it's cumulative over this invocation of the simulator, but not restored from checkpoints. Hopefully the note in the descriptions helps clarify that. Anyone else have opinions? > On Jan. 11, 2016, 11:52 p.m., Joel Hestness wrote: > > src/cpu/kvm/base.cc, line 495 > > <http://reviews.gem5.org/r/3235/diff/2/?file=53009#file53009line495> > > > > We should probably check with Andreas Sandberg on this one. It looks > > like ctrInsts is already a cummulative stat for the KVM CPU. This might be > > required by the KVM CPU for fast-forwarding correctness? This could cause > > changes in KVM correctness and/or regressions (if any are maintained). > > Hopefully, it is harmless to allow the stat to reset, though? I think it should be okay -- ctrInsts and numInsts both already existed and I don't modify them, I just changed which is used by totalInsts(), which looks like it never gets called by anything besides the stats stuff. Another pair of eyes wouldn't hurt though. - Lena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3235/#review7866 ----------------------------------------------------------- On Jan. 6, 2016, 10:07 p.m., Lena Olson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3235/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2016, 10:07 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11230:374d79447577 > --------------------------- > stats: organize and clarify resettable stats > > Some stats (e.g. sim_insts) were traditionally not reset, because they were > intended for simulator-level measurement. However, some other stats (e.g. > sim_ticks) WERE reset. In addition, some derived stats used both reset and > unreset stats in the calculation, resulting in nonsense values. This patch > creates two sets of overall stats: one set that respects reset, and is useful > for measuring the simulated program, and one set that does not reset, useful > for > studying the simulator itself. The non-resettable stats are prefixed with > "simulator_", as they behave different from other stats. > > > Diffs > ----- > > src/arch/null/cpu_dummy.hh 021524c21cbc > src/cpu/BaseCPU.py 021524c21cbc > src/cpu/base.hh 021524c21cbc > src/cpu/checker/cpu.hh 021524c21cbc > src/cpu/kvm/base.hh 021524c21cbc > src/cpu/kvm/base.cc 021524c21cbc > src/cpu/minor/cpu.cc 021524c21cbc > src/cpu/o3/cpu.hh 021524c21cbc > src/cpu/o3/cpu.cc 021524c21cbc > src/cpu/simple/base.hh 021524c21cbc > src/cpu/simple/base.cc 021524c21cbc > src/sim/stat_control.hh 021524c21cbc > src/sim/stat_control.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
