Nikos Nikoleris has uploaded this change for review. ( https://gem5-review.googlesource.com/10423

Change subject: mem-cache: Refactor the cache recvTimingResp function
......................................................................

mem-cache: Refactor the cache recvTimingResp function

The recvTimingResp function in the cache handles timing
responses. Over time, recvTimingResp has grown in complexity and code
size. This change factors out some of its functionality to a separate
function. The new function iterates through the in-service targets and
handles them accordingly.

Change-Id: I0ef28288640f6be1b30452b0664d32432e692ea6
---
M src/mem/cache/cache.cc
M src/mem/cache/cache.hh
2 files changed, 115 insertions(+), 96 deletions(-)



diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index b9625be..c0d4401 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -1322,100 +1322,20 @@
 }

 void
-Cache::recvTimingResp(PacketPtr pkt)
+Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk,
+                          PacketList &writebacks)
 {
-    assert(pkt->isResponse());
-
-    // all header delay should be paid for by the crossbar, unless
-    // this is a prefetch response from above
-    panic_if(pkt->headerDelay != 0 && pkt->cmd != MemCmd::HardPFResp,
-             "%s saw a non-zero packet delay\n", name());
-
-    bool is_error = pkt->isError();
-
-    if (is_error) {
-        DPRINTF(Cache, "%s: Cache received %s with error\n", __func__,
-                pkt->print());
-    }
-
-    DPRINTF(Cache, "%s: Handling response %s\n", __func__,
-            pkt->print());
-
-    // if this is a write, we should be looking at an uncacheable
-    // write
-    if (pkt->isWrite()) {
-        assert(pkt->req->isUncacheable());
-        handleUncacheableWriteResp(pkt);
-        return;
-    }
-
-    // we have dealt with any (uncacheable) writes above, from here on
-    // we know we are dealing with an MSHR due to a miss or a prefetch
-    MSHR *mshr = dynamic_cast<MSHR*>(pkt->popSenderState());
-    assert(mshr);
-
-    if (mshr == noTargetMSHR) {
-        // we always clear at least one target
-        clearBlocked(Blocked_NoTargets);
-        noTargetMSHR = nullptr;
-    }
-
-    // Initial target is used just for stats
     MSHR::Target *initial_tgt = mshr->getTarget();
-    int stats_cmd_idx = initial_tgt->pkt->cmdToIndex();
-    Tick miss_latency = curTick() - initial_tgt->recvTime;
+    // First offset for critical word first calculations
+    int initial_offset = initial_tgt->pkt->getOffset(blkSize);

-    if (pkt->req->isUncacheable()) {
-        assert(pkt->req->masterId() < system->maxMasters());
-        mshr_uncacheable_lat[stats_cmd_idx][pkt->req->masterId()] +=
-            miss_latency;
-    } else {
-        assert(pkt->req->masterId() < system->maxMasters());
-        mshr_miss_latency[stats_cmd_idx][pkt->req->masterId()] +=
-            miss_latency;
-    }
-
-    bool wasFull = mshrQueue.isFull();
-
-    PacketList writebacks;
-
-    Tick forward_time = clockEdge(forwardLatency) + pkt->headerDelay;
-
+    const bool is_error = pkt->isError();
     bool is_fill = !mshr->isForward &&
         (pkt->isRead() || pkt->cmd == MemCmd::UpgradeResp);
-
-    CacheBlk *blk = tags->findBlock(pkt->getAddr(), pkt->isSecure());
-    const bool valid_blk = blk && blk->isValid();
-    // If the response indicates that there are no sharers and we
-    // either had the block already or the response is filling we can
-    // promote our copy to writable
-    if (!pkt->hasSharers() &&
-        (is_fill || (valid_blk && !pkt->req->isCacheInvalidate()))) {
-        mshr->promoteWritable();
-    }
-
-    if (is_fill && !is_error) {
-        DPRINTF(Cache, "Block for addr %#llx being updated in Cache\n",
-                pkt->getAddr());
-
-        blk = handleFill(pkt, blk, writebacks, mshr->allocOnFill());
-        assert(blk != nullptr);
-    }
-
     // allow invalidation responses originating from write-line
     // requests to be discarded
     bool is_invalidate = pkt->isInvalidate();

-    // The block was marked as not readable while there was a pending
-    // cache maintenance operation, restore its flag.
-    if (pkt->isClean() && !is_invalidate && valid_blk) {
-        blk->status |= BlkReadable;
-    }
-
-    // First offset for critical word first calculations
-    int initial_offset = initial_tgt->pkt->getOffset(blkSize);
-
-    bool from_cache = false;
     MSHR::TargetList targets = mshr->extractServiceableTargets(pkt);
     for (auto &target: targets) {
         Packet *tgt_pkt = target.pkt;
@@ -1437,10 +1357,6 @@
                 break; // skip response
             }

-            // keep track of whether we have responded to another
-            // cache
-            from_cache = from_cache || tgt_pkt->fromCache();
-
             // unlike the other packet flows, where data is found in other
// caches or memory and brought back, write-line requests always
             // have the data right away, so the above check for "is fill?"
@@ -1455,7 +1371,7 @@
// NB: we use the original packet here and not the response!
                 blk = handleFill(tgt_pkt, blk, writebacks,
                                  targets.allocOnFill);
-                assert(blk != nullptr);
+                assert(blk);

                 // treat as a fill, and discard the invalidation
                 // response
@@ -1572,8 +1488,6 @@
         }
     }

-    maintainClusivity(from_cache, blk);
-
     if (blk && blk->isValid()) {
         // an invalidate response stemming from a write line request
         // should not invalidate the block, so check if the
@@ -1584,6 +1498,93 @@
             blk->status &= ~BlkWritable;
         }
     }
