> On July 31, 2015, 4:05 p.m., Joel Hestness wrote: > > I'm still pretty ambivalent about this change. There are now 4 potential > > ways to update MRU: (1) direct call from an action to a cache's setMRU, > > (2)/(3) calling an action from a transition that updates a specific cache's > > MRU, and (4) calling an action from a transition that updates the MRU of > > all caches. This seems quite fragile and could result in all kinds of very > > difficult-to-debug performance problems. > > > > I feel that we should modify some more with the aim of keeping replacement > > policy updates within actions that call hit callbacks. This might mean > > adding actions that are specific to I- vs. D-caches, but it would limit the > > number of calls to CacheMemory::setMRU() as well as the number of > > potentially sneaky performance bugs.
Joel, the same problem can happen with split callback functions. You may make updates to one but not to the other. I think we have to live with these multiple ways. > On July 31, 2015, 4:05 p.m., Joel Hestness wrote: > > src/mem/protocol/MOESI_CMP_token-L1cache.sm, line 1287 > > <http://reviews.gem5.org/r/2981/diff/1/?file=48286#file48286line1287> > > > > In this file, it appears that sm_setMruCache is only ever called from > > before x_external_load_hit and xx_external_store_hit. Can we just move the > > MRU updates into those actions instead? We can, but I want to keep those two 'actions' separate. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2981/#review6870 ----------------------------------------------------------- On Aug. 1, 2015, 6:37 p.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2981/ > ----------------------------------------------------------- > > (Updated Aug. 1, 2015, 6:37 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11001:d9b0c6ae77b1 > --------------------------- > ruby: call setMRU from L1 controllers, not from sequencer > Currently the sequencer calls the function setMRU that updates the replacement > policy structures with the first level caches. While functionally this is > correct, the problem is that this requires calling findTagInSet() which is an > expensive function. This patch removes the calls to setMRU from the > sequencer. > All controllers should now update the replacement policy on their own. > > The set and the way index for a given cache entry can be found within the > AbstractCacheEntry structure. Use these indicies to update the replacement > policy structures. > > > Diffs > ----- > > src/mem/protocol/MESI_Three_Level-L0cache.sm a618349a7953 > src/mem/protocol/MESI_Two_Level-L1cache.sm a618349a7953 > src/mem/protocol/MI_example-cache.sm a618349a7953 > src/mem/protocol/MOESI_CMP_directory-L1cache.sm a618349a7953 > src/mem/protocol/MOESI_CMP_token-L1cache.sm a618349a7953 > src/mem/protocol/MOESI_hammer-cache.sm a618349a7953 > src/mem/protocol/RubySlicc_Types.sm a618349a7953 > src/mem/ruby/structures/CacheMemory.hh a618349a7953 > src/mem/ruby/structures/CacheMemory.cc a618349a7953 > src/mem/ruby/system/Sequencer.cc a618349a7953 > > Diff: http://reviews.gem5.org/r/2981/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
