Daniel Carvalho has submitted this change and it was merged. ( https://gem5-review.googlesource.com/c/public/gem5/+/12764 )

Change subject: mem-cache: Use set and way for ReplaceableEntry
......................................................................

mem-cache: Use set and way for ReplaceableEntry

Replaceable entries belong to table-like structures, and therefore
they should be indexable by combining a row and a column. These,
using conventional cache nomenclature translate to sets and ways.

Make these entries aware of their sets and ways. The idea is to
make indexing policies usable by other table-like structures. In
order to do so we move sets and ways to ReplaceableEntry, which
will be the common base among table entries.

Change-Id: If0e3dacf9ea2f523af9cface067469ccecf82648
Reviewed-on: https://gem5-review.googlesource.com/c/12764
Reviewed-by: Jason Lowe-Power <[email protected]>
Maintainer: Jason Lowe-Power <[email protected]>
---
M src/mem/cache/blk.hh
M src/mem/cache/replacement_policies/base.hh
M src/mem/cache/tags/base.cc
M src/mem/cache/tags/base_set_assoc.cc
M src/mem/cache/tags/base_set_assoc.hh
M src/mem/cache/tags/fa_lru.cc
M src/mem/cache/tags/sector_tags.cc
M src/mem/cache/tags/skewed_assoc.cc
8 files changed, 61 insertions(+), 31 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved



diff --git a/src/mem/cache/blk.hh b/src/mem/cache/blk.hh
index 3bb0317..cc6d905 100644
--- a/src/mem/cache/blk.hh
+++ b/src/mem/cache/blk.hh
@@ -108,12 +108,6 @@
     /** Which curTick() will this block be accessible */
     Tick whenReady;

-    /**
-     * The set and way this block belongs to.
-     * @todo Move this into subclasses when we fix CacheTags to use them.
-     */
-    int set, way;
-
     /** Number of references to this block since it was brought in. */
     unsigned refCount;

diff --git a/src/mem/cache/replacement_policies/base.hh b/src/mem/cache/replacement_policies/base.hh
index 0ce86d0..6ac7dca 100644
--- a/src/mem/cache/replacement_policies/base.hh
+++ b/src/mem/cache/replacement_policies/base.hh
@@ -50,12 +50,48 @@
  */
 class ReplaceableEntry
 {
-  public:
+  private:
     /**
-     * Replacement data associated to this entry.
-     * It is instantiated by the replacement policy.
+     * Set to which this entry belongs.
      */
-    std::shared_ptr<ReplacementData> replacementData;
+    uint32_t _set;
+
+    /**
+     * Way (relative position within the set) to which this entry belongs.
+     */
+    uint32_t _way;
+
+   public:
+     /**
+      * Replacement data associated to this entry.
+      * It is instantiated by the replacement policy.
+      */
+     std::shared_ptr<ReplacementData> replacementData;
+
+    /**
+     * Set both the set and way. Should be called only once.
+     *
+     * @param set The set of this entry.
+     * @param way The way of this entry.
+     */
+    void setPosition(const uint32_t set, const uint32_t way) {
+        _set = set;
+        _way = way;
+    }
+
+    /**
+     * Get set number.
+     *
+     * @return The set to which this entry belongs.
+     */
+    uint32_t getSet() const { return _set; }
+
+    /**
+     * Get way number.
+     *
+     * @return The way to which this entry belongs.
+     */
+    uint32_t getWay() const { return _way; }
 };

 /**
diff --git a/src/mem/cache/tags/base.cc b/src/mem/cache/tags/base.cc
index 9358652..303eb04 100644
--- a/src/mem/cache/tags/base.cc
+++ b/src/mem/cache/tags/base.cc
@@ -194,8 +194,8 @@

     auto print_blk = [&str](CacheBlk &blk) {
         if (blk.isValid())
-            str += csprintf("\tset: %d way: %d %s\n", blk.set, blk.way,
-                            blk.print());
+            str += csprintf("\tset: %x, way: %x %s\n", blk.getSet(),
+                            blk.getWay(), blk.print());
     };
     forEachBlk(print_blk);

diff --git a/src/mem/cache/tags/base_set_assoc.cc b/src/mem/cache/tags/base_set_assoc.cc
index 05712ec..543231c 100644
--- a/src/mem/cache/tags/base_set_assoc.cc
+++ b/src/mem/cache/tags/base_set_assoc.cc
@@ -104,9 +104,8 @@
             // hash table; won't matter because the block is invalid
             blk->tag = j;

-            // Set its set and way
-            blk->set = i;
-            blk->way = j;
+            // Set its index
+            blk->setPosition(i, j);

             // Update block index
             ++blkIndex;
diff --git a/src/mem/cache/tags/base_set_assoc.hh b/src/mem/cache/tags/base_set_assoc.hh
index 26cc6bc..2f4f730 100644
--- a/src/mem/cache/tags/base_set_assoc.hh
+++ b/src/mem/cache/tags/base_set_assoc.hh
@@ -237,7 +237,7 @@
         evict_blks.push_back(victim);

DPRINTF(CacheRepl, "set %x, way %x: selecting blk for replacement\n",
-            victim->set, victim->way);
+                victim->getSet(), victim->getWay());

         return victim;
     }
@@ -302,7 +302,8 @@
      */
     Addr regenerateBlkAddr(const CacheBlk* blk) const override
     {
-        return ((blk->tag << tagShift) | ((Addr)blk->set << setShift));
+        const Addr set = blk->getSet() << setShift;
+        return ((blk->tag << tagShift) | set);
     }

     void forEachBlk(std::function<void(CacheBlk &)> visitor) override {
diff --git a/src/mem/cache/tags/fa_lru.cc b/src/mem/cache/tags/fa_lru.cc
index d35e37c..fa7c659 100644
--- a/src/mem/cache/tags/fa_lru.cc
+++ b/src/mem/cache/tags/fa_lru.cc
@@ -85,15 +85,13 @@
     head = &(blks[0]);
     head->prev = nullptr;
     head->next = &(blks[1]);
-    head->set = 0;
-    head->way = 0;
+    head->setPosition(0, 0);
     head->data = &dataBlks[0];

     for (unsigned i = 1; i < numBlocks - 1; i++) {
         blks[i].prev = &(blks[i-1]);
         blks[i].next = &(blks[i+1]);
-        blks[i].set = 0;
-        blks[i].way = i;
+        blks[i].setPosition(0, i);

         // Associate a data chunk to the block
         blks[i].data = &dataBlks[blkSize*i];
@@ -102,8 +100,7 @@
     tail = &(blks[numBlocks - 1]);
     tail->prev = &(blks[numBlocks - 2]);
     tail->next = nullptr;
-    tail->set = 0;
-    tail->way = numBlocks - 1;
+    tail->setPosition(0, numBlocks - 1);
     tail->data = &dataBlks[(numBlocks - 1) * blkSize];

     cacheTracking.init(head, tail);
diff --git a/src/mem/cache/tags/sector_tags.cc b/src/mem/cache/tags/sector_tags.cc
index ef3fec2..90b31b6 100644
--- a/src/mem/cache/tags/sector_tags.cc
+++ b/src/mem/cache/tags/sector_tags.cc
@@ -92,6 +92,9 @@
             // Associate a replacement data entry to the sector
sec_blk->replacementData = replacementPolicy->instantiateEntry();

+            // Set its index
+            sec_blk->setPosition(i, j);
+
             // Initialize all blocks in this sector
             sec_blk->blks.resize(numBlocksPerSector);
             for (unsigned k = 0; k < numBlocksPerSector; ++k){
@@ -110,9 +113,8 @@
                 // Associate the sector replacement data to this block
                 blk->replacementData = sec_blk->replacementData;

-                // Set its set, way and sector offset
-                blk->set = i;
-                blk->way = j;
+                // Set its index and sector offset
+                blk->setPosition(i, j);
                 blk->setSectorOffset(k);

                 // Update block index
@@ -308,8 +310,8 @@
         }
     }

-    DPRINTF(CacheRepl, "set %x, way %x, sector offset %x: %s\n",
-            "selecting blk for replacement\n", victim->set, victim->way,
+    DPRINTF(CacheRepl, "set %x, way %x, sector offset %x: selecting blk " \
+            "for replacement\n", victim->getSet(), victim->getWay(),
             victim->getSectorOffset());

     return victim;
@@ -337,7 +339,8 @@
 SectorTags::regenerateBlkAddr(const CacheBlk* blk) const
 {
     const SectorSubBlk* blk_cast = static_cast<const SectorSubBlk*>(blk);
- return ((blk_cast->getTag() << tagShift) | ((Addr)blk->set << setShift) |
+    const Addr set = blk_cast->getSectorBlock()->getSet() << setShift;
+    return ((blk_cast->getTag() << tagShift) | set |
             ((Addr)blk_cast->getSectorOffset() << sectorShift));
 }

diff --git a/src/mem/cache/tags/skewed_assoc.cc b/src/mem/cache/tags/skewed_assoc.cc
index 89ab615..d92ab46 100644
--- a/src/mem/cache/tags/skewed_assoc.cc
+++ b/src/mem/cache/tags/skewed_assoc.cc
@@ -199,8 +199,8 @@
 Addr
 SkewedAssoc::regenerateBlkAddr(const CacheBlk* blk) const
 {
-    const Addr addr = (blk->tag << (msbShift + 1)) | blk->set;
-    const Addr set = deskew(addr, blk->way) & setMask;
+    const Addr addr = (blk->tag << (msbShift + 1)) | blk->getSet();
+    const Addr set = deskew(addr, blk->getWay()) & setMask;
     return (blk->tag << tagShift) | (set << setShift);
 }


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/12764
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: If0e3dacf9ea2f523af9cface067469ccecf82648
Gerrit-Change-Number: 12764
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Nikos Nikoleris <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to