Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/13207

Change subject: mem-cache: Fix FALRU inCachesMask initialization
......................................................................

mem-cache: Fix FALRU inCachesMask initialization

Previously, if the minimum tracked size was smaller than the
cache size the inCachesMask would not be initialized, and this
would trigger an assertion on insertion.

In order to deal with this problem, and to avoid problems due to
the user forgetting to change the minimum tracked size along with
the cache size (i.e., using a command line parameter like l2_size),
the minimum tracked size has been made relative to the cache size.
Therefore the user now sets the number of tracked caches, and the
minimum size is calculated from it as (size >> (numTrackedCaches
- 1)).

As a side effect, numTrackedCaches is now a value greater than 0,
as there is always at least one tracked cache.

Change-Id: I587cf5e0191c4587d938e6ab6036ec1b32f37793
Signed-off-by: Daniel R. Carvalho <[email protected]>
---
M src/mem/cache/tags/Tags.py
M src/mem/cache/tags/fa_lru.cc
M src/mem/cache/tags/fa_lru.hh
3 files changed, 31 insertions(+), 24 deletions(-)



diff --git a/src/mem/cache/tags/Tags.py b/src/mem/cache/tags/Tags.py
index 781ac8e..ac847c5 100644
--- a/src/mem/cache/tags/Tags.py
+++ b/src/mem/cache/tags/Tags.py
@@ -94,8 +94,9 @@
     cxx_class = 'FALRU'
     cxx_header = "mem/cache/tags/fa_lru.hh"

- min_tracked_cache_size = Param.MemorySize("128kB", "Minimum cache size for"
-                                              " which we track statistics")
+ num_tracked_caches = Param.Int(1, "Number of caches for which we track " \
+                                   "statistics.")
+
 class SkewedAssoc(BaseSetAssoc):
     type = 'SkewedAssoc'
     cxx_header = "mem/cache/tags/skewed_assoc.hh"
diff --git a/src/mem/cache/tags/fa_lru.cc b/src/mem/cache/tags/fa_lru.cc
index 60998e2..da32d61 100644
--- a/src/mem/cache/tags/fa_lru.cc
+++ b/src/mem/cache/tags/fa_lru.cc
@@ -56,9 +56,7 @@
 #include "mem/cache/base.hh"

 FALRU::FALRU(const Params *p)
