> On Aug. 3, 2012, 10:53 a.m., Steve Reinhardt wrote: > > src/sim/clocked_object.hh, line 91 > > <http://reviews.gem5.org/r/1321/diff/2/?file=28275#file28275line91> > > > > Sorry not to be more clear. I meant that both lines 92 and 93 (the > > cycle and tick assignments) should be replaced. The line I provided was > > meant to come first and show how tick would need to be updated based on its > > current value. I was leaving the corresponding update of cycle as an > > exercise for the reader ;-). Actually at that point I was probably in my > > phase where I was hoping that we could eliminate cycle entirely. > > > > If we do keep cycle, it would be more like: > > > > CycleCount elapsedCycles = divCeil(curTick() - tick, clock); > > cycle += elapsedCycles; > > tick += elapsedCycles * clock; > > > > > > Andreas Hansson wrote: > I agree conceptually. However, it actually changes the regression as the > rounding is slightly different. This is not a problem as such, but I would > still like to keep it out of this patch. Agreed?
Weird... how does the rounding end up different? They're all integers, so I'm confused about how that could happen. It's early in the morning though so maybe I'm missing something obvious. If there is a good reason for the rounding to be different, then I am perfectly fine with leaving this change to another patch. Actually I'm OK with leaving this change for later regardless, but mostly I'm stumped about why there's a difference. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1321/#review3200 ----------------------------------------------------------- On Aug. 3, 2012, 4:54 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1321/ > ----------------------------------------------------------- > > (Updated Aug. 3, 2012, 4:54 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9143:10fd4f94efa4 > --------------------------- > Clock: Rework clocks to avoid tick-to-cycle transformations > > This patch introduces the notion of a clock update function that aims > to avoid costly divisions when turning the current tick into a > cycle. Each clocked object advances a private (hidden) cycle member > and a tick member and uses these to implement functions for getting > the tick of the next cycle, or the tick of a cycle some time in the > future. > > In the different modules using the clocks, changes are made to avoid > counting in ticks only to later translate to cycles. There are a few > oddities in how the O3 and inorder CPU count idle cycles, as seen by a > few locations where a cycle is subtracted in the calculation. This is > done such that the regression does not change any stats, but should be > revisited in a future patch. > > Another, much needed, change that is not done as part of this patch is > to introduce a new typedef uint64_t Cycle to be able to at least hint > at the unit of the variables counting Ticks vs Cycles. This will be > done as a follow-up patch. > > As an additional follow up, the thread context still uses ticks for > the book keeping of last activate and last suspend and this should > probably also be changed into cycles as well. > > > Diffs > ----- > > src/arch/arm/table_walker.cc b4d0bdb52694 > src/arch/x86/mmapped_ipr.hh b4d0bdb52694 > src/cpu/base.cc b4d0bdb52694 > src/cpu/inorder/cpu.hh b4d0bdb52694 > src/cpu/inorder/cpu.cc b4d0bdb52694 > src/cpu/inorder/resource.cc b4d0bdb52694 > src/cpu/inorder/resource_pool.cc b4d0bdb52694 > src/cpu/o3/O3CPU.py b4d0bdb52694 > src/cpu/o3/commit.hh b4d0bdb52694 > src/cpu/o3/commit_impl.hh b4d0bdb52694 > src/cpu/o3/cpu.hh b4d0bdb52694 > src/cpu/o3/cpu.cc b4d0bdb52694 > src/cpu/o3/fetch_impl.hh b4d0bdb52694 > src/cpu/o3/inst_queue_impl.hh b4d0bdb52694 > src/cpu/o3/lsq_unit.hh b4d0bdb52694 > src/cpu/simple/atomic.cc b4d0bdb52694 > src/cpu/simple/timing.hh b4d0bdb52694 > src/cpu/simple/timing.cc b4d0bdb52694 > src/cpu/testers/memtest/memtest.cc b4d0bdb52694 > src/cpu/testers/networktest/networktest.cc b4d0bdb52694 > src/dev/arm/pl111.cc b4d0bdb52694 > src/dev/i8254xGBe.cc b4d0bdb52694 > src/dev/ns_gige.cc b4d0bdb52694 > src/sim/clocked_object.hh PRE-CREATION > > Diff: http://reviews.gem5.org/r/1321/diff/ > > > Testing > ------- > > util/regress all passing (disregarding t1000 and eio) > > A minor update. This change did improve performance. Running the > full regression, including a clean compile of all the ISAs went > down by 8%. Note that this includes the time for building as well. > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
