> On Feb. 24, 2015, 9:17 a.m., Steve Reinhardt wrote: > > I like the idea of having a single global limit event, with a function to > > access it so clients can check whether that's what was hit. > > > > However I don't think the GlobalSimLoopExitEvent should be a singleton, as > > there are a bunch of other instances that get created (see exitSimLoop() in > > sim_events.cc). I think it's clearer just to have a global variable for > > limit_event. (One of the extremely rare cases where a global variable is > > OK.) > > > > There's already support in simulate() for first-time initialization with > > the threads_initialized flag. I think we should basically leverage this > > flag to create the global limit_event (rename the flag if you want to make > > it more general). We may want to take the static vars inside simulate() > > and make them all global for consistency; I don't have a strong opinion > > there. > > Curtis Dunham wrote: > Maybe I should just change the comment. I'm not actually making > GlobalSimLoopExitEvent a (simulator-wide) singleton, but rather just a static > with respect to simulate(). Because of how it schedules the event, the > destructor needs to deschedule it. I wrapped simulate()'s instance in a > function to deal with static initialization ordering issues. > > The other uses of GlobalSimLoopExitEvent should still work.
Yea, I read the description first, and my initial reaction on looking at the code was "if you're going to make it a singleton you should really put it inside the class", and later I realized "you don't really want a singleton here", and I didn't really go back to square one to see that it was not-really-a-singleton *because* you don't really want a singleton. However, overriding the destructor is a little weird if it only applies to that instance... can you elaborate a little more on why that's necessary? Nevertheless, I still think that just making a plain ol' global variable is clearer and cleaner. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2665/#review5899 ----------------------------------------------------------- On Feb. 20, 2015, 10:46 a.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2665/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2015, 10:46 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()'s GlobalSimLoopExitEvent a singleton reused > 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 call > getLimitEvent() and compare it 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
