Overall, I really like the direction of this patch, since replacement policy should be handled modularly by the cache controllers rather than the Sequencer.
I wonder if it would be possible to limit the number of ways that MRU can be updated. Currently, this patch uses (1) lookup() and setMRU() in actions, and (2) some specific new actions that just call setMRU(). Previously, setMRU was called from a single place in the Sequencer, so there wasn't any worry of overlooking a place where replacement policy needed to be updated. Now, it's a little more complicated and there aren't any safeguards ensuring that one remembers to update MRU.

src/mem/protocol/MESI_Three_Level-L0cache.sm (Diff revision 1)

Minor: Convention in other SLICC controller functions is to name function arguments using lower_underscore rather than lowerCamelCase.

Show all issues

1. Nilay Vaish July 14, 2015, 8:10 a.m. (July 14, 2015, 8:10 a.m.)

   Will fix this before commit.

src/mem/protocol/MESI_Three_Level-L0cache.sm (Diff revision 1)

Is it necessary to update the replacement policy through an additional action like this? This seems pretty fragile if a user forgets to include this in transitions analogous to this one. It would also be nice to have a single process for updating replacement policy.

Show all issues

1. Nilay Vaish July 14, 2015, 8:10 a.m. (July 14, 2015, 8:10 a.m.)

   I don't like doing this either.  I am thinking of adding functions that
   
   return the set id and way id within a set.  The lookup would be done
   
   in two steps and replacement policy can be updated by using a version of 
   
   setMRU that takes set and way as argument.

src/mem/protocol/RubySlicc_Types.sm (Diff revision 1)

Minor: Could you use the default parameter specifier here so you could avoid passing static 'false' in a few cases?

1. Nilay Vaish July 14, 2015, 8:10 a.m. (July 14, 2015, 8:10 a.m.)

   I would really like to do that.  The problem is with SLICC.  It handles class
   
   member functions differently from how it handles stand alone functions.
   
   You can specify defaults with stand alone functions but not with member
   
   functions.  Look at the function MemberMethodCallExprAST::generate_prefix() in
   
   file src/mem/slicc/ast/MethodCallExprAST.py.  Unless that function is fixed,
   
   it is not possible.  I have not still not made up my mind on fixing that function.