Daniel Carvalho has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/23945 )

Change subject: mem-ruby: Cleanup replacement_data usage
......................................................................

mem-ruby: Cleanup replacement_data usage

The replacement_data can be assigned as soon as a block is allocated.
With this cleanup the lookup function can be used to avoid code
duplication.

Change-Id: I7561fddaa3ed348866699ecaf1e6aa477ba0bc9a
Signed-off-by: Daniel R. Carvalho <oda...@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23945
Reviewed-by: John Alsop <johnathan.al...@amd.com>
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Maintainer: Jason Lowe-Power <power...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/mem/ruby/structures/CacheMemory.cc
M src/mem/ruby/structures/CacheMemory.hh
2 files changed, 29 insertions(+), 37 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  John Alsop: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/mem/ruby/structures/CacheMemory.cc b/src/mem/ruby/structures/CacheMemory.cc
index b734308..08004ac 100644
--- a/src/mem/ruby/structures/CacheMemory.cc
+++ b/src/mem/ruby/structures/CacheMemory.cc
@@ -180,7 +180,7 @@
     if (loc != -1) {
         // Do we even have a tag match?
         AbstractCacheEntry* entry = m_cache[cacheSet][loc];
-        m_replacementPolicy_ptr->touch(replacement_data[cacheSet][loc]);
+        m_replacementPolicy_ptr->touch(entry->replacementData);
         m_cache[cacheSet][loc]->setLastAccess(curTick());
         data_ptr = &(entry->getDataBlk());

@@ -209,7 +209,7 @@
     if (loc != -1) {
         // Do we even have a tag match?
         AbstractCacheEntry* entry = m_cache[cacheSet][loc];
-        m_replacementPolicy_ptr->touch(replacement_data[cacheSet][loc]);
+        m_replacementPolicy_ptr->touch(entry->replacementData);
         m_cache[cacheSet][loc]->setLastAccess(curTick());
         data_ptr = &(entry->getDataBlk());

@@ -291,10 +291,13 @@
             set[i]->m_locked = -1;
             m_tag_index[address] = i;
             set[i]->setPosition(cacheSet, i);
+            set[i]->replacementData = replacement_data[cacheSet][i];
+            set[i]->setLastAccess(curTick());
+
             // Call reset function here to set initial value for different
             // replacement policies.
-            m_replacementPolicy_ptr->reset(replacement_data[cacheSet][i]);
-            set[i]->setLastAccess(curTick());
+            m_replacementPolicy_ptr->reset(entry->replacementData);
+
             return entry;
         }
     }
@@ -304,17 +307,15 @@
 void
 CacheMemory::deallocate(Addr address)
 {
-    assert(address == makeLineAddress(address));
-    assert(isTagPresent(address));
     DPRINTF(RubyCache, "address: %#x\n", address);
-    int64_t cacheSet = addressToCacheSet(address);
-    int loc = findTagInSet(cacheSet, address);
-    if (loc != -1) {
- m_replacementPolicy_ptr->invalidate(replacement_data[cacheSet][loc]);
-        delete m_cache[cacheSet][loc];
-        m_cache[cacheSet][loc] = NULL;
-        m_tag_index.erase(address);
-    }
+    AbstractCacheEntry* entry = lookup(address);
+    assert(entry != nullptr);
+    m_replacementPolicy_ptr->invalidate(entry->replacementData);
+    uint32_t cache_set = entry->getSet();
+    uint32_t way = entry->getWay();
+    delete entry;
+    m_cache[cache_set][way] = NULL;
+    m_tag_index.erase(address);
 }

 // Returns with the physical address of the conflicting cache line
@@ -327,9 +328,6 @@
     int64_t cacheSet = addressToCacheSet(address);
     std::vector<ReplaceableEntry*> candidates;
     for (int i = 0; i < m_cache_assoc; i++) {
-        // Pass the value of replacement_data to the cache entry so that we
-        // can use it in the getVictim() function.
- m_cache[cacheSet][i]->replacementData = replacement_data[cacheSet][i];
         candidates.push_back(static_cast<ReplaceableEntry*>(
m_cache[cacheSet][i]));
     }
@@ -363,42 +361,36 @@
 void
 CacheMemory::setMRU(Addr address)
 {
-    int64_t cacheSet = addressToCacheSet(address);
-    int loc = findTagInSet(cacheSet, address);
-
-    if (loc != -1) {
-        m_replacementPolicy_ptr->touch(replacement_data[cacheSet][loc]);
-        m_cache[cacheSet][loc]->setLastAccess(curTick());
+    AbstractCacheEntry* entry = lookup(makeLineAddress(address));
+    if (entry != nullptr) {
+        m_replacementPolicy_ptr->touch(entry->replacementData);
+        entry->setLastAccess(curTick());
     }
 }

 void
-CacheMemory::setMRU(const AbstractCacheEntry *e)
+CacheMemory::setMRU(AbstractCacheEntry *entry)
 {
-    uint32_t cacheSet = e->getSet();
-    uint32_t loc = e->getWay();
-    m_replacementPolicy_ptr->touch(replacement_data[cacheSet][loc]);
-    m_cache[cacheSet][loc]->setLastAccess(curTick());
+    assert(entry != nullptr);
+    m_replacementPolicy_ptr->touch(entry->replacementData);
+    entry->setLastAccess(curTick());
 }

 void
 CacheMemory::setMRU(Addr address, int occupancy)
 {
-    int64_t cacheSet = addressToCacheSet(address);
-    int loc = findTagInSet(cacheSet, address);
-
-    if (loc != -1) {
+    AbstractCacheEntry* entry = lookup(makeLineAddress(address));
+    if (entry != nullptr) {
         // m_use_occupancy can decide whether we are using WeightedLRU
         // replacement policy. Depending on different replacement policies,
         // use different touch() function.
         if (m_use_occupancy) {
static_cast<WeightedLRUPolicy*>(m_replacementPolicy_ptr)->touch(
-                replacement_data[cacheSet][loc], occupancy);
+                entry->replacementData, occupancy);
         } else {
-            m_replacementPolicy_ptr->
-                touch(replacement_data[cacheSet][loc]);
+            m_replacementPolicy_ptr->touch(entry->replacementData);
         }
-        m_cache[cacheSet][loc]->setLastAccess(curTick());
+        entry->setLastAccess(curTick());
     }
 }

diff --git a/src/mem/ruby/structures/CacheMemory.hh b/src/mem/ruby/structures/CacheMemory.hh
index fc2c2c8..7d82d5f 100644
--- a/src/mem/ruby/structures/CacheMemory.hh
+++ b/src/mem/ruby/structures/CacheMemory.hh
@@ -111,8 +111,8 @@
     // Set this address to most recently used
     void setMRU(Addr address);
     void setMRU(Addr addr, int occupancy);
+    void setMRU(AbstractCacheEntry* entry);
     int getReplacementWeight(int64_t set, int64_t loc);
-    void setMRU(const AbstractCacheEntry *e);

     // Functions for locking and unlocking cache lines corresponding to the
     // provided address.  These are required for supporting atomic memory

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

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I7561fddaa3ed348866699ecaf1e6aa477ba0bc9a
Gerrit-Change-Number: 23945
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Bradford Beckmann <brad.beckm...@amd.com>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: John Alsop <johnathan.al...@amd.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to