Replace lowerMSHRPending flag with more robust scheme

based on following Packet senderState links.

--HG--
extra : convert_revision : 9027d59bd7242aa0e4275bf94d8b1fb27bd59d79

6 files changed, 60 insertions, 37 deletions

diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh
index 7dfe9e8f1..57028a05e 100644
--- a/src/mem/cache/cache.hh
+++ b/src/mem/cache/cache.hh
@@ -190,8 +190,7 @@ 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 lower_mshr_pending);
+                     bool is_timing, bool is_deferred);
 
     /**
      * 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 82410afe1..efd7f4588 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, false);
+            handleSnoop(target->pkt, blk, true, true);
         }
 
         mshr->popTarget();
@@ -917,8 +917,7 @@ Cache<TagStore>::doTimingSupplyResponse(PacketPtr req_pkt,
 template<class TagStore>
 void
 Cache<TagStore>::handleSnoop(PacketPtr pkt, BlkType *blk,
-                             bool is_timing, bool is_deferred,
-                             bool lower_mshr_pending)
+                             bool is_timing, bool is_deferred)
 {
     assert(pkt->isRequest());
 
@@ -930,9 +929,6 @@ Cache<TagStore>::handleSnoop(PacketPtr pkt, BlkType *blk,
     if (is_timing) {
         Packet *snoopPkt = new Packet(pkt, true);  // clear flags
         snoopPkt->setExpressSnoop();
-        if (lower_mshr_pending) {
-            snoopPkt->setLowerMSHRPending();
-        }
         snoopPkt->senderState = new ForwardResponseRecord(pkt, this);
         cpuSidePort->sendTiming(snoopPkt);
         if (snoopPkt->memInhibitAsserted()) {
@@ -1019,16 +1015,6 @@ Cache<TagStore>::snoopTiming(PacketPtr pkt)
     Addr blk_addr = pkt->getAddr() & ~(Addr(blkSize-1));
     MSHR *mshr = mshrQueue.findMatch(blk_addr);
 
-    // 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++)) {
@@ -1075,7 +1061,7 @@ Cache<TagStore>::snoopTiming(PacketPtr pkt)
         }
     }
 
-    handleSnoop(pkt, blk, true, false, lower_mshr_pending);
+    handleSnoop(pkt, blk, true, false);
 }
 
 
@@ -1090,7 +1076,7 @@ Cache<TagStore>::snoopAtomic(PacketPtr pkt)
     }
 
     BlkType *blk = tags->findBlock(pkt->getAddr());
-    handleSnoop(pkt, blk, false, false, false);
+    handleSnoop(pkt, blk, false, false);
     return hitLatency;
 }
 
diff --git a/src/mem/cache/miss/mshr.cc b/src/mem/cache/miss/mshr.cc
index b9dfdf729..9b05aea3f 100644
--- a/src/mem/cache/miss/mshr.cc
+++ b/src/mem/cache/miss/mshr.cc
@@ -63,7 +63,8 @@ MSHR::TargetList::TargetList()
 
 
 inline void
-MSHR::TargetList::add(PacketPtr pkt, Tick readyTime, Counter order, bool cpuSide)
+MSHR::TargetList::add(PacketPtr pkt, Tick readyTime,
+                      Counter order, bool cpuSide)
 {
     if (cpuSide) {
         if (pkt->needsExclusive()) {
@@ -73,6 +74,12 @@ MSHR::TargetList::add(PacketPtr pkt, Tick readyTime, Counter order, bool cpuSide
         if (pkt->cmd == MemCmd::UpgradeReq) {
             hasUpgrade = true;
         }
+
+        MSHR *mshr = dynamic_cast<MSHR*>(pkt->senderState);
+        if (mshr != NULL) {
+            assert(!mshr->downstreamPending);
+            mshr->downstreamPending = true;
+        }
     }
 
     push_back(Target(pkt, readyTime, order, cpuSide));
@@ -98,6 +105,20 @@ MSHR::TargetList::replaceUpgrades()
 
 
 void
+MSHR::TargetList::clearDownstreamPending()
+{
+    Iterator end_i = end();
+    for (Iterator i = begin(); i != end_i; ++i) {
+        MSHR *mshr = dynamic_cast<MSHR*>(i->pkt->senderState);
+        if (mshr != NULL) {
+            assert(mshr->downstreamPending);
+            mshr->downstreamPending = false;
+        }
+    }
+}
+
+
+void
 MSHR::allocate(Addr _addr, int _size, PacketPtr target,
                Tick whenReady, Counter _order)
 {
@@ -109,6 +130,7 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target,
     isCacheFill = false;
     _isUncacheable = target->req->isUncacheable();
     inService = false;
+    downstreamPending = false;
     threadNum = 0;
     ntargets = 1;
     // Don't know of a case where we would allocate a new MSHR for a
@@ -121,6 +143,28 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target,
     data = NULL;
 }
 
+
+bool
+MSHR::markInService()
+{
+    assert(!inService);
+    if (isSimpleForward()) {
+        // we just forwarded the request packet & don't expect a
+        // response, so get rid of it
+        assert(getNumTargets() == 1);
+        popTarget();
+        return true;
+    }
+    inService = true;
+    if (!downstreamPending) {
+        // let upstream caches know that the request has made it to a
+        // level where it's going to get a response
+        targets->clearDownstreamPending();
+    }
+    return false;
+}
+
+
 void
 MSHR::deallocate()
 {
@@ -164,7 +208,7 @@ MSHR::allocateTarget(PacketPtr pkt, Tick whenReady, Counter _order)
 bool
 MSHR::handleSnoop(PacketPtr pkt, Counter _order)
 {
-    if (!inService || (pkt->isExpressSnoop() && pkt->lowerMSHRPending())) {
+    if (!inService || downstreamPending) {
         // 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/cache/miss/mshr.hh b/src/mem/cache/miss/mshr.hh
index 06ef6e113..e850a8633 100644
--- a/src/mem/cache/miss/mshr.hh
+++ b/src/mem/cache/miss/mshr.hh
@@ -81,6 +81,7 @@ class MSHR : public Packet::SenderState
         bool isReset()    { return !needsExclusive && !hasUpgrade; }
         void add(PacketPtr pkt, Tick readyTime, Counter order, bool cpuSide);
         void replaceUpgrades();
+        void clearDownstreamPending();
     };
 
     /** A list of MSHRs. */
