> On April 9, 2013, 8:55 a.m., Joel Hestness wrote: > > From the other patch review: > > > > Jason Power: > > I just now was looking at your other patch and I see that for the option > > maxtick the help says help="Stop after T ticks". This is ambiguous as to > > what the expected behavior is. Can you update the help message in this > > patch to clarify exactly what the behavior should be. > > > > Andreas Hansson: > > Agreed, the "Run to" and "Stop after" are both rather ambiguous. It would > > be good if it was made more clear what is absolute (if anything) and what > > is relative. > > Joel Hestness wrote: > Ok. This raises a couple questions: > 1) Do we want the maxtick and maxtime options to be relative to a > restored checkpoint, or include the time from the restore? > > I can never remember which I need to use when I use maxticks when > restoring from a checkpoint. That said, I usually guess (incorrectly) that > the maxtick is relative to the restore time, which would be consistent with > the way that one thinks about maxtick when not restoring from checkpoint. If > no one minds, I'd prefer to change the handling to be relative to the restore > time for both of these options. > > 2) Do we care if the help message grows to a couple sentences? > > If we decide to leave these variables as absolute time, there isn't > really a good way to explain that in a single sentence, because it requires > explaining that it includes time from the checkpoint that will be restored. > > However we decide, I'll update the patch and resubmit. > > Ali Saidi wrote: > I can see use cases for both absolute time and relative time, I almost > wonder if we should have a --abs-max-tick and --rel-max-tick to solve the > problem. > > I'm all for the help message growing. > > Andreas Hansson wrote: > What happened with this one?
I haven't had a chance to merge this up to the latest repo and add relative vs. absolute max ticks. I'll try to tackle this today. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1816/#review4221 ----------------------------------------------------------- On April 7, 2013, 10:12 p.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1816/ > ----------------------------------------------------------- > > (Updated April 7, 2013, 10:12 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 9633:dfd9f65d2011 > --------------------------- > Simulate.py: Fix up maxtick and maxtime > > This patch contains a couple fixes: > > 1) Since the global simulator frequency isn't bound until m5.instantiate() > is called, the maxtick resolution needs to happen after this call, since > changes to the global frequency will cause m5.simulate() to misinterpret the > maxtick value. Shuffling this also requires tweaking the checkpoint directory > handling to signal the checkpoint restore tick back to run(). Fixing this > completely and correctly will require storing the simulation frequency into > checkpoints, which is beyond the scope of this patch. > > 2) The maxtick option in Options.py was defaulted to MaxTicks, so the old > code > would always skip over the maxtime part of the conditionals at the beginning > of run(). Change the maxtick default to None, and set the maxtick local > variable in run() appropriately. > > > Diffs > ----- > > configs/common/Simulation.py fa31189e1fb5 > configs/common/Options.py fa31189e1fb5 > > Diff: http://reviews.gem5.org/r/1816/diff/ > > > Testing > ------- > > > Thanks, > > Joel Hestness > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
