> On July 2, 2015, 5:31 a.m., Nilay Vaish wrote:
> > src/mem/ruby/structures/BankedArray.cc, line 74
> > <http://reviews.gem5.org/r/2902/diff/3/?file=47066#file47066line74>
> >
> >     I strongly suggest that you write a patch that is applied before this 
> > ruby system patch.  The new patch would do away with this tick handling.  
> > Everything should be handled in Cycles.  The function tryAccess() would 
> > receive a current cycle time value as input, which means that curTick() 
> > would not be called.  When CacheMemory object calls tryAccess(), either 
> > CacheMemory should itself receive a current cycle value as input, or 
> > CacheMemory itself should be converted to a ClockedObject.  Ultimately the 
> > BankedArray class should not require ruby system at all.

Hi Nilay, thanks for the review.  The other issues were clear errors and the 
address profiler catch was particularly good.

It looks like other code in Ruby relies on tick calculations instead of working 
directly with cycles.  The start_cycle field (which has been moved into 
RubySystem) also relies on RubySystem being a clocked object (it was previously 
a global field).  So, I expect that what you are suggesting that I do will not 
be trivial.  Would also argue that it's only indirectly related to this patch 
in that the patch exposes it as a problem (where people were previously taking 
the global pointers for granted).  I am not disagreeing that it is a problem or 
that it couldn't use a bit of a cleanup, but it's beyond the scope of what I am 
trying to do here to enable multi-instance Ruby support.

For clarity, what did you have in mind for ruby components that are not clocked 
objects, but rely on the ruby system to provide their clock (directly or 
indirectly)?  I could make them clocked objects and pass in their clock in the 
configuration files explicitly but that seems like a headache.  Did you have a 
cleaner solution in mind?


> On July 2, 2015, 5:31 a.m., Nilay Vaish wrote:
> > src/mem/slicc/symbols/StateMachine.py, line 41
> > <http://reviews.gem5.org/r/2902/diff/3/?file=47081#file47081line41>
> >
> >     Really! How come Brad and Steve not chide you for this?

They did and I fixed it! :P


- Brandon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2902/#review6669
-----------------------------------------------------------


On June 26, 2015, 5:37 p.m., Brandon Potter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2902/
> -----------------------------------------------------------
> 
> (Updated June 26, 2015, 5:37 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 10877:2e6e2150542a
> ---------------------------
> ruby: removes g_system_ptr and replaces with object based references
> 
> 
> Diffs
> -----
> 
>   src/mem/ruby/common/Global.hh ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/common/Global.cc ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/network/MessageBuffer.hh 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/network/MessageBuffer.cc 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/network/simple/Throttle.cc 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/profiler/AddressProfiler.hh 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/profiler/AddressProfiler.cc 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/profiler/Profiler.hh ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/profiler/Profiler.cc ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/structures/BankedArray.hh 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/structures/BankedArray.cc 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/structures/Cache.py ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/structures/CacheMemory.cc 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/structures/RubyMemoryControl.cc 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/structures/WireBuffer.hh 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/structures/WireBuffer.cc 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/structures/WireBuffer.py 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/system/DMASequencer.hh 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/system/DMASequencer.cc 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/system/RubyPort.hh ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/system/RubyPort.cc ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/system/Sequencer.cc ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/system/Sequencer.py ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/system/System.hh ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/ruby/system/System.cc ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/slicc/symbols/StateMachine.py 
> ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
>   src/mem/slicc/symbols/Type.py ebb3d0737aa72ec4fa24b6af9cf9a6b2a1109d18 
> 
> Diff: http://reviews.gem5.org/r/2902/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to