Replace DeferredSnoop flag with LowerMSHRPending flag.

Turns out DeferredSnoop isn't quite the right bit of info
we needed... see new comment in cache_impl.hh.

--HG--
extra : convert_revision : a38de8c1677a37acafb743b7074ef88b21d3b7be

4 files changed, 26 insertions, 13 deletions

diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh
index 57028a05e..7dfe9e8f1 100644
--- a/src/mem/cache/cache.hh
+++ b/src/mem/cache/cache.hh
@@ -190,7 +190,8 @@ class Cache : public BaseCache
      * @param new_state The new coherence state for the block.
      */
     void handleSnoop(PacketPtr ptk, BlkType *blk,
-                     bool is_timing, bool is_deferred);
+                     bool is_timing, bool is_deferred,
+                     bool lower_mshr_pending);
 
     /**
      * Create a writeback request for the given block.
diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index c8c1a239c..82410afe1 100644
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -754,7 +754,7 @@ Cache<TagStore>::handleResponse(PacketPtr pkt)
         } else {
             // response to snoop request
             DPRINTF(Cache, "processing deferred snoop...\n");
-            handleSnoop(target->pkt, blk, true, true);
+            handleSnoop(target->pkt, blk, true, true, false);
         }
 
         mshr->popTarget();
@@ -917,7 +917,8 @@ Cache<TagStore>::doTimingSupplyResponse(PacketPtr req_pkt,
 template<class TagStore>
 void
 Cache<TagStore>::handleSnoop(PacketPtr pkt, BlkType *blk,
-                             bool is_timing, bool is_deferred)
+                             bool is_timing, bool is_deferred,
+                             bool lower_mshr_pending)
 {
     assert(pkt->isRequest());
 
@@ -929,8 +930,8 @@ Cache<TagStore>::handleSnoop(PacketPtr pkt, BlkType *blk,
     if (is_timing) {
         Packet *snoopPkt = new Packet(pkt, true);  // clear flags
         snoopPkt->setExpressSnoop();
-        if (is_deferred) {
-            snoopPkt->setDeferredSnoop();
+        if (lower_mshr_pending) {
+            snoopPkt->setLowerMSHRPending();
         }
         snoopPkt->senderState = new ForwardResponseRecord(pkt, this);
         cpuSidePort->sendTiming(snoopPkt);
@@ -1017,8 +1018,19 @@ Cache<TagStore>::snoopTiming(PacketPtr pkt)
 
     Addr blk_addr = pkt->getAddr() & ~(Addr(blkSize-1));
     MSHR *mshr = mshrQueue.findMatch(blk_addr);
-    // better not be snooping a request that conflicts with something
-    // we have outstanding...
+
+    // If a lower cache has an operation on this block pending (not
+    // yet in service) on the MSHR, then the upper caches need to know
+    // about it, as this means that the pending operation logically
+    // succeeds the current snoop.  It's not sufficient to record
+    // whether the MSHR *is* in service, as this misses the window
+    // where the lower cache has completed the request and the
+    // response is on its way back up the hierarchy.
+    bool lower_mshr_pending =
+        (mshr && (!mshr->inService) || pkt->lowerMSHRPending());
+
+    // Let the MSHR itself track the snoop and decide whether we want
+    // to go ahead and do the regular cache snoop
     if (mshr && mshr->handleSnoop(pkt, order++)) {
         DPRINTF(Cache, "Deferring snoop on in-service MSHR to blk %x\n",
                 blk_addr);
@@ -1063,7 +1075,7 @@ Cache<TagStore>::snoopTiming(PacketPtr pkt)
         }
     }
 
-    handleSnoop(pkt, blk, true, false);
+    handleSnoop(pkt, blk, true, false, lower_mshr_pending);
 }
 
 
@@ -1078,7 +1090,7 @@ Cache<TagStore>::snoopAtomic(PacketPtr pkt)
     }
 
     BlkType *blk = tags->findBlock(pkt->getAddr());
-    handleSnoop(pkt, blk, false, false);
+    handleSnoop(pkt, blk, false, false, false);
     return hitLatency;
 }
 
diff --git a/src/mem/cache/miss/mshr.cc b/src/mem/cache/miss/mshr.cc
index 856819c10..b9dfdf729 100644
--- a/src/mem/cache/miss/mshr.cc
+++ b/src/mem/cache/miss/mshr.cc
@@ -164,7 +164,7 @@ MSHR::allocateTarget(PacketPtr pkt, Tick whenReady, Counter _order)
 bool
 MSHR::handleSnoop(PacketPtr pkt, Counter _order)
 {
-    if (!inService || (pkt->isExpressSnoop() && !pkt->isDeferredSnoop())) {
+    if (!inService || (pkt->isExpressSnoop() && pkt->lowerMSHRPending())) {
         // Request has not been issued yet, or it's been issued
         // locally but is buffered unissued at some downstream cache
         // which is forwarding us this snoop.  Either way, the packet
diff --git a/src/mem/packet.hh b/src/mem/packet.hh
index 8063c7ae7..779ea49a2 100644
--- a/src/mem/packet.hh
+++ b/src/mem/packet.hh
@@ -257,7 +257,7 @@ class Packet : public FastAlloc
         Shared,
         // Special control flags
         ExpressSnoop,
-        DeferredSnoop,
+        LowerMSHRPending,  // not yet in service
         NUM_PACKET_FLAGS
     };
 
@@ -323,8 +323,8 @@ class Packet : public FastAlloc
     // Special control flags
     void setExpressSnoop()      { flags[ExpressSnoop] = true; }
     bool isExpressSnoop()       { return flags[ExpressSnoop]; }
-    void setDeferredSnoop()     { flags[DeferredSnoop] = true; }
-    bool isDeferredSnoop()      { return flags[DeferredSnoop]; }
+    void setLowerMSHRPending()  { flags[LowerMSHRPending] = true; }
+    bool lowerMSHRPending()     { return flags[LowerMSHRPending]; }
 
     // Network error conditions... encapsulate them as methods since
     // their encoding keeps changing (from result field to command