@@ -117,6 +118,8 @@ class MSHR : public Packet::SenderState
     /** True if the request is uncacheable */
     bool _isUncacheable;
 
+    bool downstreamPending;
+
     bool pendingInvalidate;
     bool pendingShared;
 
@@ -163,6 +166,8 @@ public:
     void allocate(Addr addr, int size, PacketPtr pkt,
                   Tick when, Counter _order);
 
+    bool markInService();
+
     /**
      * Mark this MSHR as free.
      */
diff --git a/src/mem/cache/miss/mshr_queue.cc b/src/mem/cache/miss/mshr_queue.cc
index 4d3cf30e1..50a28fb3c 100644
--- a/src/mem/cache/miss/mshr_queue.cc
+++ b/src/mem/cache/miss/mshr_queue.cc
@@ -179,20 +179,12 @@ MSHRQueue::moveToFront(MSHR *mshr)
 void
 MSHRQueue::markInService(MSHR *mshr)
 {
-    assert(!mshr->inService);
-    if (mshr->isSimpleForward()) {
-        // we just forwarded the request packet & don't expect a
-        // response, so get rid of it
-        assert(mshr->getNumTargets() == 1);
-        mshr->popTarget();
+    if (mshr->markInService()) {
         deallocate(mshr);
-        return;
+    } else {
+        readyList.erase(mshr->readyIter);
+        inServiceEntries += 1;
     }
-    mshr->inService = true;
-    readyList.erase(mshr->readyIter);
-    //mshr->readyIter = NULL;
-    inServiceEntries += 1;
-    //readyList.pop_front();
 }
 
 void
diff --git a/src/mem/packet.hh b/src/mem/packet.hh
index 779ea49a2..036bd3fd7 100644
--- a/src/mem/packet.hh
+++ b/src/mem/packet.hh
@@ -257,7 +257,6 @@ class Packet : public FastAlloc
         Shared,
         // Special control flags
         ExpressSnoop,
-        LowerMSHRPending,  // not yet in service
         NUM_PACKET_FLAGS
     };
 
@@ -323,8 +322,6 @@ class Packet : public FastAlloc
     // Special control flags
     void setExpressSnoop()      { flags[ExpressSnoop] = true; }
     bool isExpressSnoop()       { return flags[ExpressSnoop]; }
-    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