----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1321/#review3300 -----------------------------------------------------------
Overall this looks great. It would be nice if we could figure out how to make things like trapLatency below have their latencies depend on other clocks. That way if you change the frequency of one clock all dependent clocks get updated automatically. This is along the lines of what Joel was e-mailing about. src/arch/x86/mmapped_ipr.hh <http://reviews.gem5.org/r/1321/#comment3391> Why is the return value here 1 when the return type is still Tick? src/arch/x86/mmapped_ipr.hh <http://reviews.gem5.org/r/1321/#comment3395> ditto src/cpu/inorder/cpu.cc <http://reviews.gem5.org/r/1321/#comment3392> This seems to be an absolute cycle. Do we no longer require that cycles are relative? We ensure that cycles are always increasing? src/cpu/o3/O3CPU.py <http://reviews.gem5.org/r/1321/#comment3394> Be nice to have a Param type that specifically indicates Cycles, but I understand why not. src/cpu/o3/commit.hh <http://reviews.gem5.org/r/1321/#comment3393> Do we allow uint as a type? Seems like we should use unsigned since we don't use uint elsewhere. - Nathan Binkert On Aug. 23, 2012, 2:48 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1321/ > ----------------------------------------------------------- > > (Updated Aug. 23, 2012, 2:48 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9168:571d3ae56173 > --------------------------- > 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 1d983855df2c > src/arch/x86/mmapped_ipr.hh 1d983855df2c > src/cpu/base.cc 1d983855df2c > src/cpu/inorder/cpu.hh 1d983855df2c > src/cpu/inorder/cpu.cc 1d983855df2c > src/cpu/inorder/resource.cc 1d983855df2c > src/cpu/inorder/resource_pool.cc 1d983855df2c > src/cpu/o3/O3CPU.py 1d983855df2c > src/cpu/o3/commit.hh 1d983855df2c > src/cpu/o3/commit_impl.hh 1d983855df2c > src/cpu/o3/cpu.hh 1d983855df2c > src/cpu/o3/cpu.cc 1d983855df2c > src/cpu/o3/fetch_impl.hh 1d983855df2c > src/cpu/o3/inst_queue_impl.hh 1d983855df2c > src/cpu/o3/lsq_unit.hh 1d983855df2c > src/cpu/simple/atomic.cc 1d983855df2c > src/cpu/simple/timing.hh 1d983855df2c > src/cpu/simple/timing.cc 1d983855df2c > src/cpu/testers/memtest/memtest.cc 1d983855df2c > src/cpu/testers/networktest/networktest.cc 1d983855df2c > src/dev/arm/pl111.cc 1d983855df2c > src/dev/i8254xGBe.cc 1d983855df2c > src/dev/ns_gige.cc 1d983855df2c > src/sim/clocked_object.hh 1d983855df2c > > 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
