Handle deferred snoops better.

--HG--
extra : convert_revision : 703da6128832eb0d5cfed7724e5105f4b3fe4f90

src/mem/cache/cache.hh

@@ -185,14 +185,16 @@ class Cache : public BaseCache
     void satisfyCpuSideRequest(PacketPtr pkt, BlkType *blk);
     bool satisfyMSHR(MSHR *mshr, PacketPtr pkt, BlkType *blk);
 
-    void doTimingSupplyResponse(PacketPtr req_pkt, uint8_t *blk_data);
+    void doTimingSupplyResponse(PacketPtr req_pkt, uint8_t *blk_data,
+                                bool already_copied);
 
     /**
      * Sets the blk to the new state.
      * @param blk The cache block being snooped.
      * @param new_state The new coherence state for the block.
      */
-    void handleSnoop(PacketPtr ptk, BlkType *blk, bool is_timing);
+    void handleSnoop(PacketPtr ptk, BlkType *blk,
+                     bool is_timing, bool is_deferred);
 
     /**
      * Create a writeback request for the given block.

src/mem/cache/cache_impl.hh

@@ -622,7 +622,7 @@ Cache<TagStore,Coherence>::satisfyMSHR(MSHR *mshr, PacketPtr pkt,
         } else {
             // response to snoop request
             DPRINTF(Cache, "processing deferred snoop...\n");
-            handleSnoop(target->pkt, blk, true);
+            handleSnoop(target->pkt, blk, true, true);
         }
 
         mshr->popTarget();
@@ -678,12 +678,10 @@ Cache<TagStore,Coherence>::handleResponse(PacketPtr pkt)
                 pkt->getAddr());
         BlkType *blk = tags->findBlock(pkt->getAddr());
 
-        if (blk == NULL && pkt->cmd == MemCmd::UpgradeResp) {
-            if (!mshr->handleReplacedPendingUpgrade(pkt)) {
-                mq->markPending(mshr);
-                requestMemSideBus((RequestCause)mq->index, pkt->finishTime);
-                return;
-            }
+        if (!mshr->handleFill(pkt, blk)) {
+            mq->markPending(mshr);
+            requestMemSideBus((RequestCause)mq->index, pkt->finishTime);
+            return;
         }
 
         PacketList writebacks;
@@ -814,10 +812,12 @@ Cache<TagStore,Coherence>::handleFill(PacketPtr pkt, BlkType *blk,
 template<class TagStore, class Coherence>
 void
 Cache<TagStore,Coherence>::doTimingSupplyResponse(PacketPtr req_pkt,
-                                                  uint8_t *blk_data)
+                                                  uint8_t *blk_data,
+                                                  bool already_copied)
 {
-    // timing-mode snoop responses require a new packet
-    PacketPtr pkt = new Packet(req_pkt);
+    // timing-mode snoop responses require a new packet, unless we
+    // already made a copy...
+    PacketPtr pkt = already_copied ? req_pkt : new Packet(req_pkt);
     pkt->allocate();
     pkt->makeTimingResponse();
     pkt->setDataFromBlock(blk_data, blkSize);
@@ -827,7 +827,7 @@ Cache<TagStore,Coherence>::doTimingSupplyResponse(PacketPtr req_pkt,
 template<class TagStore, class Coherence>
 void
 Cache<TagStore,Coherence>::handleSnoop(PacketPtr pkt, BlkType *blk,
-                                       bool is_timing)
+                                       bool is_timing, bool is_deferred)
 {
     if (!blk || !blk->isValid()) {
         return;
@@ -854,9 +854,10 @@ Cache<TagStore,Coherence>::handleSnoop(PacketPtr pkt, BlkType *blk,
     }
 
     if (supply) {
+        assert(!pkt->memInhibitAsserted());
         pkt->assertMemInhibit();
         if (is_timing) {
-            doTimingSupplyResponse(pkt, blk->data);
+            doTimingSupplyResponse(pkt, blk->data, is_deferred);
         } else {
             pkt->makeAtomicResponse();
             pkt->setDataFromBlock(blk->data, blkSize);
@@ -892,6 +893,8 @@ Cache<TagStore,Coherence>::snoopTiming(PacketPtr pkt)
     // better not be snooping a request that conflicts with something
     // we have outstanding...
     if (mshr && mshr->inService) {
+        DPRINTF(Cache, "Deferring snoop on in-service MSHR to blk %x\n",
+                blk_addr);
         mshr->allocateSnoopTarget(pkt, curTick, order++);
         if (mshr->getNumTargets() > numTarget)
            warn("allocating bonus target for snoop"); //handle later
@@ -913,6 +916,7 @@ Cache<TagStore,Coherence>::snoopTiming(PacketPtr pkt)
             assert(wb_pkt->cmd == MemCmd::Writeback);
 
             if (pkt->isRead()) {
+                assert(!pkt->memInhibitAsserted());
                 pkt->assertMemInhibit();
                 if (!pkt->needsExclusive()) {
                     pkt->assertShared();
@@ -922,7 +926,7 @@ Cache<TagStore,Coherence>::snoopTiming(PacketPtr pkt)
                     // the packet's invalidate flag is set...
                     assert(pkt->isInvalidate());
                 }
-                doTimingSupplyResponse(pkt, wb_pkt->getPtr<uint8_t>());
+                doTimingSupplyResponse(pkt, wb_pkt->getPtr<uint8_t>(), false);
             }
 
             if (pkt->isInvalidate()) {
@@ -933,7 +937,7 @@ Cache<TagStore,Coherence>::snoopTiming(PacketPtr pkt)
         }
     }
 
-    handleSnoop(pkt, blk, true);
+    handleSnoop(pkt, blk, true, false);
 }
 
 
@@ -948,7 +952,7 @@ Cache<TagStore,Coherence>::snoopAtomic(PacketPtr pkt)
     }
 
     BlkType *blk = tags->findBlock(pkt->getAddr());
-    handleSnoop(pkt, blk, false);
+    handleSnoop(pkt, blk, false, false);
     return hitLatency;
 }
 

src/mem/cache/miss/mshr.cc

@@ -75,6 +75,7 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target,
     assert(deferredTargets.empty());
     deferredNeedsExclusive = false;
     pendingInvalidate = false;
+    pendingShared = false;
     replacedPendingUpgrade = false;
     data = NULL;
 }
@@ -120,7 +121,7 @@ MSHR::allocateTarget(PacketPtr target, Tick when, Counter _order)
 }
 
 void
-MSHR::allocateSnoopTarget(PacketPtr target, Tick when, Counter _order)
+MSHR::allocateSnoopTarget(PacketPtr pkt, Tick when, Counter _order)
 {
     assert(inService); // don't bother to call otherwise
 
@@ -130,23 +131,33 @@ MSHR::allocateSnoopTarget(PacketPtr target, Tick when, Counter _order)
         return;
     }
 
-    if (needsExclusive) {
-        // We're awaiting an exclusive copy, so ownership is pending.
-        // It's up to us to respond once the data arrives.
-        target->assertMemInhibit();
-    }
+    DPRINTF(Cache, "deferred snoop on %x: %s %s\n", addr,
+            needsExclusive ? "needsExclusive" : "",
+            pkt->needsExclusive() ? "pkt->needsExclusive()" : "");
 
-    if (target->needsExclusive()) {
-        // This transaction will take away our pending copy
-        pendingInvalidate = true;
+    if (needsExclusive || pkt->needsExclusive()) {
+        // actual target device (typ. PhysicalMemory) will delete the
+        // packet on reception, so we need to save a copy here
+        targets.push_back(Target(new Packet(pkt), when, _order, false));
+        ++ntargets;
+
+        if (needsExclusive) {
+            // We're awaiting an exclusive copy, so ownership is pending.
+            // It's up to us to respond once the data arrives.
+            pkt->assertMemInhibit();
+        }
+
+        if (pkt->needsExclusive()) {
+            // This transaction will take away our pending copy
+            pendingInvalidate = true;
+        }
     } else {
-        // We'll keep our pending copy, but we can't let the other guy
-        // think he's getting it exclusive
-        target->assertShared();
+        // Read to a read: no conflict, so no need to record as
+        // target, but make sure neither reader thinks he's getting an
+        // exclusive copy
+        pendingShared = true;
+        pkt->assertShared();
     }
-
-    targets.push_back(Target(target, when, _order, false));
-    ++ntargets;
 }
 
 
@@ -164,6 +175,7 @@ MSHR::promoteDeferredTargets()
 
     needsExclusive = deferredNeedsExclusive;
     pendingInvalidate = false;
+    pendingShared = false;
     deferredNeedsExclusive = false;
     order = targets.front().order;
     readyTick = std::max(curTick, targets.front().time);
@@ -200,25 +212,33 @@ MSHR::handleReplacement(CacheBlk *blk, int blkSize)
 
 
 bool
-MSHR::handleReplacedPendingUpgrade(Packet *pkt)
+MSHR::handleFill(Packet *pkt, CacheBlk *blk)
 {
-    // @TODO: if upgrade is nacked and replacedPendingUpgradeDirty is true, then we need to writeback the data (or rel
-    assert(pkt->cmd == MemCmd::UpgradeResp);
-    assert(replacedPendingUpgrade);
-    replacedPendingUpgrade = false; // reset
-    if (replacedPendingUpgradeDirty) {
-        // we wrote back the previous copy; just reissue as a ReadEx
-        return false;
+    if (replacedPendingUpgrade) {
+        // block was replaced while upgrade request was in service
+        assert(pkt->cmd == MemCmd::UpgradeResp);
+        assert(blk == NULL);
+        assert(replacedPendingUpgrade);
+        replacedPendingUpgrade = false; // reset
+        if (replacedPendingUpgradeDirty) {
+            // we wrote back the previous copy; just reissue as a ReadEx
+            return false;
+        }
+
+        // previous copy was not dirty, but we are now owner...  fake out
+        // cache by taking saved data and converting UpgradeResp to
+        // ReadExResp
+        assert(data);
+        pkt->cmd = MemCmd::ReadExResp;
+        pkt->setData(data);
+        delete [] data;
+        data = NULL;
+    } else if (pendingShared) {
+        // we snooped another read while this read was in
+        // service... assert shared line on its behalf
+        pkt->assertShared();
     }
 
-    // previous copy was not dirty, but we are now owner...  fake out
-    // cache by taking saved data and converting UpgradeResp to
-    // ReadExResp
-    assert(data);
-    pkt->cmd = MemCmd::ReadExResp;
-    pkt->setData(data);
-    delete [] data;
-    data = NULL;
     return true;
 }
 

src/mem/cache/miss/mshr.hh

@@ -105,6 +105,7 @@ class MSHR : public Packet::SenderState
 
     bool deferredNeedsExclusive;
     bool pendingInvalidate;
+    bool pendingShared;
     /** Is there a pending upgrade that got replaced? */
     bool replacedPendingUpgrade;
     bool replacedPendingUpgradeDirty;
