> On July 27, 2012, 6:50 a.m., Ali Saidi wrote: > > this sort of thing worries me because of the possibilities of introducing > > subtle bugs, but it looks like a good change long term.
Agreed. I think for DVFS it is practically a must as the fixed relation between cycles and ticks will no longer be true. Ultimately it would be a blessing if C++11 units could help out, but that is probably quite a few years away :) The Cycle typedef should be a good starting point to at least indicate what the intention is. In a future patch I will also change the name of the ClockedObject::clock member to period to try and avoid confusion. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1321/#review3162 ----------------------------------------------------------- On July 27, 2012, 3:01 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1321/ > ----------------------------------------------------------- > > (Updated July 27, 2012, 3:01 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9134:93f1c90b3226 > --------------------------- > 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 b57966a6c512 > src/arch/x86/mmapped_ipr.hh b57966a6c512 > src/cpu/inorder/cpu.hh b57966a6c512 > src/cpu/inorder/cpu.cc b57966a6c512 > src/cpu/inorder/resource_pool.cc b57966a6c512 > src/cpu/o3/cpu.hh b57966a6c512 > src/cpu/o3/cpu.cc b57966a6c512 > src/cpu/o3/fetch_impl.hh b57966a6c512 > src/cpu/o3/lsq_unit.hh b57966a6c512 > src/cpu/simple/atomic.cc b57966a6c512 > src/cpu/simple/timing.hh b57966a6c512 > src/cpu/simple/timing.cc b57966a6c512 > src/dev/arm/pl111.cc b57966a6c512 > 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
