> On July 19, 2012, 12:52 p.m., Nathan Binkert wrote: > > src/sim/ClockedObject.py, line 45 > > <http://reviews.gem5.org/r/1303/diff/3/?file=28040#file28040line45> > > > > Do we want a clock phase? I certainly wanted that when I did a similar > > thing. I'll send you some code that manages that. It could be pushed off > > to a future changeset. > > Steve Reinhardt wrote: > It looked like the phase code was there in the CPU (but not in other > objects) and was unused. So it seemed reasonable to me to leave it out until > someone really needed it again. Can you elaborate on why you found it > necessary? > > Andreas Hansson wrote: > My observation was also: not used anywhere, easy to add back if we want, > so prune
Hmmm. It was pretty critical for me when doing network simulations, but perhaps that is just and oddball use case. > On July 19, 2012, 12:52 p.m., Nathan Binkert wrote: > > src/sim/clocked_object.hh, line 95 > > <http://reviews.gem5.org/r/1303/diff/3/?file=28042#file28042line95> > > > > This is likely premature optimization, but sticking a div in every > > clock schedule seems scary. The trick that I've used in the past was to > > save the clock tick and check to see if curtick is == to the saved tick. > > If so, you can just do an add and almost all the time, the condition will > > pass. Again, perhaps over optimized. I'll send you the code. > > Andreas Hansson wrote: > I guess it all boils down to how it is used. Ultimately gem5 should be > event based, not a collection of cycle callable models. As such, I would hope > it doesn't get called every cycle. I have not stats to back that up though, > so it could be that you're absolutely right. > > At this point, I merely wanted to tidy up what was there...that did a > modulo operation for every call. > > Nathan Binkert wrote: > Sure it's event based, but most things get clocked every cycle for many > cycles and then go idle for a period of time and then do it again. (Think > CPUs, caches). In this case, the addition works except from waking after > idle. When that happens, the condition fails, and then you do the divide. > > Andreas Hansson wrote: > I can give it a bash and compare the performance. Perhaps as a follow on > patch or you want to see it as part of this one? As a follow-on is fine with me. I just get concerned with event queue performance because in certain situations the performance can be particularly bad. As I recall, AMD had some use of a really old version of m5 (that ali did for them) that really stressed the event queue and ran into similar problems. If we ever get the network model in ruby moved over to our event queue, and people run network only experiments, you'll certainly see performance issues. - Nathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1303/#review3101 ----------------------------------------------------------- On July 19, 2012, 5:46 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1303/ > ----------------------------------------------------------- > > (Updated July 19, 2012, 5:46 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9127:f00508d2c790 > --------------------------- > Clock: Move the clock and related functions to ClockedObject > > This patch moves the clock of the CPU, bus, and numerous devices to > the new class ClockedObject, that sits in between the SimObject and > MemObject in the class hierarchy. Although there are currently a fair > amount of MemObjects that do not make use of the clock, they > potentially should do so, e.g. the caches should at some point have > the same clock as the CPU, potentially with a 1:n ratio. This patch > does not introduce any new clock objects or object hierarchies > (clusters, clock domains etc), but is still a step in the direction of > having a more structured approach clock domains. > > The most contentious part of this patch is the serialisation of clocks > that some of the modules (but not all) did previously. This > serialisation should not be needed as the clock is set through the > parameters even when restoring from the checkpoint. In other words, > the state is "stored" in the Python code that creates the modules. > > The nextCycle methods are also simplified and the clock phase > parameter of the CPU is removed (this could be part of a clock object > once they are introduced). > > > Diffs > ----- > > src/arch/x86/interrupts.hh 48eeef8a0997 > src/arch/x86/interrupts.cc 48eeef8a0997 > src/arch/x86/utility.cc 48eeef8a0997 > src/cpu/BaseCPU.py 48eeef8a0997 > src/cpu/base.hh 48eeef8a0997 > src/cpu/base.cc 48eeef8a0997 > src/cpu/testers/memtest/memtest.hh 48eeef8a0997 > src/cpu/testers/networktest/networktest.hh 48eeef8a0997 > src/dev/CopyEngine.py 48eeef8a0997 > src/dev/Ethernet.py 48eeef8a0997 > src/dev/arm/RealView.py 48eeef8a0997 > src/dev/arm/pl111.hh 48eeef8a0997 > src/dev/arm/pl111.cc 48eeef8a0997 > src/dev/arm/timer_cpulocal.cc 48eeef8a0997 > src/dev/i8254xGBe.hh 48eeef8a0997 > src/dev/i8254xGBe.cc 48eeef8a0997 > src/dev/ns_gige.hh 48eeef8a0997 > src/dev/ns_gige.cc 48eeef8a0997 > src/dev/sinic.hh 48eeef8a0997 > src/dev/sinic.cc 48eeef8a0997 > src/mem/Bus.py 48eeef8a0997 > src/mem/MemObject.py 48eeef8a0997 > src/mem/bus.hh 48eeef8a0997 > src/mem/bus.cc 48eeef8a0997 > src/mem/mem_object.hh 48eeef8a0997 > src/mem/mem_object.cc 48eeef8a0997 > src/sim/ClockedObject.py PRE-CREATION > src/sim/SConscript 48eeef8a0997 > src/sim/clocked_object.hh PRE-CREATION > > Diff: http://reviews.gem5.org/r/1303/diff/ > > > Testing > ------- > > util/regress all passing (disregarding t1000 and eio) > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