@@ -213,7 +214,7 @@ public:
     bool promoteDeferredTargets();
 
     void handleReplacement(CacheBlk *blk, int blkSize);
-    bool handleReplacedPendingUpgrade(Packet *pkt);
+    bool handleFill(Packet *pkt, CacheBlk *blk);
 
     /**
      * Prints the contents of this MSHR to stderr.

src/mem/cache/tags/lru.cc

@@ -207,6 +207,9 @@ LRU::findReplacement(Addr addr, PacketList &writebacks)
         totalRefs += blk->refCount;
         ++sampledRefs;
         blk->refCount = 0;
+
+        DPRINTF(Cache, "set %x: selecting blk %x for replacement\n",
+                set, regenerateBlkAddr(blk->tag, set));
     } else if (!blk->isTouched) {
         tagsInUse++;
         blk->isTouched = true;
@@ -216,8 +219,6 @@ LRU::findReplacement(Addr addr, PacketList &writebacks)
         }
     }
 
-    DPRINTF(Cache, "set %x: selecting blk %x for replacement\n",
-            set, regenerateBlkAddr(blk->tag, set));
     return blk;
 }
 

src/mem/tport.cc

@@ -69,11 +69,21 @@ SimpleTimingPort::recvTiming(PacketPtr pkt)
     // if we ever added it back.
     assert(pkt->isRequest());
     assert(pkt->result == Packet::Unknown);
+
+    if (pkt->memInhibitAsserted()) {
+        // snooper will supply based on copy of packet
+        // still target's responsibility to delete packet
+        delete pkt->req;
+        delete pkt;
+        return true;
+    }
+
     bool needsResponse = pkt->needsResponse();
     Tick latency = recvAtomic(pkt);
     // turn packet around to go back to requester if response expected
     if (needsResponse) {
-        // recvAtomic() should already have turned packet into atomic response
+        // recvAtomic() should already have turned packet into
+        // atomic response
         assert(pkt->isResponse());
         pkt->convertAtomicToTimingResponse();
         schedSendTiming(pkt, curTick + latency);
@@ -81,6 +91,7 @@ SimpleTimingPort::recvTiming(PacketPtr pkt)
         delete pkt->req;
         delete pkt;
     }
+
     return true;
 }