> 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.
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. - 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. > > > 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
