> On Feb. 24, 2015, 8:26 p.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().
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? - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2665/#review5913 ----------------------------------------------------------- On Feb. 24, 2015, 5:22 p.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2665/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2015, 5:22 p.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 gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
