> On Aug. 24, 2012, 9:07 a.m., Nathan Binkert wrote: > > I looked at the big code blocks in depth and skimmed the rest. The biggest > > bizarre thing to me is where there is a comment about thread ID and you > > replace the value with Cycles. I looked at the declarations and it looks > > like your right, but is there any breakage from the threadid thing or are > > they all just bad comments?
Where is this exactly? I'll have another look. All the regressions are unchanged with the current patch. > On Aug. 24, 2012, 9:07 a.m., Nathan Binkert wrote: > > src/base/types.hh, line 85 > > <http://reviews.gem5.org/r/1338/diff/4/?file=29115#file29115line85> > > > > Any reason this shouldn't be private? Fixed and re-running. > On Aug. 24, 2012, 9:07 a.m., Nathan Binkert wrote: > > src/base/types.hh, line 107 > > <http://reviews.gem5.org/r/1338/diff/4/?file=29115#file29115line107> > > > > How about equality (or all of them really)? Is that really never used? I only added the ones that are currently used. I would suggest to add or remove as needed rather than doing it pro-actively. Agreed? - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1338/#review3309 ----------------------------------------------------------- On Aug. 23, 2012, 2:44 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1338/ > ----------------------------------------------------------- > > (Updated Aug. 23, 2012, 2:44 p.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9169:9c3c49e0eb85 > --------------------------- > Clock: Add a Cycles wrapper class and use where applicable > > This patch addresses the comments and feedback on the preceding patch > that reworks the clocks and now more clearly shows where cycles > (relative cycle counts) are used to express time. > > Instead of bumping the existing patch I chose to make this a separate > patch, merely to try and focus the discussion around a smaller set of > changes. The two patches will be pushed together though. > > This changes done as part of this patch are mostly following directly > from the introduction of the wrapper class, and change enough code to > make things compile and run again. There are definitely more places > where int/uint/Tick is still used to represent cycles, and it will > take some time to chase them all down. Similarly, a lot of parameters > should be changed from Param.Tick and Param.Unsigned to > Param.Cycles. > > In addition, the use of curTick is questionable as there should not be > an absolute cycle. Potential solutions can be built on top of this > patch. There is a similar situation in the o3 CPU where > lastRunningCycle is currently counting in Cycles, and is still an > absolute time. More discussion to be had in other words. > > An additional change that would be appropriate in the future is to > perform a similar wrapping of Tick and probably also introduce a > Ticks class along with suitable operators for all these classes. > > > Diffs > ----- > > src/arch/alpha/mmapped_ipr.hh 1d983855df2c > src/arch/alpha/utility.hh 1d983855df2c > src/arch/arm/mmapped_ipr.hh 1d983855df2c > src/arch/arm/table_walker.cc 1d983855df2c > src/arch/arm/utility.hh 1d983855df2c > src/arch/mips/isa.hh 1d983855df2c > src/arch/mips/isa.cc 1d983855df2c > src/arch/mips/mmapped_ipr.hh 1d983855df2c > src/arch/mips/mt.hh 1d983855df2c > src/arch/mips/utility.cc 1d983855df2c > src/arch/power/mmapped_ipr.hh 1d983855df2c > src/arch/power/utility.hh 1d983855df2c > src/arch/sparc/mmapped_ipr.hh 1d983855df2c > src/arch/sparc/tlb.hh 1d983855df2c > src/arch/sparc/tlb.cc 1d983855df2c > src/arch/sparc/ua2005.cc 1d983855df2c > src/arch/sparc/utility.hh 1d983855df2c > src/arch/x86/mmapped_ipr.hh 1d983855df2c > src/arch/x86/utility.cc 1d983855df2c > src/base/types.hh 1d983855df2c > src/cpu/BaseCPU.py 1d983855df2c > src/cpu/base.hh 1d983855df2c > src/cpu/checker/thread_context.hh 1d983855df2c > src/cpu/inorder/cpu.hh 1d983855df2c > src/cpu/inorder/cpu.cc 1d983855df2c > src/cpu/inorder/pipeline_stage.cc 1d983855df2c > src/cpu/inorder/resource.hh 1d983855df2c > src/cpu/inorder/resource.cc 1d983855df2c > src/cpu/inorder/resource_pool.hh 1d983855df2c > src/cpu/inorder/resource_pool.cc 1d983855df2c > src/cpu/inorder/resources/agen_unit.hh 1d983855df2c > src/cpu/inorder/resources/agen_unit.cc 1d983855df2c > src/cpu/inorder/resources/branch_predictor.hh 1d983855df2c > src/cpu/inorder/resources/branch_predictor.cc 1d983855df2c > src/cpu/inorder/resources/cache_unit.hh 1d983855df2c > src/cpu/inorder/resources/cache_unit.cc 1d983855df2c > src/cpu/inorder/resources/decode_unit.hh 1d983855df2c > src/cpu/inorder/resources/decode_unit.cc 1d983855df2c > src/cpu/inorder/resources/execution_unit.hh 1d983855df2c > src/cpu/inorder/resources/execution_unit.cc 1d983855df2c > src/cpu/inorder/resources/fetch_seq_unit.hh 1d983855df2c > src/cpu/inorder/resources/fetch_seq_unit.cc 1d983855df2c > src/cpu/inorder/resources/fetch_unit.hh 1d983855df2c > src/cpu/inorder/resources/fetch_unit.cc 1d983855df2c > src/cpu/inorder/resources/graduation_unit.hh 1d983855df2c > src/cpu/inorder/resources/graduation_unit.cc 1d983855df2c > src/cpu/inorder/resources/inst_buffer.hh 1d983855df2c > src/cpu/inorder/resources/inst_buffer.cc 1d983855df2c > src/cpu/inorder/resources/mem_dep_unit.hh 1d983855df2c > src/cpu/inorder/resources/mult_div_unit.hh 1d983855df2c > src/cpu/inorder/resources/mult_div_unit.cc 1d983855df2c > src/cpu/inorder/resources/tlb_unit.hh 1d983855df2c > src/cpu/inorder/resources/tlb_unit.cc 1d983855df2c > src/cpu/inorder/resources/use_def.hh 1d983855df2c > src/cpu/inorder/resources/use_def.cc 1d983855df2c > src/cpu/inorder/thread_context.hh 1d983855df2c > src/cpu/inorder/thread_context.cc 1d983855df2c > src/cpu/o3/commit.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/o3/thread_context.hh 1d983855df2c > src/cpu/o3/thread_context_impl.hh 1d983855df2c > src/cpu/simple/atomic.hh 1d983855df2c > src/cpu/simple/atomic.cc 1d983855df2c > src/cpu/simple/timing.hh 1d983855df2c > src/cpu/simple/timing.cc 1d983855df2c > src/cpu/simple_thread.hh 1d983855df2c > src/cpu/simple_thread.cc 1d983855df2c > src/cpu/testers/memtest/memtest.cc 1d983855df2c > src/cpu/testers/networktest/networktest.cc 1d983855df2c > src/cpu/thread_context.hh 1d983855df2c > src/dev/arm/pl111.cc 1d983855df2c > src/dev/i8254xGBe.cc 1d983855df2c > src/dev/sinic.cc 1d983855df2c > src/mem/bridge.hh 1d983855df2c > src/mem/bridge.cc 1d983855df2c > src/python/m5/params.py 1d983855df2c > src/sim/clocked_object.hh 1d983855df2c > src/sim/process.cc 1d983855df2c > src/sim/pseudo_inst.cc 1d983855df2c > > Diff: http://reviews.gem5.org/r/1338/diff/ > > > Testing > ------- > > util/regress all passing (disregarding t1000 and eio) > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