+}
+
+void
+Cache::recvTimingResp(PacketPtr pkt)
+{
+    assert(pkt->isResponse());
+
+    // all header delay should be paid for by the crossbar, unless
+    // this is a prefetch response from above
+    panic_if(pkt->headerDelay != 0 && pkt->cmd != MemCmd::HardPFResp,
+             "%s saw a non-zero packet delay\n", name());
+
+    const bool is_error = pkt->isError();
+
+    if (is_error) {
+        DPRINTF(Cache, "%s: Cache received %s with error\n", __func__,
+                pkt->print());
+    }
+
+    DPRINTF(Cache, "%s: Handling response %s\n", __func__,
+            pkt->print());
+
+    // if this is a write, we should be looking at an uncacheable
+    // write
+    if (pkt->isWrite()) {
+        assert(pkt->req->isUncacheable());
+        handleUncacheableWriteResp(pkt);
+        return;
+    }
+
+    // we have dealt with any (uncacheable) writes above, from here on
+    // we know we are dealing with an MSHR due to a miss or a prefetch
+    MSHR *mshr = dynamic_cast<MSHR*>(pkt->popSenderState());
+    assert(mshr);
+
+    if (mshr == noTargetMSHR) {
+        // we always clear at least one target
+        clearBlocked(Blocked_NoTargets);
+        noTargetMSHR = nullptr;
+    }
+
+    // Initial target is used just for stats
+    MSHR::Target *initial_tgt = mshr->getTarget();
+    int stats_cmd_idx = initial_tgt->pkt->cmdToIndex();
+    Tick miss_latency = curTick() - initial_tgt->recvTime;
+
+    if (pkt->req->isUncacheable()) {
+        assert(pkt->req->masterId() < system->maxMasters());
+        mshr_uncacheable_lat[stats_cmd_idx][pkt->req->masterId()] +=
+            miss_latency;
+    } else {
+        assert(pkt->req->masterId() < system->maxMasters());
+        mshr_miss_latency[stats_cmd_idx][pkt->req->masterId()] +=
+            miss_latency;
+    }
+
+    PacketList writebacks;
+
+    bool is_fill = !mshr->isForward &&
+        (pkt->isRead() || pkt->cmd == MemCmd::UpgradeResp);
+
+    CacheBlk *blk = tags->findBlock(pkt->getAddr(), pkt->isSecure());
+
+    if (is_fill && !is_error) {
+        DPRINTF(Cache, "Block for addr %#llx being updated in Cache\n",
+                pkt->getAddr());
+
+        blk = handleFill(pkt, blk, writebacks, mshr->allocOnFill());
+        assert(blk != nullptr);
+    }
+
+    if (blk && blk->isValid() && pkt->isClean() && !pkt->isInvalidate()) {
+        // The block was marked not readable while there was a pending
+        // cache maintenance operation, restore its flag.
+        blk->status |= BlkReadable;
+    }
+
+    if (blk && blk->isWritable() && !pkt->req->isCacheInvalidate()) {
+        // If at this point the referenced block is writable and the
+        // response is not a cache invalidate, we promote targets that
+        // were deferred as we couldn't guarrantee a writable copy
+        mshr->promoteWritable();
+    }
+
+    serviceMSHRTargets(mshr, pkt, blk, writebacks);
+
+    maintainClusivity(mshr->hasFromCache(), blk);

     if (mshr->promoteDeferredTargets()) {
         // avoid later read getting stale data while write miss is
@@ -1594,8 +1595,12 @@
         mshrQueue.markPending(mshr);
         schedMemSideSendEvent(clockEdge() + pkt->payloadDelay);
     } else {
+        // while we deallocate an mshr from the queue we still have to
+        // check the isFull condition before and after as we might
+        // have been using the reserved entries already
+        const bool was_full = mshrQueue.isFull();
         mshrQueue.deallocate(mshr);
-        if (wasFull && !mshrQueue.isFull()) {
+        if (was_full && !mshrQueue.isFull()) {
             clearBlocked(Blocked_NoMSHRs);
         }

@@ -1608,8 +1613,6 @@
                 schedMemSideSendEvent(next_pf_time);
         }
     }
-    // reset the xbar additional timinig  as it is now accounted for
-    pkt->headerDelay = pkt->payloadDelay = 0;

// if we used temp block, check to see if its valid and then clear it out
     if (blk == tempBlock && tempBlock->isValid()) {
@@ -1619,6 +1622,7 @@
         invalidateBlock(tempBlock);
     }

+    const Tick forward_time = clockEdge(forwardLatency) + pkt->headerDelay;
     // copy writebacks to write buffer
     doWritebacks(writebacks, forward_time);

diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh
index 7d28279..b01be39 100644
--- a/src/mem/cache/cache.hh
+++ b/src/mem/cache/cache.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2017 ARM Limited
+ * Copyright (c) 2012-2018 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -370,6 +370,21 @@
     void handleUncacheableWriteResp(PacketPtr pkt);

     /**
+     * Given a response service the non-deferred MSHR targets
+     *
+     * Iterates through the list of targets that can be serviced with
+     * the current response. Any writebacks that need to performed
+     * must be appended to the writebacks parameter.
+     *
+     * @param mshr The MSHR that corresponds to the reponse
+     * @param pkt The response packet
+     * @param blk The reference block
+     * @param writebacks List of writebacks that need to be performed
+     */
+    void serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk,
+                            PacketList& writebacks);
+
+    /**
      * Handles a response (cache line fill/write ack) from the bus.
      * @param pkt The response packet
      */

--
To view, visit https://gem5-review.googlesource.com/10423
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: I0ef28288640f6be1b30452b0664d32432e692ea6
Gerrit-Change-Number: 10423
Gerrit-PatchSet: 1
Gerrit-Owner: Nikos Nikoleris <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to