On Mon, 6 Jul 2015, Brandon Potter wrote:



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?



In this particular case , I probably would make CacheMemory a clocked object and pass curTick() from the cache object to its banks. Other components may need to be decided upon individually.

You are the author, so I'll the decision, of whether or not to have more clocked objects, to you.

--
Nilay
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to