Discussion:
[gem5-dev] Review Request 2957: ruby: call setMRU from L1 controllers, not from sequencer
(too old to reply)
Nilay Vaish
2015-07-10 16:29:37 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2957/
-----------------------------------------------------------

Review request for Default.


Repository: gem5


Description
-------

Changeset 10933:184eac6db147
---------------------------
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.
The first level controllers shoul now update the replacement policy. If the
access is a cache hit, the function findTagInSet is called only once when the
block is first found (common case). If the access is a miss, then the
controller calls setMRU() just before calling the sequencer for returning the
data.


Diffs
-----

src/mem/protocol/MESI_Three_Level-L0cache.sm 5c76426fd9ee
src/mem/protocol/MESI_Three_Level-L1cache.sm 5c76426fd9ee
src/mem/protocol/MESI_Two_Level-L1cache.sm 5c76426fd9ee
src/mem/protocol/MESI_Two_Level-L2cache.sm 5c76426fd9ee
src/mem/protocol/MI_example-cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_directory-L1cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_directory-L2cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_token-L1cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_token-L2cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_hammer-cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_hammer-dir.sm 5c76426fd9ee
src/mem/protocol/RubySlicc_Types.sm 5c76426fd9ee
src/mem/ruby/structures/CacheMemory.hh 5c76426fd9ee
src/mem/ruby/structures/CacheMemory.cc 5c76426fd9ee
src/mem/ruby/system/Sequencer.cc 5c76426fd9ee

Diff: http://reviews.gem5.org/r/2957/diff/


Testing
-------


Thanks,

Nilay Vaish
Joel Hestness
2015-07-13 20:16:57 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2957/#review6744
-----------------------------------------------------------


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 (line 147)
<http://reviews.gem5.org/r/2957/#comment5848>

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



src/mem/protocol/MESI_Three_Level-L0cache.sm (line 697)
<http://reviews.gem5.org/r/2957/#comment5847>

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.



src/mem/protocol/RubySlicc_Types.sm (line 150)
<http://reviews.gem5.org/r/2957/#comment5849>

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


