> On May 12, 2015, 4:22 p.m., Joel Hestness wrote: > > This change seems to go away from its apparent aim: > > > > First, this patch seems to be aimed at correcting some common confusion: > > The RubyCache latency parameter is strange, because the cache itself does > > not enforce the latency. Instead, the protocol controllers enforce cache > > access latencies, which can be defined either as part of the cache or as > > parameters to the controller. This current organization disagregates the > > accountability for enforcing reasonable cache read/write latency, and makes > > it very confusing/difficult to validate memory access times. > > > > However, just defaulting the RubyCache latency to 0 doesn't fix any > > confusion about protocol memory access time validation, and actually > > encourages poorer use of the latency parameter. Right now, when I open any > > configs/ruby/*.py protocol file, I see a latency declaration in a cache > > with a comment that says the latency is ignored (except sometimes for fast > > path hits...?). Why declare a cache latency if it won't be used?! This > > patch could eliminate the need to forward declare cache latencies, but the > > cache declaration itself must remain. This means that this patch would > > leave numerous cache declarations with confusing, unused latency > > declarations even though cache latencies now default to 0 cycles (which - I > > suppose - could be used to indicate that the latency is not used, but > > again, what should happen if I change the latency?). Worse, new protocols > > might not even define a cache's unused latencies, leaving the user baffled > > about why they can change a cache's latency without it affecting simulated > > system performance. > > > > We should be aiming to either use or lose the RubyCache latency parameter, > > but *not* settle somewhere between. Here are the two extremes: > > 1) All RubyCache latencies *must* be defined and enforced by their > > respective protocols. In config files, this would involve passing the > > RubyCache's latency variable as a parameter to the controllers that need > > them. In the protocol files, controllers could also enforce these latencies > > by referencing the cache's latency during enqueue to a buffer. This would > > actually make memory access times depend on the cache latency parameter > > (though in practice, it's hard to make sure protocols do this). > > > > 2) Get rid of the RubyCache latency parameter altogether. This would > > require protocols to parameterize any cache latencies (most already do, > > though sometimes aggregated with other latencies, which is cumbersome and > > very fragile). This would side-step user confusion about the RubyCache > > latency variable. > > > > The former option is probably better: We've added tag and data array > > resource conflicts to the RubyCache, so it is partially responsible for > > enforcing latencies already. Further, it would be easier to find and change > > a cache's latency rather than digging through a controller file to see how > > latencies are used and setting some aggregated parameter. Finally, the > > RubyCache's latency is saved to config.ini, so it would be easier to > > reference prior simulations for comparison and validation.
Joel, I understand your concern, but I believe this is at least a step in the right direction. In the end, I believe that the second option you describe is where we want to be. In real systems, cache latency is more complicated than a simple single parameter. This patch at least eliminates the latency parameter from those protocols that want to be smarter. A later patch can remove it altogether. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2796/#review6170 ----------------------------------------------------------- On May 11, 2015, 10:21 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2796/ > ----------------------------------------------------------- > > (Updated May 11, 2015, 10:21 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10853:1ce6a4401f97 > --------------------------- > ruby: set default latency for ruby caches > > Set to 0 since many protocols do not use the parameter. > > > Diffs > ----- > > src/mem/ruby/structures/Cache.py fbdaa08aaa426b9f4660c366f934ccb670d954ec > > Diff: http://reviews.gem5.org/r/2796/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
