> On Feb. 25, 2015, 4:26 a.m., Steve Reinhardt wrote: > > src/sim/sim_events.hh, line 77 > > <http://reviews.gem5.org/r/2665/diff/2/?file=43797#file43797line77> > > > > seems like it would be safer just to say: > > > > if (scheduled()) > > deschedule(); > > > > then if some derived class no longer meets the condition of always > > being scheduled, we'll still be OK > > Curtis Dunham wrote: > I can do this, however I originally made this trade-off because > BaseGlobalEvent::deschedule() already checks for scheduled(). > > Steve Reinhardt wrote: > I see... I was confused because EventQueue::deschedule() asserts if the > event is not scheduled. It seems inconsistent that deschedule() behaves > differently for global events. I can understand the caution in > BaseGlobalEvent::deschedule(), where it's at least theoretically possible > that some of the local sub-events have been reached so they are no longer > scheduled, but descheduling an event that's already half-executed seems like > a really bad idea anyway. Is there a situation where that could legitimately > happen? I can't think of one. > > Also, note that BaseGlobalEvent::deschedule() never checks whether the > global event is scheduled, though that's partly because BaseGlobalEvent > doesn't even track its own scheduled/descheduled status. Which would make > adding a single check here in the destructor difficult as well... > > So when does the GlobalSimLoopExitEvent get destroyed anyway? Now that > we're allocating it dynamically, the destructor shouldn't get called > implicitly; and if we call it explicitly, we can just explicitly call > deschedule() right before then, right? Can we just drop this?
I understand the confusion - the scheduling interfaces across the family of Event* classes isn't entirely consistent. You bring up a good point about the destruction, as the last tweak made it heap allocated rather than an initialized-on-demand static. I will check on this and update as such if it works! Thanks for the careful feedback. - Curtis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2665/#review5913 ----------------------------------------------------------- On Feb. 25, 2015, 1:22 a.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2665/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2015, 1:22 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10706:e77f25583997 > --------------------------- > sim: Reuse the same limit_event in simulate() > > This patch accomplishes two things: > 1. Makes simulate() reuse the same GlobalSimLoopExitEvent > across calls. This is slightly more efficient than recreating > it every time. > 2. Gives callers to simulate() (especially other simulators) a > foolproof way of knowing that the simulation period ended > successfully by hitting the limit event. They can compare the > global simulate_limit_event to the return value of simulate(). > > This change was motivated by an ongoing effort to integrate gem5 > and SST, with SST as the master sim and gem5 as the slave sim. > > > Diffs > ----- > > src/sim/sim_events.hh c6cb94a14fea4c59780d73d1623d7031bcede6af > src/sim/simulate.hh c6cb94a14fea4c59780d73d1623d7031bcede6af > src/sim/simulate.cc c6cb94a14fea4c59780d73d1623d7031bcede6af > > Diff: http://reviews.gem5.org/r/2665/diff/ > > > Testing > ------- > > > Thanks, > > Curtis Dunham > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