-    : BaseTags(p),
-
-      cacheTracking(p->min_tracked_cache_size, size, blkSize)
+    : BaseTags(p), cacheTracking(p->num_tracked_caches, size, blkSize)
 {
     if (!isPowerOf2(blkSize))
         fatal("cache block size (in bytes) `%d' must be a power of two",
@@ -313,8 +311,7 @@
 void
 FALRU::CacheTracking::init(FALRUBlk *head, FALRUBlk *tail)
 {
-    // early exit if we are not tracking any extra caches
-    FALRUBlk* blk = numTrackedCaches ? head : nullptr;
+    FALRUBlk* blk = head;
     unsigned curr_size = 0;
     unsigned tracked_cache_size = minTrackedSize;
     CachesMask in_caches_mask = inAllCachesMask;
@@ -344,7 +341,7 @@
     // before moving it to the head
     CachesMask update_caches_mask = inAllCachesMask ^ blk->inCachesMask;

-    for (int i = 0; i < numTrackedCaches; i++) {
+    for (int i = 0; i < numTrackedCaches - 1; i++) {
         CachesMask current_cache_mask = 1U << i;
         if (current_cache_mask & update_caches_mask) {
             // if the ith cache didn't fit the block (before it is moved to
@@ -368,7 +365,7 @@
 {
     CachesMask update_caches_mask = blk->inCachesMask;

-    for (int i = 0; i < numTrackedCaches; i++) {
+    for (int i = 0; i < numTrackedCaches - 1; i++) {
         CachesMask current_cache_mask = 1U << i;
         if (current_cache_mask & update_caches_mask) {
             // if the ith cache fitted the block (before it is moved to
@@ -391,7 +388,7 @@
 void
 FALRU::CacheTracking::recordAccess(FALRUBlk *blk)
 {
-    for (int i = 0; i < numTrackedCaches; i++) {
+    for (int i = 0; i < numTrackedCaches - 1; i++) {
         if (blk && ((1U << i) & blk->inCachesMask)) {
             hits[i]++;
         } else {
@@ -401,9 +398,9 @@

     // Record stats for the actual cache too
     if (blk && blk->isValid()) {
-        hits[numTrackedCaches]++;
+        hits[numTrackedCaches - 1]++;
     } else {
-        misses[numTrackedCaches]++;
+        misses[numTrackedCaches - 1]++;
     }

     accesses++;
@@ -425,12 +422,12 @@
 FALRU::CacheTracking::regStats(std::string name)
 {
     hits
-        .init(numTrackedCaches + 1)
+        .init(numTrackedCaches)
         .name(name + ".falru_hits")
         .desc("The number of hits in each cache size.")
         ;
     misses
-        .init(numTrackedCaches + 1)
+        .init(numTrackedCaches)
         .name(name + ".falru_misses")
         .desc("The number of misses in each cache size.")
         ;
@@ -439,7 +436,7 @@
         .desc("The number of accesses to the FA LRU cache.")
         ;

-    for (unsigned i = 0; i < numTrackedCaches + 1; ++i) {
+    for (unsigned i = 0; i < numTrackedCaches; ++i) {
         std::stringstream size_str;
         printSize(size_str, minTrackedSize << i);
         hits.subname(i, size_str.str());
diff --git a/src/mem/cache/tags/fa_lru.hh b/src/mem/cache/tags/fa_lru.hh
index 876219b..e21ae8a 100644
--- a/src/mem/cache/tags/fa_lru.hh
+++ b/src/mem/cache/tags/fa_lru.hh
@@ -263,15 +263,24 @@
     class CacheTracking
     {
       public:
-        CacheTracking(unsigned min_size, unsigned max_size,
+        /**
+         * Default constructor.
+         *
+         * @param num_tracked_caches Number of tracked caches.
+         * @param size Max cache size for which tracking is applied.
+         * @param block_size Block size in bytes.
+         */
+        CacheTracking(unsigned num_tracked_caches, unsigned size,
                       unsigned block_size)
-            : blkSize(block_size),
-              minTrackedSize(min_size),
-              numTrackedCaches(max_size > min_size ?
- floorLog2(max_size) - floorLog2(min_size) : 0),
-              inAllCachesMask(mask(numTrackedCaches)),
-              boundaries(new FALRUBlk *[numTrackedCaches])
+            : blkSize(block_size), numTrackedCaches(num_tracked_caches),
+              minTrackedSize(size >> (numTrackedCaches - 1)),
+              inAllCachesMask(mask(numTrackedCaches - 1)),
+              boundaries(new FALRUBlk *[numTrackedCaches - 1])
         {
+            fatal_if(minTrackedSize <= 0,
+ "Minimum tracked cache size must be greater than zero.");
+            fatal_if(numTrackedCaches <= 0,
+                     "There must be at least one tracked cache.");
             fatal_if(numTrackedCaches > sizeof(CachesMask) * 8,
"Not enough bits (%s) in type CachesMask type to keep "
                      "track of %d caches\n", sizeof(CachesMask),
@@ -347,10 +356,10 @@
       private:
         /** The size of the cache block */
         const unsigned blkSize;
-        /** The smallest cache we are tracking */
-        const unsigned minTrackedSize;
         /** The number of different size caches being tracked. */
         const int numTrackedCaches;
+        /** The smallest cache we are tracking */
+        const unsigned minTrackedSize;
         /** A mask for all cache being tracked. */
         const CachesMask inAllCachesMask;
         /** Array of pointers to blocks at the cache boundaries. */

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/13207
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: I587cf5e0191c4587d938e6ab6036ec1b32f37793
Gerrit-Change-Number: 13207
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Carvalho <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to