Cache: Refactor packet forwarding a bit.

Makes adding write-through operations easier.

3 files changed, 56 insertions, 34 deletions

diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index 40d1c9c2f..c4a19ad5c 100644
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -613,38 +613,53 @@ Cache<TagStore>::atomicAccess(PacketPtr pkt)
 
     if (!access(pkt, blk, lat, writebacks)) {
         // MISS
-        PacketPtr busPkt = getBusPacket(pkt, blk, pkt->needsExclusive());
+        PacketPtr bus_pkt = getBusPacket(pkt, blk, pkt->needsExclusive());
 
-        bool isCacheFill = (busPkt != NULL);
+        bool is_forward = (bus_pkt == NULL);
 
-        if (busPkt == NULL) {
+        if (is_forward) {
             // just forwarding the same request to the next level
             // no local cache operation involved
-            busPkt = pkt;
+            bus_pkt = pkt;
         }
 
         DPRINTF(Cache, "Sending an atomic %s for %x\n",
-                busPkt->cmdString(), busPkt->getAddr());
+                bus_pkt->cmdString(), bus_pkt->getAddr());
 
 #if TRACING_ON
         CacheBlk::State old_state = blk ? blk->status : 0;
 #endif
 
-        lat += memSidePort->sendAtomic(busPkt);
+        lat += memSidePort->sendAtomic(bus_pkt);
 
         DPRINTF(Cache, "Receive response: %s for addr %x in state %i\n",
-                busPkt->cmdString(), busPkt->getAddr(), old_state);
-
-        bool is_error = busPkt->isError();
-        assert(!busPkt->wasNacked());
-
-        if (is_error && pkt->needsResponse()) {
-            pkt->makeAtomicResponse();
-            pkt->copyError(busPkt);
-        } else if (isCacheFill && !is_error) {
-            blk = handleFill(busPkt, blk, writebacks);
-            satisfyCpuSideRequest(pkt, blk);
-            delete busPkt;
+                bus_pkt->cmdString(), bus_pkt->getAddr(), old_state);
+
+        assert(!bus_pkt->wasNacked());
+
+        // If packet was a forward, the response (if any) is already
+        // in place in the bus_pkt == pkt structure, so we don't need
+        // to do anything.  Otherwise, use the separate bus_pkt to
+        // generate response to pkt and then delete it.
+        if (!is_forward) {
+            if (pkt->needsResponse()) {
+                assert(bus_pkt->isResponse());
+                if (bus_pkt->isError()) {
+                    pkt->makeAtomicResponse();
+                    pkt->copyError(bus_pkt);
+                } else if (bus_pkt->isRead() ||
+                           bus_pkt->cmd == MemCmd::UpgradeResp) {
+                    // we're updating cache state to allow us to
+                    // satisfy the upstream request from the cache
+                    blk = handleFill(bus_pkt, blk, writebacks);
+                    satisfyCpuSideRequest(pkt, blk);
+                } else {
+                    // we're satisfying the upstream request without
+                    // modifying cache state, e.g., a write-through
+                    pkt->makeAtomicResponse();
+                }
+            }
+            delete bus_pkt;
         }
     }
 
@@ -748,7 +763,10 @@ Cache<TagStore>::handleResponse(PacketPtr pkt)
             miss_latency;
     }
 
