mem: Unify delayed packet deletion

This patch unifies how we deal with delayed packet deletion, where the
receiving slave is responsible for deleting the packet, but the
sending agent (e.g. a cache) is still relying on the pointer until the
call to sendTimingReq completes. Previously we used a mix of a
deletion vector and a construct using unique_ptr. With this patch we
ensure all slaves use the latter approach.

12 files changed, 43 insertions, 88 deletions

diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index 3e5ed42bc..aa95d5604 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -542,15 +542,6 @@ bool
 Cache::recvTimingReq(PacketPtr pkt)
 {
     DPRINTF(CacheTags, "%s tags: %s\n", __func__, tags->print());
-//@todo Add back in MemDebug Calls
-//    MemDebug::cacheAccess(pkt);
-
-
-    /// @todo temporary hack to deal with memory corruption issue until
-    /// 4-phase transactions are complete
-    for (int x = 0; x < pendingDelete.size(); x++)
-        delete pendingDelete[x];
-    pendingDelete.clear();
 
     assert(pkt->isRequest());
 
@@ -602,10 +593,9 @@ Cache::recvTimingReq(PacketPtr pkt)
             // main memory will delete the packet
         }
 
-        /// @todo nominally we should just delete the packet here,
-        /// however, until 4-phase stuff we can't because sending
-        /// cache is still relying on it.
-        pendingDelete.push_back(pkt);
+        // queue for deletion, as the sending cache is still relying
+        // on the packet
+        pendingDelete.reset(pkt);
 
         // no need to take any action in this particular cache as the
         // caches along the path to memory are allowed to keep lines
