> 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?
>
> Nathan Binkert wrote:
> 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.
Are you concerned how this performs compared to what it used to be (see below)?
If so I can change it back. If the issue is more forward looking I'd prefer to
keep it for a future patch.
Tick
BaseCPU::nextCycle(Tick begin_tick)
{
Tick next_tick = begin_tick;
if (next_tick % clock != 0)
next_tick = next_tick - (next_tick % clock) + clock;
next_tick += phase;
assert(next_tick >= curTick());
return next_tick;
}
- Andreas
-----------------------------------------------------------
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
