> On 2010-07-10 08:52:15, Steve Reinhardt wrote: > > src/sim/eventq.hh, line 489 > > <http://reviews.m5sim.org/r/51/diff/1/?file=755#file755line489> > > > > I don't like getting rid of this assertion... it's actually pretty > > useful in knowing when something's not right. You should add some code > > upstream somewhere to skip adding the event if we're already past it. > > Timothy Jones wrote: > So the main problem with this assertion is that the event I'm trying to > add is triggered by an instruction count, not a certain Tick. I'm happy to > either add code somewhere else to avoid adding old events or to put this > assertion back in and find a new way of adding instruction-count-based > events. Which would be the preferred option? > > Steve Reinhardt wrote: > I understand that... but even with instructions, I think you'd have the > same problem, which is that you're scheduling an event that's never going to > get triggered. So if you want a callback after 1000 insts are executed, but > 2000 insts have already been executed, it seems like you'd still need to > check somewhere before you call schedule() and realize that there's no point > in calling schedule() because this event is not going to happen. I don't > think instruction-based events should be treated differently. > > Ali Saidi wrote: > I ran into this problem a few weeks ago and have just removed the > assertion in my tree. It's not ideal, but i had no idea how to solve it in > any generic fashion. The best I could come up with was adding a parameter to > the event queue constructor that specified it wasn't tick based, although > getting to current count to compare it to is still problematic. > > > Steve Reinhardt wrote: > I see... the problem is that it's comparing with curTick and not the > actual thing that's being used to drive the queue processing. Duh, I missed > that before. We'll run into a similar problem when we parallelize the > simulation and have per-thread definitions of curTick. Seems like we need to > have the EventQueue class have a pointer to the curTick-equivalent variable > for that queue... is it reasonably straightforward to provide that at the > point where each queue is created? Is there any scenario in which that > wouldn't work, such as a queue would switch variables midstream? I don't > think so.
I actually believe that curTick shouldn't be a global variable. The class itself should have an internal curTick variable that can be accessed. (Probably better if it were a curTick() function). That said, if we leave it as a variable, we could make curTick a Tick & to mainEventQueue.current. As Steve alluded to though, we're going to run into issues will parallelization. > On 2010-07-10 08:52:15, Steve Reinhardt wrote: > > src/sim/sim_object.cc, line 275 > > <http://reviews.m5sim.org/r/51/diff/1/?file=757#file757line275> > > > > I'd prefer a more informative message like "Error: setMaxInsts called > > on non-CPU" (and same with the following function). > > > > There's a broader question of whether this is how we want to expose > > subclass-specific methods to Python... I don't have any great ideas for > > alternatives, but Nate might. > > Timothy Jones wrote: > I can definitely alter the error message here and can make alterations to > the way this is exposed to python, if necessary. > > Steve Reinhardt wrote: > Thanks... changing the error message is all I need to be happy. I'd like > to hear Nate's opinion on whether it's good to end up with a bunch of > class-specific methods on SimObject, and what the alternatives might be, but > until he speaks up you can leave the rest of it like it is. Actually, I don't think these should be done that way. System.py, and src/python/swig/system.i Illustrate another way. It's somewhat messy if we want to do that a lot though. We could try to generalize it. - Nathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/51/#review65 ----------------------------------------------------------- On 2010-07-09 18:14:44, Timothy Jones wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/51/ > ----------------------------------------------------------- > > (Updated 2010-07-09 18:14:44) > > > Review request for Default. > > > Summary > ------- > > CPU: Add functions to get the number of executed instructions and set the > maximum number of instructions to execute to the CPUs and allow them to be > called from python. > > > Diffs > ----- > > src/cpu/BaseCPU.py 249f174e6f37 > src/cpu/base.hh 249f174e6f37 > src/cpu/base.cc 249f174e6f37 > src/cpu/o3/cpu.hh 249f174e6f37 > src/cpu/simple/base.hh 249f174e6f37 > src/python/swig/sim_object.i 249f174e6f37 > src/sim/eventq.hh 249f174e6f37 > src/sim/sim_object.hh 249f174e6f37 > src/sim/sim_object.cc 249f174e6f37 > > Diff: http://reviews.m5sim.org/r/51/diff > > > Testing > ------- > > > Thanks, > > Timothy > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
