diff options
author | Andreas Hansson <andreas.hansson@arm.com> | 2015-11-06 03:26:21 -0500 |
---|---|---|
committer | Andreas Hansson <andreas.hansson@arm.com> | 2015-11-06 03:26:21 -0500 |
commit | ac1368df50af123b32b41d7115ea4a0f15f7c97f (patch) | |
tree | 107ba41258ee0a30de841624bc6f3c5fab041d8f /src/mem | |
parent | 2cb5467e8514934c4d88304c65050adead0ffc7e (diff) | |
download | gem5-ac1368df50af123b32b41d7115ea4a0f15f7c97f.tar.xz |
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.
Diffstat (limited to 'src/mem')
-rw-r--r-- | src/mem/cache/cache.cc | 29 | ||||
-rw-r--r-- | src/mem/cache/cache.hh | 7 | ||||
-rw-r--r-- | src/mem/coherent_xbar.cc | 11 | ||||
-rw-r--r-- | src/mem/coherent_xbar.hh | 7 | ||||
-rw-r--r-- | src/mem/dram_ctrl.cc | 10 | ||||
-rw-r--r-- | src/mem/dram_ctrl.hh | 8 | ||||
-rw-r--r-- | src/mem/dramsim2.cc | 13 | ||||
-rw-r--r-- | src/mem/dramsim2.hh | 8 | ||||
-rw-r--r-- | src/mem/simple_mem.cc | 10 | ||||
-rw-r--r-- | src/mem/simple_mem.hh | 8 | ||||
-rw-r--r-- | src/mem/tport.cc | 12 | ||||
-rw-r--r-- | src/mem/tport.hh | 8 |
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: |