> 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?
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. - Joel ----------------------------------------------------------- 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
