-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2981/#review6893
-----------------------------------------------------------


Nilay said
> It takes about 50ns per call to findTagInSet (on the machine I use).  All
> those times when the line is found in the first level cache, we would not
> those extra calls.  Overall, a simulation would be about 2% faster.

I personally don't think that 2% performance is worth the likelyhood that 
users' protocols will be broken. A change like this one was exactly the kind of 
thing I talked about in my workshop presentation. This change may cause 
significant headaches for gem5 users. My *personal opinion* is that 2% 
performance isn't worth it.

I think that we should only include this change if most of the community wants 
to move setMRU into the L1 controllers, instead of being implicit for the L1s, 
as it is now. If we do decide to move this way, I think we need to broadcast 
the change loudly. One option is to set the implicit version as deprecated and 
print warnings if the cache lines are not explicitly setMRU. We could have one 
version of gem5 stable with the warning, then the next gem5 stable can remove 
the warning and move to requiring explicit use of setMRU in the L1 controllers.

Also, Nilay, to echo Brad's previous requests, could you please respond to 
things on reviewboard. Reviewboard is where most people go to see the whole 
conversation. By responding via email it makes it very difficult for anyone to 
follow the conversation.

- Jason Power


On Aug. 2, 2015, 5:42 p.m., Nilay Vaish wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2981/
> -----------------------------------------------------------
> 
> (Updated Aug. 2, 2015, 5:42 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11002:86776d8b9a7d
> ---------------------------
> 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

Reply via email to