From f54020eb8155371725ab75b0fc5c419287eca084 Mon Sep 17 00:00:00 2001 From: Giacomo Travaglini Date: Mon, 4 Jun 2018 09:40:19 +0100 Subject: misc: Using smart pointers for memory Requests This patch is changing the underlying type for RequestPtr from Request* to shared_ptr. Having memory requests being managed by smart pointers will simplify the code; it will also prevent memory leakage and dangling pointers. Change-Id: I7749af38a11ac8eb4d53d8df1252951e0890fde3 Signed-off-by: Giacomo Travaglini Reviewed-by: Andreas Sandberg Reviewed-on: https://gem5-review.googlesource.com/10996 Reviewed-by: Nikos Nikoleris Maintainer: Nikos Nikoleris --- src/mem/cache/base.cc | 22 ++++++++++++---------- src/mem/cache/blk.hh | 10 +++++----- src/mem/cache/cache.cc | 26 +++++++------------------- src/mem/cache/mshr.cc | 3 ++- src/mem/cache/noncoherent_cache.cc | 1 - src/mem/cache/prefetch/queued.cc | 5 +---- 6 files changed, 27 insertions(+), 40 deletions(-) (limited to 'src/mem/cache') diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc index f753cc315..a5ad07d6c 100644 --- a/src/mem/cache/base.cc +++ b/src/mem/cache/base.cc @@ -810,7 +810,6 @@ BaseCache::getNextQueueEntry() return allocateMissBuffer(pkt, curTick(), false); } else { // free the request and packet - delete pkt->req; delete pkt; } } @@ -1278,8 +1277,9 @@ BaseCache::writebackBlk(CacheBlk *blk) writebacks[Request::wbMasterId]++; - RequestPtr req = new Request(regenerateBlkAddr(blk), blkSize, 0, - Request::wbMasterId); + RequestPtr req = std::make_shared( + regenerateBlkAddr(blk), blkSize, 0, Request::wbMasterId); + if (blk->isSecure()) req->setFlags(Request::SECURE); @@ -1313,8 +1313,9 @@ BaseCache::writebackBlk(CacheBlk *blk) PacketPtr BaseCache::writecleanBlk(CacheBlk *blk, Request::Flags dest, PacketId id) { - RequestPtr req = new Request(regenerateBlkAddr(blk), blkSize, 0, - Request::wbMasterId); + RequestPtr req = std::make_shared( + regenerateBlkAddr(blk), blkSize, 0, Request::wbMasterId); + if (blk->isSecure()) { req->setFlags(Request::SECURE); } @@ -1373,14 +1374,15 @@ BaseCache::writebackVisitor(CacheBlk &blk) if (blk.isDirty()) { assert(blk.isValid()); - Request request(regenerateBlkAddr(&blk), - blkSize, 0, Request::funcMasterId); - request.taskId(blk.task_id); + RequestPtr request = std::make_shared( + regenerateBlkAddr(&blk), blkSize, 0, Request::funcMasterId); + + request->taskId(blk.task_id); if (blk.isSecure()) { - request.setFlags(Request::SECURE); + request->setFlags(Request::SECURE); } - Packet packet(&request, MemCmd::WriteReq); + Packet packet(request, MemCmd::WriteReq); packet.dataStatic(blk.data); memSidePort.sendFunctional(&packet); diff --git a/src/mem/cache/blk.hh b/src/mem/cache/blk.hh index c4ec12ff3..93189bd97 100644 --- a/src/mem/cache/blk.hh +++ b/src/mem/cache/blk.hh @@ -136,7 +136,7 @@ class CacheBlk : public ReplaceableEntry // check for matching execution context, and an address that // is within the lock - bool matches(const RequestPtr req) const + bool matches(const RequestPtr &req) const { Addr req_low = req->getPaddr(); Addr req_high = req_low + req->getSize() -1; @@ -145,7 +145,7 @@ class CacheBlk : public ReplaceableEntry } // check if a request is intersecting and thus invalidating the lock - bool intersects(const RequestPtr req) const + bool intersects(const RequestPtr &req) const { Addr req_low = req->getPaddr(); Addr req_high = req_low + req->getSize() - 1; @@ -153,7 +153,7 @@ class CacheBlk : public ReplaceableEntry return (req_low <= highAddr) && (req_high >= lowAddr); } - Lock(const RequestPtr req) + Lock(const RequestPtr &req) : contextId(req->contextId()), lowAddr(req->getPaddr()), highAddr(lowAddr + req->getSize() - 1) @@ -285,7 +285,7 @@ class CacheBlk : public ReplaceableEntry * Clear the any load lock that intersect the request, and is from * a different context. */ - void clearLoadLocks(RequestPtr req) + void clearLoadLocks(const RequestPtr &req) { auto l = lockList.begin(); while (l != lockList.end()) { @@ -357,7 +357,7 @@ class CacheBlk : public ReplaceableEntry if (!pkt->isLLSC() && lockList.empty()) return true; - RequestPtr req = pkt->req; + const RequestPtr &req = pkt->req; if (pkt->isLLSC()) { // it's a store conditional... have to check for matching diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc index 86c1640e5..ffd60811e 100644 --- a/src/mem/cache/cache.cc +++ b/src/mem/cache/cache.cc @@ -377,10 +377,10 @@ Cache::handleTimingReqMiss(PacketPtr pkt, CacheBlk *blk, Tick forward_time, if (!mshr) { // copy the request and create a new SoftPFReq packet - RequestPtr req = new Request(pkt->req->getPaddr(), - pkt->req->getSize(), - pkt->req->getFlags(), - pkt->req->masterId()); + RequestPtr req = std::make_shared(pkt->req->getPaddr(), + pkt->req->getSize(), + pkt->req->getFlags(), + pkt->req->masterId()); pf = new Packet(req, pkt->cmd); pf->allocate(); assert(pf->getAddr() == pkt->getAddr()); @@ -696,7 +696,6 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk, // immediately with dummy data so the core would be able to // retire it. This request completes right here, so we // deallocate it. - delete tgt_pkt->req; delete tgt_pkt; break; // skip response } @@ -803,7 +802,6 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk, assert(tgt_pkt->cmd == MemCmd::HardPFReq); if (blk) blk->status |= BlkHWPrefetched; - delete tgt_pkt->req; delete tgt_pkt; break; @@ -871,11 +869,11 @@ Cache::cleanEvictBlk(CacheBlk *blk) { assert(!writebackClean); assert(blk && blk->isValid() && !blk->isDirty()); + // Creating a zero sized write, a message to the snoop filter + RequestPtr req = std::make_shared( + regenerateBlkAddr(blk), blkSize, 0, Request::wbMasterId); - RequestPtr req = - new Request(regenerateBlkAddr(blk), blkSize, 0, - Request::wbMasterId); if (blk->isSecure()) req->setFlags(Request::SECURE); @@ -1138,15 +1136,6 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing, if (!respond && is_deferred) { assert(pkt->needsResponse()); - - // if we copied the deferred packet with the intention to - // respond, but are not responding, then a cache above us must - // be, and we can use this as the indication of whether this - // is a packet where we created a copy of the request or not - if (!pkt->cacheResponding()) { - delete pkt->req; - } - delete pkt; } @@ -1396,7 +1385,6 @@ Cache::sendMSHRQueuePacket(MSHR* mshr) } // given that no response is expected, delete Request and Packet - delete tgt_pkt->req; delete tgt_pkt; return false; diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc index dc490790b..21ce8a36d 100644 --- a/src/mem/cache/mshr.cc +++ b/src/mem/cache/mshr.cc @@ -424,7 +424,8 @@ MSHR::handleSnoop(PacketPtr pkt, Counter _order) // the packet and the request as part of handling the deferred // snoop. PacketPtr cp_pkt = will_respond ? new Packet(pkt, true, true) : - new Packet(new Request(*pkt->req), pkt->cmd, blkSize, pkt->id); + new Packet(std::make_shared(*pkt->req), pkt->cmd, + blkSize, pkt->id); if (will_respond) { // we are the ordering point, and will consequently diff --git a/src/mem/cache/noncoherent_cache.cc b/src/mem/cache/noncoherent_cache.cc index bf75be4d6..50738375e 100644 --- a/src/mem/cache/noncoherent_cache.cc +++ b/src/mem/cache/noncoherent_cache.cc @@ -299,7 +299,6 @@ NoncoherentCache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, // We have filled the block and the prefetcher does not // require responses. - delete tgt_pkt->req; delete tgt_pkt; break; diff --git a/src/mem/cache/prefetch/queued.cc b/src/mem/cache/prefetch/queued.cc index 00d62f17f..3c5647ae3 100644 --- a/src/mem/cache/prefetch/queued.cc +++ b/src/mem/cache/prefetch/queued.cc @@ -59,7 +59,6 @@ QueuedPrefetcher::~QueuedPrefetcher() { // Delete the queued prefetch packets for (DeferredPacket &p : pfq) { - delete p.pkt->req; delete p.pkt; } } @@ -78,7 +77,6 @@ QueuedPrefetcher::notify(const PacketPtr &pkt) while (itr != pfq.end()) { if (itr->pkt->getAddr() == blk_addr && itr->pkt->isSecure() == is_secure) { - delete itr->pkt->req; delete itr->pkt; itr = pfq.erase(itr); } else { @@ -224,7 +222,7 @@ QueuedPrefetcher::insert(AddrPriority &pf_info, bool is_secure) /* Create a prefetch memory request */ RequestPtr pf_req = - new Request(pf_info.first, blkSize, 0, masterId); + std::make_shared(pf_info.first, blkSize, 0, masterId); if (is_secure) { pf_req->setFlags(Request::SECURE); @@ -255,7 +253,6 @@ QueuedPrefetcher::insert(AddrPriority &pf_info, bool is_secure) } DPRINTF(HWPrefetch, "Prefetch queue full, removing lowest priority " "oldest packet, addr: %#x", it->pkt->getAddr()); - delete it->pkt->req; delete it->pkt; pfq.erase(it); } -- cgit v1.2.3