----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2120/ -----------------------------------------------------------
Review request for Default. Repository: gem5 Description ------- sim: Fix a bug when scaling a clock domain's frequency A SrcClockDomain (src/sim/clock_domain.hh) can change its clock period with the clockPeriod(Tick) function. However, when a SrcClockDomain changes the clock period, the ClockedObjects in the clock domain may be inactive. When these ClockedObjects wake up, they update their internal tick incorrectly, leading to incorrect latency calculations throughout the system. For example, a 2 cycle L1 cache hit latency can turn into 3 cycles, causing significant slowdowns in simulated runtime after the frequency change (4% to 20% slowdowns in my experiments). This patch fixes the bug by adding a member list to the ClockDomain -- ClockedObjects register themselves with their clock domain at construction time and are added to the member list. Using this list, before a clock domain modifies its clock period, it can update each member's tick to the curTick(). Diffed from Changeset 9993 Diffs ----- src/sim/clocked_object.hh bdd606534bdc src/sim/clock_domain.cc bdd606534bdc src/sim/clock_domain.hh bdd606534bdc Diff: http://reviews.gem5.org/r/2120/diff/ Testing ------- Purely to demonstrate this bug, I've added the following code, using the inorder cpu. In src/sim/stat_control.cc (changed precision from 6 to 12 for more detail):: simSeconds .name("sim_seconds") .desc("Number of seconds simulated") .precision(12) ; In src/sim/clocked_object.hh (added function for a clocked object to get its clock domain's pointer):: inline ClockDomain* getClockDomain() const { ClockDomain *c = &clockDomain; return c; } In src/cpu/inorder/cpu.hh (create a pointer to a SrcClockDomain to control the CPU's clock period from within the CPU):: SrcClockDomain *clockDomain; In src/cpu/inorder/cpu.cc (let CPU constructor set the clock domain pointer):: InOrderCPU::InOrderCPU(Params *params) : (...) { (...) clockDomain = (SrcClockDomain*) getClockDomain(); } In src/cpu/inorder/cpu.cc (after 5000 cycles, change the frequency to 1999ps):: void InOrderCPU::tick() { (...) if (numCycles.value() > 5000) { clockDomain->clockPeriod(1999); } } After compiling these testing changes, I run MIPS hello world at 500 MHz (2000 ps):: $ ./build/MIPS/gem5.opt configs/example/se.py -c tests/test-progs/hello/bin/mips/linux/hello --cpu-type=inorder --caches --cpu-clock=500MHz --l1d_size=16kB --l1i_size=16kB --l1d_assoc=1 --l1i_assoc=1 --cacheline_size=32 --num-cpus=1 Results: - Without patch, simSeconds is 0.000027967012. - With patch, simSeconds is 0.000026771610. The patch speeds up simulated runtime by 4% here. You can check the Exec debug-flag dump to see that memory instructions complete with different latencies with and without the patch. Instructions in middle of simulation, without patch, the time difference is 7996 ps (4 cycles):: 11995002: system.cpu T0 : @__libc_malloc+244 : addu r17, r2, r0 : IntAlu : D=0x000000001000c060 12002998: system.cpu T0 : @__libc_malloc+248 : lw r25, -31084(r28) : MemRead : D=0x0000000000000000 A=0x10005624 Same instructions with patch, the time difference is 5997 ps (3 cycles):: 11787106: system.cpu T0 : @__libc_malloc+244 : addu r17, r2, r0 : IntAlu : D=0x000000001000c060 11793103: system.cpu T0 : @__libc_malloc+248 : lw r25, -31084(r28) : MemRead : D=0x0000000000000000 A=0x10005624 For more information, you can look at src/sim/clocked_object.hh update() and insert debug messages to see that the tick is incorrectly updated after a clocked object wakes up. Here's the relevant section of code in src/sim/clocked_object.hh update():: void update() const { (...) // if not, we have to recalculate the cycle and tick, we // perform the calculations in terms of relative cycles to // allow changes to the clock period in the future Cycles elapsedCycles(divCeil(curTick() - tick, clockPeriod())); cycle += elapsedCycles; tick += elapsedCycles * clockPeriod(); } Thanks, Christopher Torng _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
