mem-cache: Move insertBlock functionality in FALRU

Block insertion is being done in the getCandidates function, while the
insertBlock function does not do anything.

Besides, BaseTags' stats weren't being updated.

Change-Id: Iadab9c1ea61519214f66fa24c4b91c4fc95604c0
Reviewed-on: https://gem5-review.googlesource.com/8882
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Maintainer: Nikos Nikoleris <nikos.nikoleris@arm.com>

diff --git a/src/mem/cache/tags/base.cc b/src/mem/cache/tags/base.cc
index d2d6e8eef389369bb3d1597a0b33feaa74d031e0..d467019f6ac10f92bc2b042765731790e8555f0f 100644 (file)
--- a/src/mem/cache/tags/base.cc
+++ b/src/mem/cache/tags/base.cc
@@ -75,6 +75,54 @@ BaseTags::setCache(BaseCache *_cache)
     cache = _cache;
 }
 
+void
+BaseTags::insertBlock(PacketPtr pkt, CacheBlk *blk)
+{
+    // Get address
+    Addr addr = pkt->getAddr();
+
+    // Update warmup data
+    if (!blk->isTouched) {
+        if (!warmedUp && tagsInUse.value() >= warmupBound) {
+            warmedUp = true;
+            warmupCycle = curTick();
+        }
+    }
+
+    // If we're replacing a block that was previously valid update
+    // stats for it. This can't be done in findBlock() because a
+    // found block might not actually be replaced there if the
+    // coherence protocol says it can't be.
+    if (blk->isValid()) {
+        replacements[0]++;
+        totalRefs += blk->refCount;
+        ++sampledRefs;
+
+        invalidate(blk);
+        blk->invalidate();
+    }
+
+    // Previous block, if existed, has been removed, and now we have
+    // to insert the new one
+    tagsInUse++;
+
+    // Set tag for new block.  Caller is responsible for setting status.
+    blk->tag = extractTag(addr);
+
+    // Deal with what we are bringing in
+    MasterID master_id = pkt->req->masterId();
+    assert(master_id < cache->system->maxMasters());
+    occupancies[master_id]++;
+    blk->srcMasterId = master_id;
+
+    // Set task id
+    blk->task_id = pkt->req->taskId();
+
+    // We only need to write into one tag and one data block.
+    tagAccesses += 1;
+    dataAccesses += 1;
+}
+
 void
 BaseTags::regStats()
 {

diff --git a/src/mem/cache/tags/base.hh b/src/mem/cache/tags/base.hh
index 3370de284b898d418115c7b61308e0283ad59cee..4cf67747b85f8b23ea6bbb052f9dbcb64a037d38 100644 (file)
--- a/src/mem/cache/tags/base.hh
+++ b/src/mem/cache/tags/base.hh
@@ -265,7 +265,13 @@ class BaseTags : public ClockedObject
 
     virtual Addr extractTag(Addr addr) const = 0;
 
-    virtual void insertBlock(PacketPtr pkt, CacheBlk *blk) = 0;
+    /**
+     * Insert the new block into the cache and update stats.
+     *
+     * @param pkt Packet holding the address to update
+     * @param blk The block to update.
+     */
+    virtual void insertBlock(PacketPtr pkt, CacheBlk *blk);
 
     /**
      * Regenerate the block address.

diff --git a/src/mem/cache/tags/base_set_assoc.hh b/src/mem/cache/tags/base_set_assoc.hh
index 3bc275b281191036004ea3ad42d8f82b62e79fc9..ed9997608a91b910b053db2575355d1246c6ce41 100644 (file)
--- a/src/mem/cache/tags/base_set_assoc.hh
+++ b/src/mem/cache/tags/base_set_assoc.hh
@@ -212,55 +212,19 @@ class BaseSetAssoc : public BaseTags
     }
 
     /**
-     * Insert the new block into the cache.
+     * Insert the new block into the cache and update replacement data.
+     *
      * @param pkt Packet holding the address to update
      * @param blk The block to update.
      */
-     void insertBlock(PacketPtr pkt, CacheBlk *blk) override
-     {
-         Addr addr = pkt->getAddr();
-         MasterID master_id = pkt->req->masterId();
-         uint32_t task_id = pkt->req->taskId();
-
-         if (!blk->isTouched) {
-             if (!warmedUp && tagsInUse.value() >= warmupBound) {
-                 warmedUp = true;
-                 warmupCycle = curTick();
-             }
-         }
-
-         // If we're replacing a block that was previously valid update
-         // stats for it. This can't be done in findBlock() because a
-         // found block might not actually be replaced there if the
-         // coherence protocol says it can't be.
-         if (blk->isValid()) {
-             replacements[0]++;
-             totalRefs += blk->refCount;
-             ++sampledRefs;
-
-             invalidate(blk);
-             blk->invalidate();
-         }
-
-         // Previous block, if existed, has been removed, and now we have
-         // to insert the new one
-         tagsInUse++;
-
-         // Set tag for new block.  Caller is responsible for setting status.
-         blk->tag = extractTag(addr);
-
-         // deal with what we are bringing in
-         assert(master_id < cache->system->maxMasters());
-         occupancies[master_id]++;
-         blk->srcMasterId = master_id;
-         blk->task_id = task_id;
-
-         // We only need to write into one tag and one data block.
-         tagAccesses += 1;
-         dataAccesses += 1;
-
-         replacementPolicy->reset(blk);
-     }
+    void insertBlock(PacketPtr pkt, CacheBlk *blk) override
+    {
+        // Insert block
+        BaseTags::insertBlock(pkt, blk);
+
+        // Update replacement policy
+        replacementPolicy->reset(blk);
+    }
 
     /**
      * Limit the allocation for the cache ways.

diff --git a/src/mem/cache/tags/fa_lru.cc b/src/mem/cache/tags/fa_lru.cc
index 1f2909e31c3d0fdc632449d3665da3723ff73f0a..652abc36003494a0842af012c7a91988e3c58ca7 100644 (file)
--- a/src/mem/cache/tags/fa_lru.cc
+++ b/src/mem/cache/tags/fa_lru.cc
@@ -169,6 +169,9 @@ FALRU::invalidate(CacheBlk *blk)
 {
     // TODO: We need to move the block to the tail to make it the next victim
     BaseTags::invalidate(blk);
+
+    // Erase block entry in the hash table
+    tagHash.erase(blk->tag);
 }
 
 CacheBlk*
@@ -250,28 +253,27 @@ FALRU::findBlockBySetAndWay(int set, int way) const
 CacheBlk*
 FALRU::findVictim(Addr addr)
 {
-    FALRUBlk * blk = tail;
-    assert(blk->inCache == 0);
-    moveToHead(blk);
-    tagHash.erase(blk->tag);
-    tagHash[blkAlign(addr)] = blk;
-    if (blk->isValid()) {
-        replacements[0]++;
-    } else {
-        tagsInUse++;
-        if (!warmedUp && tagsInUse.value() >= warmupBound) {
-            warmedUp = true;
-            warmupCycle = curTick();
-        }
-    }
-    //assert(check());
-
-    return blk;
+    return tail;
 }
 
 void
 FALRU::insertBlock(PacketPtr pkt, CacheBlk *blk)
 {
+    FALRUBlk* falruBlk = static_cast<FALRUBlk*>(blk);
+
+    // Make sure block is not present in the cache
+    assert(falruBlk->inCache == 0);
+
+    // Do common block insertion functionality
+    BaseTags::insertBlock(pkt, blk);
+
+    // New block is the MRU
+    moveToHead(falruBlk);
+
+    // Insert new block in the hash table
+    tagHash[falruBlk->tag] = falruBlk;
+
+    //assert(check());
 }
 
 void

diff --git a/src/mem/cache/tags/fa_lru.hh b/src/mem/cache/tags/fa_lru.hh
index 7f3f99ae04030fe0bacb997b941542333f46b64d..129af9f3760570a2f4f1fb1852b2a0ca0a2ec23d 100644 (file)
--- a/src/mem/cache/tags/fa_lru.hh
+++ b/src/mem/cache/tags/fa_lru.hh
@@ -62,7 +62,7 @@
  */
 class FALRUBlk : public CacheBlk
 {
-public:
+  public:
     /** The previous block in LRU order. */
     FALRUBlk *prev;
     /** The next block in LRU order. */
@@ -151,8 +151,7 @@ class FALRU : public BaseTags
      * @}
      */
 
-public:
-
+  public:
     typedef FALRUParams Params;
 
     /**
@@ -209,6 +208,12 @@ public:
      */
     CacheBlk* findVictim(Addr addr) override;
 
+    /**
+     * Insert the new block into the cache and update replacement data.
+     *
+     * @param pkt Packet holding the address to update
+     * @param blk The block to update.
+     */
     void insertBlock(PacketPtr pkt, CacheBlk *blk) override;
 
     /**
@@ -274,7 +279,6 @@ public:
                 return;
         }
     }
-
 };
 
 #endif // __MEM_CACHE_TAGS_FA_LRU_HH__