@@ -678,12 +668,11 @@ Cache::recvTimingReq(PacketPtr pkt)
             // by access(), that calls accessBlock() function.
             cpuSidePort->schedTimingResp(pkt, request_time);
         } else {
-            /// @todo nominally we should just delete the packet here,
-            /// however, until 4-phase stuff we can't because sending cache is
-            /// still relying on it. If the block is found in access(),
-            /// CleanEvict and Writeback messages will be deleted here as
-            /// well.
-            pendingDelete.push_back(pkt);
+            // queue the packet for deletion, as the sending cache is
+            // still relying on it; if the block is found in access(),
+            // CleanEvict and Writeback messages will be deleted
+            // here as well
+            pendingDelete.reset(pkt);
         }
     } else {
         // miss
@@ -754,7 +743,7 @@ Cache::recvTimingReq(PacketPtr pkt)
                 // CleanEvicts corresponding to blocks which have outstanding
                 // requests in MSHRs can be deleted here.
                 if (pkt->cmd == MemCmd::CleanEvict) {
-                    pendingDelete.push_back(pkt);
+                    pendingDelete.reset(pkt);
                 } else {
                     DPRINTF(Cache, "%s coalescing MSHR for %s addr %#llx size %d\n",
                             __func__, pkt->cmdString(), pkt->getAddr(),
diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh
index ec436201b..ae9e7e694 100644
--- a/src/mem/cache/cache.hh
+++ b/src/mem/cache/cache.hh
@@ -195,11 +195,10 @@ class Cache : public BaseCache
     const bool prefetchOnAccess;
 
     /**
-     * @todo this is a temporary workaround until the 4-phase code is committed.
-     * upstream caches need this packet until true is returned, so hold it for
-     * deletion until a subsequent call
+     * Upstream caches need this packet until true is returned, so
+     * hold it for deletion until a subsequent call
      */
-    std::vector<PacketPtr> pendingDelete;
+    std::unique_ptr<Packet> pendingDelete;
 
     /**
      * Does all the processing necessary to perform the provided request.
diff --git a/src/mem/coherent_xbar.cc b/src/mem/coherent_xbar.cc
index 6a0136ea8..223ab6ab5 100644
--- a/src/mem/coherent_xbar.cc
+++ b/src/mem/coherent_xbar.cc
@@ -140,12 +140,6 @@ CoherentXBar::init()
 bool
 CoherentXBar::recvTimingReq(PacketPtr pkt, PortID slave_port_id)
 {
-    // @todo temporary hack to deal with memory corruption issue until
-    // 4-phase transactions are complete
-    for (int x = 0; x < pendingDelete.size(); x++)
-        delete pendingDelete[x];
-    pendingDelete.clear();
-
     // determine the source port based on the id
     SlavePort *src_port = slavePorts[slave_port_id];
 
@@ -223,7 +217,10 @@ CoherentXBar::recvTimingReq(PacketPtr pkt, PortID slave_port_id)
 
         // update the layer state and schedule an idle event
         reqLayers[master_port_id]->succeededTiming(packetFinishTime);
-        pendingDelete.push_back(pkt);
+
+        // queue the packet for deletion
+        pendingDelete.reset(pkt);
+
         return true;
     }
 
diff --git a/src/mem/coherent_xbar.hh b/src/mem/coherent_xbar.hh
index e99e9374f..24d00628b 100644
--- a/src/mem/coherent_xbar.hh
+++ b/src/mem/coherent_xbar.hh
@@ -274,11 +274,10 @@ class CoherentXBar : public BaseXBar
     const Cycles snoopResponseLatency;
 
     /**
-     * @todo this is a temporary workaround until the 4-phase code is committed.
-     * upstream caches need this packet until true is returned, so hold it for
-     * deletion until a subsequent call
+     * Upstream caches need this packet until true is returned, so
+     * hold it for deletion until a subsequent call
      */
-    std::vector<PacketPtr> pendingDelete;
+    std::unique_ptr<Packet> pendingDelete;
 
     /** Function called by the port when the crossbar is recieving a Timing
       request packet.*/
diff --git a/src/mem/dram_ctrl.cc b/src/mem/dram_ctrl.cc
index 3ba4616f7..4bd83f761 100644
--- a/src/mem/dram_ctrl.cc
+++ b/src/mem/dram_ctrl.cc
@@ -586,12 +586,6 @@ DRAMCtrl::printQs() const {
 bool
 DRAMCtrl::recvTimingReq(PacketPtr pkt)
 {
-    /// @todo temporary hack to deal with memory corruption issues until
-    /// 4-phase transactions are complete
-    for (int x = 0; x < pendingDelete.size(); x++)
-        delete pendingDelete[x];
-    pendingDelete.clear();
-
     // This is where we enter from the outside world
     DPRINTF(DRAM, "recvTimingReq: request %s addr %lld size %d\n",
             pkt->cmdString(), pkt->getAddr(), pkt->getSize());
@@ -600,7 +594,7 @@ DRAMCtrl::recvTimingReq(PacketPtr pkt)
     if (pkt->memInhibitAsserted() ||
         pkt->cmd == MemCmd::CleanEvict) {
         DPRINTF(DRAM, "Inhibited packet or clean evict -- Dropping it now\n");
-        pendingDelete.push_back(pkt);
+        pendingDelete.reset(pkt);
         return true;
     }
 
@@ -872,7 +866,7 @@ DRAMCtrl::accessAndRespond(PacketPtr pkt, Tick static_latency)
     } else {
         // @todo the packet is going to be deleted, and the DRAMPacket
         // is still having a pointer to it
-        pendingDelete.push_back(pkt);
+        pendingDelete.reset(pkt);
     }
 
     DPRINTF(DRAM, "Done\n");
diff --git a/src/mem/dram_ctrl.hh b/src/mem/dram_ctrl.hh
index 617c94914..6cd72b266 100644
--- a/src/mem/dram_ctrl.hh
+++ b/src/mem/dram_ctrl.hh
@@ -836,11 +836,11 @@ class DRAMCtrl : public AbstractMemory
     // timestamp offset
     uint64_t timeStampOffset;
 
-    /** @todo this is a temporary workaround until the 4-phase code is
-     * committed. upstream caches needs this packet until true is returned, so
-     * hold onto it for deletion until a subsequent call
+    /**
+     * Upstream caches need this packet until true is returned, so
+     * hold it for deletion until a subsequent call
      */
-    std::vector<PacketPtr> pendingDelete;
+    std::unique_ptr<Packet> pendingDelete;
 
     /**
      * This function increments the energy when called. If stats are
diff --git a/src/mem/dramsim2.cc b/src/mem/dramsim2.cc
index 64acc5b57..cd0f45b23 100644
--- a/src/mem/dramsim2.cc
+++ b/src/mem/dramsim2.cc
@@ -178,16 +178,10 @@ DRAMSim2::recvTimingReq(PacketPtr pkt)
     // we should never see a new request while in retry
     assert(!retryReq);
 
-    // @todo temporary hack to deal with memory corruption issues until
-    // 4-phase transactions are complete
-    for (int x = 0; x < pendingDelete.size(); x++)
-        delete pendingDelete[x];
-    pendingDelete.clear();
-
     if (pkt->memInhibitAsserted()) {
         // snooper will supply based on copy of packet
         // still target's responsibility to delete packet
-        pendingDelete.push_back(pkt);
+        pendingDelete.reset(pkt);
         return true;
     }
 
@@ -281,9 +275,8 @@ DRAMSim2::accessAndRespond(PacketPtr pkt)
         if (!retryResp && !sendResponseEvent.scheduled())
             schedule(sendResponseEvent, time);
     } else {
-        // @todo the packet is going to be deleted, and the DRAMPacket
-        // is still having a pointer to it
-        pendingDelete.push_back(pkt);
+        // queue the packet for deletion
+        pendingDelete.reset(pkt);
     }
 }
 
diff --git a/src/mem/dramsim2.hh b/src/mem/dramsim2.hh
index e57479247..5cde19cd8 100644
--- a/src/mem/dramsim2.hh
+++ b/src/mem/dramsim2.hh
@@ -160,11 +160,11 @@ class DRAMSim2 : public AbstractMemory
      */
     EventWrapper<DRAMSim2, &DRAMSim2::tick> tickEvent;
 
-    /** @todo this is a temporary workaround until the 4-phase code is
-     * committed. upstream caches needs this packet until true is returned, so
-     * hold onto it for deletion until a subsequent call
+    /**
+     * Upstream caches need this packet until true is returned, so
+     * hold it for deletion until a subsequent call
      */
-    std::vector<PacketPtr> pendingDelete;
+    std::unique_ptr<Packet> pendingDelete;
 
   public:
 
diff --git a/src/mem/simple_mem.cc b/src/mem/simple_mem.cc
index f68066e75..7e350feb6 100644
--- a/src/mem/simple_mem.cc
+++ b/src/mem/simple_mem.cc
@@ -97,16 +97,10 @@ SimpleMemory::recvFunctional(PacketPtr pkt)
 bool
 SimpleMemory::recvTimingReq(PacketPtr pkt)
 {
-    /// @todo temporary hack to deal with memory corruption issues until
-    /// 4-phase transactions are complete
-    for (int x = 0; x < pendingDelete.size(); x++)
-        delete pendingDelete[x];
-    pendingDelete.clear();
-
     if (pkt->memInhibitAsserted()) {
         // snooper will supply based on copy of packet
         // still target's responsibility to delete packet
-        pendingDelete.push_back(pkt);
+        pendingDelete.reset(pkt);
         return true;
     }
 
@@ -165,7 +159,7 @@ SimpleMemory::recvTimingReq(PacketPtr pkt)
         if (!retryResp && !dequeueEvent.scheduled())
             schedule(dequeueEvent, packetQueue.back().tick);
     } else {
-        pendingDelete.push_back(pkt);
+        pendingDelete.reset(pkt);
     }
 
     return true;
diff --git a/src/mem/simple_mem.hh b/src/mem/simple_mem.hh
index 35d8aeafb..d19de7608 100644
--- a/src/mem/simple_mem.hh
+++ b/src/mem/simple_mem.hh
@@ -175,11 +175,11 @@ class SimpleMemory : public AbstractMemory
      */
     Tick getLatency() const;
 
-    /** @todo this is a temporary workaround until the 4-phase code is
-     * committed. upstream caches needs this packet until true is returned, so
-     * hold onto it for deletion until a subsequent call
+    /**
+     * Upstream caches need this packet until true is returned, so
+     * hold it for deletion until a subsequent call
      */
-    std::vector<PacketPtr> pendingDelete;
+    std::unique_ptr<Packet> pendingDelete;
 
   public:
 
diff --git a/src/mem/tport.cc b/src/mem/tport.cc
index aa783ada0..9f9403a22 100644
--- a/src/mem/tport.cc
+++ b/src/mem/tport.cc
@@ -62,12 +62,6 @@ SimpleTimingPort::recvFunctional(PacketPtr pkt)
 bool
 SimpleTimingPort::recvTimingReq(PacketPtr pkt)
 {
-    /// @todo temporary hack to deal with memory corruption issue until
-    /// 4-phase transactions are complete. Remove me later
-    for (int x = 0; x < pendingDelete.size(); x++)
-        delete pendingDelete[x];
-    pendingDelete.clear();
-
     // the SimpleTimingPort should not be used anywhere where there is
     // a need to deal with inhibited packets
     if (pkt->memInhibitAsserted())
@@ -82,10 +76,8 @@ SimpleTimingPort::recvTimingReq(PacketPtr pkt)
         assert(pkt->isResponse());
         schedTimingResp(pkt, curTick() + latency);
     } else {
-        /// @todo nominally we should just delete the packet here.
-        /// Until 4-phase stuff we can't because the sending
-        /// cache is still relying on it
-        pendingDelete.push_back(pkt);
+        // queue the packet for deletion
+        pendingDelete.reset(pkt);
     }
 
     return true;
diff --git a/src/mem/tport.hh b/src/mem/tport.hh
index 166a23125..d7e4bbc74 100644
--- a/src/mem/tport.hh
+++ b/src/mem/tport.hh
@@ -81,12 +81,10 @@ class SimpleTimingPort : public QueuedSlavePort
     virtual Tick recvAtomic(PacketPtr pkt) = 0;
 
     /**
-     * @todo this is a temporary workaround until the 4-phase code is committed.
-     * upstream caches need this packet until true is returned, so hold it for
-     * deletion until a subsequent call
+     * Upstream caches need this packet until true is returned, so
+     * hold it for deletion until a subsequent call
      */
-    std::vector<PacketPtr> pendingDelete;
-
+    std::unique_ptr<Packet> pendingDelete;
 
   public: