> On May 25, 2015, 5:21 p.m., Andreas Hansson wrote: > > I am not familiar with the Ruby caches, but on the classic side of things > > we have recently _added_ cache parameters, for misses, forwarding, snopping > > etc. I am mostly surprised to see that Ruby only has a single parameter for > > the cache latency, and now even that one is being removed/moved. Is there > > not a case for keeping them similar and encapsulate the cache-related > > timings by leaving them as a responsibility of the cache? > > Joel Hestness wrote: > Yes, there is a case to be made for keeping the cache latencies within > the cache objects and then actually using that value to assess the latency in > the protocol controllers. Such a plan could make it much clearer how > latencies are imposed. > > Unfortunately, SLICC doesn't exactly make this simple or rigid, so > existing coherence protocols have lumped cache access latencies with other > controller latencies to expose as few latency parameters as possible when > instantiating a cache controller in config files. Part of the problem is that > the controller imposes the latencies rather than the CacheMemory object > itself, which functionally handles tag checks, line allocation/deallocation, > and data accesses. Protocol authors would need to instantiate delay queues in > the controllers to impose these cache latencies. I think it would be > straightforward to do this, but it would take a while to get all existing > protocols fixed up. > > I prepared this patch in an effort to dissuade us from accepting AMD's > proposed patch, http://reviews.gem5.org/r/2796/, which aims to default cache > latencies to 0 to avoid defining a latency value for some caches in the > protocol config files. This patch is a different route to that desired > effect, though we could require that the existing parameter be used > appropriately (as you suggest). I feel that either would be better than > defaulting the latencies to 0.
Ah, thanks for the clarification. I certainly do not oppose the patch, I was mostly trying to understand the rationale behind it. I agree with the "better than 0" statement. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2841/#review6391 ----------------------------------------------------------- On May 21, 2015, 4:05 p.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2841/ > ----------------------------------------------------------- > > (Updated May 21, 2015, 4:05 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10838:d02a5584e395 > --------------------------- > ruby: Remove the RubyCache/CacheMemory latency > > The RubyCache (CacheMemory) latency parameter is only used for top-level > caches > instantiated for Ruby coherence protocols. However, the top-level cache hit > latency is assessed by the Sequencer as accesses flow through to the cache > hierarchy. Further, protocol state machines should be enforcing these cache > hit > latencies, but RubyCaches do not expose their latency to any existng state > machines through the SLICC/C++ interface. Thus, the RubyCache latency > parameter > is superfluous for all caches. This is confusing for users. > > As a step toward pushing L0/L1 cache hit latency into the top-level cache > controllers, move their latencies out of the RubyCache declarations and over > to > their Sequencers. Eventually, these Sequencer parameters should be exposed as > parameters to the top-level cache controllers, which should assess the > latency. > NOTE: Assessing these latencies in the cache controllers will require > modifying > each to eliminate instantaneous Ruby hit callbacks in transitions that finish > accesses, which is likely a large undertaking. > > > Diffs > ----- > > configs/ruby/MOESI_CMP_token.py ecbab2522757 > configs/ruby/MOESI_hammer.py ecbab2522757 > configs/ruby/Network_test.py ecbab2522757 > src/mem/ruby/structures/Cache.py ecbab2522757 > src/mem/ruby/structures/CacheMemory.hh ecbab2522757 > src/mem/ruby/structures/CacheMemory.cc ecbab2522757 > src/mem/ruby/system/Sequencer.hh ecbab2522757 > src/mem/ruby/system/Sequencer.cc ecbab2522757 > src/mem/ruby/system/Sequencer.py ecbab2522757 > configs/ruby/MESI_Three_Level.py ecbab2522757 > configs/ruby/MESI_Two_Level.py ecbab2522757 > configs/ruby/MI_example.py ecbab2522757 > configs/ruby/MOESI_CMP_directory.py ecbab2522757 > > Diff: http://reviews.gem5.org/r/2841/diff/ > > > Testing > ------- > > Small tests with all different protocols to verify that performance does not > change. > > Please consider this patch as a substitute for http://reviews.gem5.org/r/2796/ > > > Thanks, > > Joel Hestness > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
