misc: Using smart pointers for memory Requests

This patch is changing the underlying type for RequestPtr from Request*
to shared_ptr<Request>. 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 <giacomo.travaglini@arm.com>
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/10996
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Maintainer: Nikos Nikoleris <nikos.nikoleris@arm.com>

6 files changed, 24 insertions, 21 deletions

diff --git a/src/mem/ruby/slicc_interface/AbstractController.cc b/src/mem/ruby/slicc_interface/AbstractController.cc
index de5e81057..5f7eb6558 100644
--- a/src/mem/ruby/slicc_interface/AbstractController.cc
+++ b/src/mem/ruby/slicc_interface/AbstractController.cc
@@ -240,8 +240,8 @@ void
 AbstractController::queueMemoryRead(const MachineID &id, Addr addr,
                                     Cycles latency)
 {
-    RequestPtr req = new Request(addr, RubySystem::getBlockSizeBytes(), 0,
-                                 m_masterId);
+    RequestPtr req = std::make_shared<Request>(
+        addr, RubySystem::getBlockSizeBytes(), 0, m_masterId);
 
     PacketPtr pkt = Packet::createRead(req);
     uint8_t *newData = new uint8_t[RubySystem::getBlockSizeBytes()];
@@ -264,8 +264,8 @@ void
 AbstractController::queueMemoryWrite(const MachineID &id, Addr addr,
                                      Cycles latency, const DataBlock &block)
 {
-    RequestPtr req = new Request(addr, RubySystem::getBlockSizeBytes(), 0,
-                                 m_masterId);
+    RequestPtr req = std::make_shared<Request>(
+        addr, RubySystem::getBlockSizeBytes(), 0, m_masterId);
 
     PacketPtr pkt = Packet::createWrite(req);
     uint8_t *newData = new uint8_t[RubySystem::getBlockSizeBytes()];
@@ -292,7 +292,7 @@ AbstractController::queueMemoryWritePartial(const MachineID &id, Addr addr,
                                             Cycles latency,
                                             const DataBlock &block, int size)
 {
-    RequestPtr req = new Request(addr, size, 0, m_masterId);
+    RequestPtr req = std::make_shared<Request>(addr, size, 0, m_masterId);
 
     PacketPtr pkt = Packet::createWrite(req);
     uint8_t *newData = new uint8_t[size];
@@ -356,7 +356,6 @@ AbstractController::recvTimingResp(PacketPtr pkt)
     }
 
     getMemoryQueue()->enqueue(msg, clockEdge(), cyclesToTicks(Cycles(1)));
-    delete pkt->req;
     delete pkt;
 }
 
diff --git a/src/mem/ruby/system/CacheRecorder.cc b/src/mem/ruby/system/CacheRecorder.cc
index 83e8a70dd..1fc7bb8ef 100644
--- a/src/mem/ruby/system/CacheRecorder.cc
+++ b/src/mem/ruby/system/CacheRecorder.cc
@@ -85,9 +85,9 @@ CacheRecorder::enqueueNextFlushRequest()
     if (m_records_flushed < m_records.size()) {
         TraceRecord* rec = m_records[m_records_flushed];
         m_records_flushed++;
-        Request* req = new Request(rec->m_data_address,
-                                   m_block_size_bytes, 0,
-                                   Request::funcMasterId);
+        auto req = std::make_shared<Request>(rec->m_data_address,
+                                             m_block_size_bytes, 0,
+                                             Request::funcMasterId);
         MemCmd::Command requestType = MemCmd::FlushReq;
         Packet *pkt = new Packet(req, requestType);
 
@@ -112,21 +112,24 @@ CacheRecorder::enqueueNextFetchRequest()
 
         for (int rec_bytes_read = 0; rec_bytes_read < m_block_size_bytes;
                 rec_bytes_read += RubySystem::getBlockSizeBytes()) {
-            Request* req = nullptr;
+            RequestPtr req;
             MemCmd::Command requestType;
 
             if (traceRecord->m_type == RubyRequestType_LD) {
                 requestType = MemCmd::ReadReq;
-                req = new Request(traceRecord->m_data_address + rec_bytes_read,
+                req = std::make_shared<Request>(
+                    traceRecord->m_data_address + rec_bytes_read,
                     RubySystem::getBlockSizeBytes(), 0, Request::funcMasterId);
             }   else if (traceRecord->m_type == RubyRequestType_IFETCH) {
                 requestType = MemCmd::ReadReq;
-                req = new Request(traceRecord->m_data_address + rec_bytes_read,
+                req = std::make_shared<Request>(
+                        traceRecord->m_data_address + rec_bytes_read,
                         RubySystem::getBlockSizeBytes(),
                         Request::INST_FETCH, Request::funcMasterId);
             }   else {
                 requestType = MemCmd::WriteReq;
-                req = new Request(traceRecord->m_data_address + rec_bytes_read,
+                req = std::make_shared<Request>(
+                    traceRecord->m_data_address + rec_bytes_read,
                     RubySystem::getBlockSizeBytes(), 0, Request::funcMasterId);
             }
 
diff --git a/src/mem/ruby/system/GPUCoalescer.cc b/src/mem/ruby/system/GPUCoalescer.cc
index ef726be3d..e17bf63fd 100644
--- a/src/mem/ruby/system/GPUCoalescer.cc
+++ b/src/mem/ruby/system/GPUCoalescer.cc
@@ -70,7 +70,7 @@ RubyGPUCoalescerParams::create()
 }
 
 HSAScope
-reqScopeToHSAScope(Request* req)
+reqScopeToHSAScope(const RequestPtr &req)
 {
     HSAScope accessScope = HSAScope_UNSPECIFIED;
     if (req->isScoped()) {
@@ -90,7 +90,7 @@ reqScopeToHSAScope(Request* req)
 }
 
 HSASegment
-reqSegmentToHSASegment(Request* req)
+reqSegmentToHSASegment(const RequestPtr &req)
 {
     HSASegment accessSegment = HSASegment_GLOBAL;
 
diff --git a/src/mem/ruby/system/GPUCoalescer.hh b/src/mem/ruby/system/GPUCoalescer.hh
index cf2005046..6576ecb36 100644
--- a/src/mem/ruby/system/GPUCoalescer.hh
+++ b/src/mem/ruby/system/GPUCoalescer.hh
@@ -58,8 +58,8 @@ class CacheMemory;
 
 class RubyGPUCoalescerParams;
 
-HSAScope reqScopeToHSAScope(Request* req);
-HSASegment reqSegmentToHSASegment(Request* req);
+HSAScope reqScopeToHSAScope(const RequestPtr &req);
+HSASegment reqSegmentToHSASegment(const RequestPtr &req);
 
 struct GPUCoalescerRequest
 {
diff --git a/src/mem/ruby/system/RubyPort.cc b/src/mem/ruby/system/RubyPort.cc
index 02d23790a..84a70c0f1 100644
--- a/src/mem/ruby/system/RubyPort.cc
+++ b/src/mem/ruby/system/RubyPort.cc
@@ -608,11 +608,13 @@ RubyPort::ruby_eviction_callback(Addr address)
     // Allocate the invalidate request and packet on the stack, as it is
     // assumed they will not be modified or deleted by receivers.
     // TODO: should this really be using funcMasterId?
-    Request request(address, RubySystem::getBlockSizeBytes(), 0,
-                    Request::funcMasterId);
+    auto request = std::make_shared<Request>(
+        address, RubySystem::getBlockSizeBytes(), 0,
+        Request::funcMasterId);
+
     // Use a single packet to signal all snooping ports of the invalidation.
     // This assumes that snooping ports do NOT modify the packet/request
-    Packet pkt(&request, MemCmd::InvalidateReq);
+    Packet pkt(request, MemCmd::InvalidateReq);
     for (CpuPortIter p = slave_ports.begin(); p != slave_ports.end(); ++p) {
         // check if the connected master port is snooping
         if ((*p)->isSnooping()) {
diff --git a/src/mem/ruby/system/Sequencer.cc b/src/mem/ruby/system/Sequencer.cc
index 4037fb8f1..f30369710 100644
--- a/src/mem/ruby/system/Sequencer.cc
+++ b/src/mem/ruby/system/Sequencer.cc
@@ -516,7 +516,6 @@ Sequencer::hitCallback(SequencerRequest* srequest, DataBlock& data,
     RubySystem *rs = m_ruby_system;
     if (RubySystem::getWarmupEnabled()) {
         assert(pkt->req);
-        delete pkt->req;
         delete pkt;
         rs->m_cache_recorder->enqueueNextFetchRequest();
     } else if (RubySystem::getCooldownEnabled()) {