-    if (mshr->isCacheFill && !is_error) {
+    bool is_fill = !mshr->isForward &&
+        (pkt->isRead() || pkt->cmd == MemCmd::UpgradeResp);
+
+    if (is_fill && !is_error) {
         DPRINTF(Cache, "Block for addr %x being updated in Cache\n",
                 pkt->getAddr());
 
@@ -771,7 +789,7 @@ Cache<TagStore>::handleResponse(PacketPtr pkt)
 
         if (target->isCpuSide()) {
             Tick completion_time;
-            if (blk != NULL) {
+            if (is_fill) {
                 satisfyCpuSideRequest(target->pkt, blk);
                 // How many bytes past the first request is this one
                 int transfer_offset =
@@ -1287,7 +1305,7 @@ Cache<TagStore>::getTimingPacket()
     PacketPtr tgt_pkt = mshr->getTarget()->pkt;
     PacketPtr pkt = NULL;
 
-    if (mshr->isSimpleForward()) {
+    if (mshr->isForwardNoResponse()) {
         // no response expected, just forward packet as it is
         assert(tags->findBlock(mshr->addr) == NULL);
         pkt = tgt_pkt;
@@ -1295,11 +1313,10 @@ Cache<TagStore>::getTimingPacket()
         BlkType *blk = tags->findBlock(mshr->addr);
         pkt = getBusPacket(tgt_pkt, blk, mshr->needsExclusive());
 
-        mshr->isCacheFill = (pkt != NULL);
+        mshr->isForward = (pkt == NULL);
 
-        if (pkt == NULL) {
+        if (mshr->isForward) {
             // not a cache block request, but a response is expected
-            assert(!mshr->isSimpleForward());
             // make copy of current packet to forward, keep current
             // copy for response handling
             pkt = new Packet(tgt_pkt);
@@ -1473,7 +1490,7 @@ Cache<TagStore>::MemSidePort::sendPacket()
             waitingOnRetry = !success;
             if (waitingOnRetry) {
                 DPRINTF(CachePort, "now waiting on a retry\n");
-                if (!mshr->isSimpleForward()) {
+                if (!mshr->isForwardNoResponse()) {
                     delete pkt;
                 }
             } else {
diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc
index 6537f6343..04b2b8d77 100644
--- a/src/mem/cache/mshr.cc
+++ b/src/mem/cache/mshr.cc
@@ -156,7 +156,7 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target,
     readyTime = whenReady;
     order = _order;
     assert(target);
-    isCacheFill = false;
+    isForward = false;
     _isUncacheable = target->req->isUncacheable();
     inService = false;
     downstreamPending = false;
@@ -187,7 +187,7 @@ bool
 MSHR::markInService()
 {
     assert(!inService);
-    if (isSimpleForward()) {
+    if (isForwardNoResponse()) {
         // we just forwarded the request packet & don't expect a
         // response, so get rid of it
         assert(getNumTargets() == 1);
@@ -403,7 +403,8 @@ MSHR::print(std::ostream &os, int verbosity, const std::string &prefix) const
 {
     ccprintf(os, "%s[%x:%x] %s %s %s state: %s %s %s %s\n",
              prefix, addr, addr+size-1,
-             isCacheFill ? "Fill" : "",
+             isForward ? "Forward" : "",
+             isForwardNoResponse() ? "ForwNoResp" : "",
              needsExclusive() ? "Excl" : "",
              _isUncacheable ? "Unc" : "",
              inService ? "InSvc" : "",
diff --git a/src/mem/cache/mshr.hh b/src/mem/cache/mshr.hh
index fdb0485cb..2ff1c2489 100644
--- a/src/mem/cache/mshr.hh
+++ b/src/mem/cache/mshr.hh
@@ -118,8 +118,8 @@ class MSHR : public Packet::SenderState, public Printable
     /** True if the request has been sent to the bus. */
     bool inService;
 
-    /** True if we will be putting the returned block in the cache */
-    bool isCacheFill;
+    /** True if the request is just a simple forward from an upper level */
+    bool isForward;
 
     /** True if we need to get an exclusive copy of the block. */
     bool needsExclusive() const { return targets->needsExclusive; }
@@ -200,7 +200,7 @@ public:
      * Returns the current number of allocated targets.
      * @return The current number of allocated targets.
      */
-    int getNumTargets() { return ntargets; }
+    int getNumTargets() const { return ntargets; }
 
     /**
      * Returns a pointer to the target list.
@@ -212,13 +212,17 @@ public:
      * Returns true if there are targets left.
      * @return true if there are targets
      */
-    bool hasTargets() { return !targets->empty(); }
+    bool hasTargets() const { return !targets->empty(); }
 
     /**
      * Returns a reference to the first target.
      * @return A pointer to the first target.
      */
-    Target *getTarget() { assert(hasTargets());  return &targets->front(); }
+    Target *getTarget() const
+    {
+        assert(hasTargets());
+        return &targets->front();
+    }
 
     /**
      * Pop first target.
@@ -229,7 +233,7 @@ public:
         targets->pop_front();
     }
 
-    bool isSimpleForward()
+    bool isForwardNoResponse() const
     {
         if (getNumTargets() != 1)
             return false;