> 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;
> >     
> >

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?


- Andreas


-----------------------------------------------------------
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

Reply via email to