- Joel Hestness
Post by Nilay Vaish
-----------------------------------------------------------
http://reviews.gem5.org/r/2957/
-----------------------------------------------------------
(Updated July 10, 2015, 4:29 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10933:184eac6db147
---------------------------
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.
The first level controllers shoul now update the replacement policy. If the
access is a cache hit, the function findTagInSet is called only once when the
block is first found (common case). If the access is a miss, then the
controller calls setMRU() just before calling the sequencer for returning the
data.
Diffs
-----
src/mem/protocol/MESI_Three_Level-L0cache.sm 5c76426fd9ee
src/mem/protocol/MESI_Three_Level-L1cache.sm 5c76426fd9ee
src/mem/protocol/MESI_Two_Level-L1cache.sm 5c76426fd9ee
src/mem/protocol/MESI_Two_Level-L2cache.sm 5c76426fd9ee
src/mem/protocol/MI_example-cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_directory-L1cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_directory-L2cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_token-L1cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_token-L2cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_hammer-cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_hammer-dir.sm 5c76426fd9ee
src/mem/protocol/RubySlicc_Types.sm 5c76426fd9ee
src/mem/ruby/structures/CacheMemory.hh 5c76426fd9ee
src/mem/ruby/structures/CacheMemory.cc 5c76426fd9ee
src/mem/ruby/system/Sequencer.cc 5c76426fd9ee
Diff: http://reviews.gem5.org/r/2957/diff/
Testing
-------
Thanks,
Nilay Vaish
Nilay Vaish
2015-07-14 15:10:22 UTC
Permalink
Post by Nilay Vaish
src/mem/protocol/MESI_Three_Level-L0cache.sm, line 147
<http://reviews.gem5.org/r/2957/diff/1/?file=47973#file47973line147>
Minor: Convention in other SLICC controller functions is to name function arguments using lower_underscore rather than lowerCamelCase.
Will fix this before commit.
Post by Nilay Vaish
src/mem/protocol/RubySlicc_Types.sm, line 150
<http://reviews.gem5.org/r/2957/diff/1/?file=47984#file47984line150>
Minor: Could you use the default parameter specifier here so you could avoid passing static 'false' in a few cases?
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.
Post by Nilay Vaish
src/mem/protocol/MESI_Three_Level-L0cache.sm, line 697
<http://reviews.gem5.org/r/2957/diff/1/?file=47973#file47973line697>
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.
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.


- Nilay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2957/#review6744
-----------------------------------------------------------
Post by Nilay Vaish
-----------------------------------------------------------
http://reviews.gem5.org/r/2957/
-----------------------------------------------------------
(Updated July 10, 2015, 4:29 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10933:184eac6db147
---------------------------
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.
The first level controllers shoul now update the replacement policy. If the
access is a cache hit, the function findTagInSet is called only once when the
block is first found (common case). If the access is a miss, then the
controller calls setMRU() just before calling the sequencer for returning the
data.
Diffs
-----
src/mem/protocol/MESI_Three_Level-L0cache.sm 5c76426fd9ee
src/mem/protocol/MESI_Three_Level-L1cache.sm 5c76426fd9ee
src/mem/protocol/MESI_Two_Level-L1cache.sm 5c76426fd9ee
src/mem/protocol/MESI_Two_Level-L2cache.sm 5c76426fd9ee
src/mem/protocol/MI_example-cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_directory-L1cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_directory-L2cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_token-L1cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_CMP_token-L2cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_hammer-cache.sm 5c76426fd9ee
src/mem/protocol/MOESI_hammer-dir.sm 5c76426fd9ee
src/mem/protocol/RubySlicc_Types.sm 5c76426fd9ee
src/mem/ruby/structures/CacheMemory.hh 5c76426fd9ee
src/mem/ruby/structures/CacheMemory.cc 5c76426fd9ee
src/mem/ruby/system/Sequencer.cc 5c76426fd9ee
Diff: http://reviews.gem5.org/r/2957/diff/
Testing
-------
Thanks,
Nilay Vaish
Nilay Vaish
2015-07-19 23:43:33 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2957/
-----------------------------------------------------------

(Updated July 19, 2015, 11:43 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
-------

Changeset 10938:f35b9b0e6cb2
---------------------------
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 (updated)
-----

src/mem/protocol/MESI_Three_Level-L0cache.sm 4a3f90af41c5
src/mem/protocol/MESI_Two_Level-L1cache.sm 4a3f90af41c5
src/mem/protocol/MI_example-cache.sm 4a3f90af41c5
src/mem/protocol/MOESI_CMP_directory-L1cache.sm 4a3f90af41c5
src/mem/protocol/MOESI_CMP_token-L1cache.sm 4a3f90af41c5
src/mem/protocol/MOESI_hammer-cache.sm 4a3f90af41c5
src/mem/protocol/RubySlicc_Types.sm 4a3f90af41c5
src/mem/ruby/structures/CacheMemory.hh 4a3f90af41c5
src/mem/ruby/structures/CacheMemory.cc 4a3f90af41c5
src/mem/ruby/system/Sequencer.cc 4a3f90af41c5

Diff: http://reviews.gem5.org/r/2957/diff/


Testing
-------


Thanks,

Nilay Vaish
Nilay Vaish
2015-07-19 23:44:22 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2957/
-----------------------------------------------------------

(Updated July 19, 2015, 11:44 p.m.)


Review request for Default.


Repository: gem5


Description
-------

Changeset 10938:f35b9b0e6cb2
---------------------------
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 (updated)
-----

src/mem/protocol/MESI_Three_Level-L0cache.sm 3a925f9856b1
src/mem/protocol/MESI_Two_Level-L1cache.sm 3a925f9856b1
src/mem/protocol/MI_example-cache.sm 3a925f9856b1
src/mem/protocol/MOESI_CMP_directory-L1cache.sm 3a925f9856b1
src/mem/protocol/MOESI_CMP_token-L1cache.sm 3a925f9856b1
src/mem/protocol/MOESI_hammer-cache.sm 3a925f9856b1
src/mem/protocol/RubySlicc_Types.sm 3a925f9856b1
src/mem/ruby/structures/CacheMemory.hh 3a925f9856b1
src/mem/ruby/structures/CacheMemory.cc 3a925f9856b1
src/mem/ruby/system/Sequencer.cc 3a925f9856b1

Diff: http://reviews.gem5.org/r/2957/diff/


Testing
-------


Thanks,

Nilay Vaish
Nilay Vaish
2015-07-19 23:45:04 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2957/
-----------------------------------------------------------

(Updated July 19, 2015, 11:45 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
-------

Changeset 10938:fa07a01fe745
---------------------------
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 (updated)
-----

src/mem/protocol/MESI_Three_Level-L0cache.sm 3a925f9856b1
src/mem/protocol/MESI_Two_Level-L1cache.sm 3a925f9856b1
src/mem/protocol/MI_example-cache.sm 3a925f9856b1
src/mem/protocol/MOESI_CMP_directory-L1cache.sm 3a925f9856b1
src/mem/protocol/MOESI_CMP_token-L1cache.sm 3a925f9856b1
src/mem/protocol/MOESI_hammer-cache.sm 3a925f9856b1
src/mem/protocol/RubySlicc_Types.sm 3a925f9856b1
src/mem/ruby/structures/CacheMemory.hh 3a925f9856b1
src/mem/ruby/structures/CacheMemory.cc 3a925f9856b1
src/mem/ruby/system/Sequencer.cc 3a925f9856b1

Diff: http://reviews.gem5.org/r/2957/diff/


Testing
-------


Thanks,

Nilay Vaish
Nilay Vaish
2015-07-19 23:45:51 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2957/
-----------------------------------------------------------

(Updated July 19, 2015, 11:45 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
-------

Changeset 10937:3724230bfee5
---------------------------
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 (updated)
-----

src/mem/ruby/system/Sequencer.cc 3a925f9856b1
src/mem/protocol/MOESI_hammer-cache.sm 3a925f9856b1
src/mem/protocol/RubySlicc_Types.sm 3a925f9856b1
src/mem/ruby/structures/CacheMemory.hh 3a925f9856b1
src/mem/ruby/structures/CacheMemory.cc 3a925f9856b1
src/mem/protocol/MESI_Three_Level-L0cache.sm 3a925f9856b1
src/mem/protocol/MESI_Two_Level-L1cache.sm 3a925f9856b1
src/mem/protocol/MI_example-cache.sm 3a925f9856b1
src/mem/protocol/MOESI_CMP_directory-L1cache.sm 3a925f9856b1
src/mem/protocol/MOESI_CMP_token-L1cache.sm 3a925f9856b1

Diff: http://reviews.gem5.org/r/2957/diff/


Testing
-------


Thanks,

Nilay Vaish
Brandon Potter
2015-07-20 16:10:50 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2957/#review6794
-----------------------------------------------------------


This might be a silly question but I was wondering:

Some of the protocols have getCacheEntry without a way to pass whether to setMRU or not; they default to false. What is the reason for these automatically being false? Since the sequencer cannot set the recently used status, how do these caches update their entry information?

- Brandon Potter
Post by Nilay Vaish
-----------------------------------------------------------
http://reviews.gem5.org/r/2957/
-----------------------------------------------------------
(Updated July 19, 2015, 11:45 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 10937:3724230bfee5
---------------------------
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/ruby/system/Sequencer.cc 3a925f9856b1
src/mem/protocol/MOESI_hammer-cache.sm 3a925f9856b1
src/mem/protocol/RubySlicc_Types.sm 3a925f9856b1
src/mem/ruby/structures/CacheMemory.hh 3a925f9856b1
src/mem/ruby/structures/CacheMemory.cc 3a925f9856b1
src/mem/protocol/MESI_Three_Level-L0cache.sm 3a925f9856b1
src/mem/protocol/MESI_Two_Level-L1cache.sm 3a925f9856b1
src/mem/protocol/MI_example-cache.sm 3a925f9856b1
src/mem/protocol/MOESI_CMP_directory-L1cache.sm 3a925f9856b1
src/mem/protocol/MOESI_CMP_token-L1cache.sm 3a925f9856b1
Diff: http://reviews.gem5.org/r/2957/diff/
Testing
-------
Thanks,
Nilay Vaish
Nilay Vaish
2015-07-20 17:17:33 UTC
Permalink
Post by Nilay Vaish
-----------------------------------------------------------
http://reviews.gem5.org/r/2957/#review6794
-----------------------------------------------------------
Some of the protocols have getCacheEntry without a way to pass whether
to setMRU or not; they default to false. What is the reason for these
automatically being false? Since the sequencer cannot set the recently
used status, how do these caches update their entry information?
I wanted to update 2957, but failed to do so. So don't look at it.
Instead look at 2981.

--
Nilay

Loading...