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>

7 files changed, 38 insertions, 75 deletions

diff --git a/src/cpu/o3/cpu.hh b/src/cpu/o3/cpu.hh
index 10af087d1..1589220a9 100644
--- a/src/cpu/o3/cpu.hh
+++ b/src/cpu/o3/cpu.hh
@@ -744,14 +744,16 @@ class FullO3CPU : public BaseO3CPU
     std::vector<ThreadID> tids;
 
     /** CPU read function, forwards read to LSQ. */
-    Fault read(RequestPtr &req, RequestPtr &sreqLow, RequestPtr &sreqHigh,
+    Fault read(const RequestPtr &req,
+               RequestPtr &sreqLow, RequestPtr &sreqHigh,
                int load_idx)
     {
         return this->iew.ldstQueue.read(req, sreqLow, sreqHigh, load_idx);
     }
 
     /** CPU write function, forwards write to LSQ. */
-    Fault write(RequestPtr &req, RequestPtr &sreqLow, RequestPtr &sreqHigh,
+    Fault write(const RequestPtr &req,
+                const RequestPtr &sreqLow, const RequestPtr &sreqHigh,
                 uint8_t *data, int store_idx)
     {
         return this->iew.ldstQueue.write(req, sreqLow, sreqHigh,
diff --git a/src/cpu/o3/fetch.hh b/src/cpu/o3/fetch.hh
index 4382197f4..da7ba4bb3 100644
--- a/src/cpu/o3/fetch.hh
+++ b/src/cpu/o3/fetch.hh
@@ -99,7 +99,7 @@ class DefaultFetch
         {}
 
         void
-        finish(const Fault &fault, RequestPtr req, ThreadContext *tc,
+        finish(const Fault &fault, const RequestPtr &req, ThreadContext *tc,
                BaseTLB::Mode mode)
         {
             assert(mode == BaseTLB::Execute);
@@ -129,7 +129,7 @@ class DefaultFetch
             fault = _fault;
         }
 
-        void setReq(RequestPtr _req)
+        void setReq(const RequestPtr &_req)
         {
             req = _req;
         }
@@ -295,7 +295,7 @@ class DefaultFetch
      * @return Any fault that occured.
      */
     bool fetchCacheLine(Addr vaddr, ThreadID tid, Addr pc);
-    void finishTranslation(const Fault &fault, RequestPtr mem_req);
+    void finishTranslation(const Fault &fault, const RequestPtr &mem_req);
 
 
     /** Check if an interrupt is pending and that we need to handle
diff --git a/src/cpu/o3/fetch_impl.hh b/src/cpu/o3/fetch_impl.hh
index 2e8ec67ae..2df7b84ee 100644
--- a/src/cpu/o3/fetch_impl.hh
+++ b/src/cpu/o3/fetch_impl.hh
@@ -388,7 +388,6 @@ DefaultFetch<Impl>::processCacheCompletion(PacketPtr pkt)
     if (fetchStatus[tid] != IcacheWaitResponse ||
         pkt->req != memReq[tid]) {
         ++fetchIcacheSquashes;
-        delete pkt->req;
         delete pkt;
         return;
     }
@@ -415,7 +414,6 @@ DefaultFetch<Impl>::processCacheCompletion(PacketPtr pkt)
     pkt->req->setAccessLatency();
     cpu->ppInstAccessComplete->notify(pkt);
     // Reset the mem req to NULL.
-    delete pkt->req;
     delete pkt;
     memReq[tid] = NULL;
 }
@@ -621,10 +619,10 @@ DefaultFetch<Impl>::fetchCacheLine(Addr vaddr, ThreadID tid, Addr pc)
     // Setup the memReq to do a read of the first instruction's address.
     // Set the appropriate read size and flags as well.
     // Build request here.
-    RequestPtr mem_req =
-        new Request(tid, fetchBufferBlockPC, fetchBufferSize,
-                    Request::INST_FETCH, cpu->instMasterId(), pc,
-                    cpu->thread[tid]->contextId());
+    RequestPtr mem_req = std::make_shared<Request>(
+        tid, fetchBufferBlockPC, fetchBufferSize,
+        Request::INST_FETCH, cpu->instMasterId(), pc,
+        cpu->thread[tid]->contextId());
 
     mem_req->taskId(cpu->taskId());
 
@@ -640,7 +638,8 @@ DefaultFetch<Impl>::fetchCacheLine(Addr vaddr, ThreadID tid, Addr pc)
 
 template <class Impl>
 void
-DefaultFetch<Impl>::finishTranslation(const Fault &fault, RequestPtr mem_req)
+DefaultFetch<Impl>::finishTranslation(const Fault &fault,
+                                      const RequestPtr &mem_req)
 {
     ThreadID tid = cpu->contextToThread(mem_req->contextId());
     Addr fetchBufferBlockPC = mem_req->getVaddr();
@@ -655,7 +654,6 @@ DefaultFetch<Impl>::finishTranslation(const Fault &fault, RequestPtr mem_req)
         DPRINTF(Fetch, "[tid:%i] Ignoring itlb completed after squash\n",
                 tid);
         ++fetchTlbSquashes;
-        delete mem_req;
         return;
     }
 
@@ -669,7 +667,6 @@ DefaultFetch<Impl>::finishTranslation(const Fault &fault, RequestPtr mem_req)
             warn("Address %#x is outside of physical memory, stopping fetch\n",
                     mem_req->getPaddr());
             fetchStatus[tid] = NoGoodAddr;
-            delete mem_req;
             memReq[tid] = NULL;
             return;
         }
@@ -717,7 +714,6 @@ DefaultFetch<Impl>::finishTranslation(const Fault &fault, RequestPtr mem_req)
         DPRINTF(Fetch, "[tid:%i] Got back req with addr %#x but expected %#x\n",
                 tid, mem_req->getVaddr(), memReq[tid]->getVaddr());
         // Translation faulted, icache request won't be sent.
-        delete mem_req;
         memReq[tid] = NULL;
 
         // Send the fault to commit.  This thread will not do anything
@@ -778,7 +774,6 @@ DefaultFetch<Impl>::doSquash(const TheISA::PCState &newPC,
     if (retryTid == tid) {
         assert(cacheBlocked);
         if (retryPkt) {
-            delete retryPkt->req;
             delete retryPkt;
         }
         retryPkt = NULL;
diff --git a/src/cpu/o3/lsq.hh b/src/cpu/o3/lsq.hh
index 6bc9b3d73..7c78156d5 100644
--- a/src/cpu/o3/lsq.hh
+++ b/src/cpu/o3/lsq.hh
@@ -274,13 +274,15 @@ class LSQ {
     /** Executes a read operation, using the load specified at the load
      * index.
      */
-    Fault read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
+    Fault read(const RequestPtr &req,
+               RequestPtr &sreqLow, RequestPtr &sreqHigh,
                int load_idx);
 
     /** Executes a store operation, using the store specified at the store
      * index.
      */
-    Fault write(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
+    Fault write(const RequestPtr &req,
+                const RequestPtr &sreqLow, const RequestPtr &sreqHigh,
                 uint8_t *data, int store_idx);
 
     /**
@@ -331,7 +333,8 @@ class LSQ {
 
 template <class Impl>
 Fault
-LSQ<Impl>::read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
+LSQ<Impl>::read(const RequestPtr &req,
+                RequestPtr &sreqLow, RequestPtr &sreqHigh,
                 int load_idx)
 {
     ThreadID tid = cpu->contextToThread(req->contextId());
@@ -341,7 +344,8 @@ LSQ<Impl>::read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
 
 template <class Impl>
 Fault
-LSQ<Impl>::write(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
+LSQ<Impl>::write(const RequestPtr &req,
+                 const RequestPtr &sreqLow, const RequestPtr &sreqHigh,
                  uint8_t *data, int store_idx)
 {
     ThreadID tid = cpu->contextToThread(req->contextId());
diff --git a/src/cpu/o3/lsq_impl.hh b/src/cpu/o3/lsq_impl.hh
index 9080907fe..56b95a5b6 100644
--- a/src/cpu/o3/lsq_impl.hh
+++ b/src/cpu/o3/lsq_impl.hh
@@ -370,7 +370,6 @@ LSQ<Impl>::recvTimingResp(PacketPtr pkt)
         }
     }
 
-    delete pkt->req;
     delete pkt;
     return true;
 }
diff --git a/src/cpu/o3/lsq_unit.hh b/src/cpu/o3/lsq_unit.hh
index a7a095c82..f5b60b2fc 100644
--- a/src/cpu/o3/lsq_unit.hh
+++ b/src/cpu/o3/lsq_unit.hh
@@ -510,11 +510,13 @@ class LSQUnit {
 
   public:
     /** Executes the load at the given index. */
-    Fault read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
+    Fault read(const RequestPtr &req,
+               RequestPtr &sreqLow, RequestPtr &sreqHigh,
                int load_idx);
 
     /** Executes the store at the given index. */
-    Fault write(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
+    Fault write(const RequestPtr &req,
+                const RequestPtr &sreqLow, const RequestPtr &sreqHigh,
                 uint8_t *data, int store_idx);
 
     /** Returns the index of the head load instruction. */
@@ -549,7 +551,8 @@ class LSQUnit {
 
 template <class Impl>
 Fault
-LSQUnit<Impl>::read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
+LSQUnit<Impl>::read(const RequestPtr &req,
+                    RequestPtr &sreqLow, RequestPtr &sreqHigh,
                     int load_idx)
 {
     DynInstPtr load_inst = loadQueue[load_idx];
@@ -569,14 +572,6 @@ LSQUnit<Impl>::read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
         DPRINTF(LSQUnit, "Strictly ordered load [sn:%lli] PC %s\n",
                 load_inst->seqNum, load_inst->pcState());
 
-        // Must delete request now that it wasn't handed off to
-        // memory.  This is quite ugly.  @todo: Figure out the proper
-        // place to really handle request deletes.
-        delete req;
-        if (TheISA::HasUnalignedMemAcc && sreqLow) {
-            delete sreqLow;
-            delete sreqHigh;
-        }
         return std::make_shared<GenericISA::M5PanicFault>(
             "Strictly ordered load [sn:%llx] PC %s\n",
             load_inst->seqNum, load_inst->pcState());
@@ -626,8 +621,6 @@ LSQUnit<Impl>::read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
             if (delay2 > delay)
                 delay = delay2;
 
-            delete sreqLow;
-            delete sreqHigh;
             delete fst_data_pkt;
             delete snd_data_pkt;
         }
@@ -704,12 +697,6 @@ LSQUnit<Impl>::read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
             // @todo: Need to make this a parameter.
             cpu->schedule(wb, curTick());
 
-            // Don't need to do anything special for split loads.
-            if (TheISA::HasUnalignedMemAcc && sreqLow) {
-                delete sreqLow;
-                delete sreqHigh;
-            }
-
             ++lsqForwLoads;
             return NoFault;
         } else if (
@@ -755,15 +742,6 @@ LSQUnit<Impl>::read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
                     "Store idx %i to load addr %#x\n",
                     store_idx, req->getVaddr());
 
-            // Must delete request now that it wasn't handed off to
-            // memory.  This is quite ugly.  @todo: Figure out the
-            // proper place to really handle request deletes.
-            delete req;
-            if (TheISA::HasUnalignedMemAcc && sreqLow) {
-                delete sreqLow;
-                delete sreqHigh;
-            }
-
             return NoFault;
         }
     }
@@ -843,7 +821,6 @@ LSQUnit<Impl>::read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
         if (!sreqLow) {
             // Packet wasn't split, just delete main packet info
             delete state;
-            delete req;
             delete data_pkt;
         }
 
@@ -851,22 +828,17 @@ LSQUnit<Impl>::read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
             if (!completedFirst) {
                 // Split packet, but first failed.  Delete all state.
                 delete state;
-                delete req;
                 delete data_pkt;
                 delete fst_data_pkt;
                 delete snd_data_pkt;
-                delete sreqLow;
-                delete sreqHigh;
-                sreqLow = NULL;
-                sreqHigh = NULL;
+                sreqLow.reset();
+                sreqHigh.reset();
             } else {
                 // Can't delete main packet data or state because first packet
                 // was sent to the memory system
                 delete data_pkt;
-                delete req;
-                delete sreqHigh;
                 delete snd_data_pkt;
-                sreqHigh = NULL;
+                sreqHigh.reset();
             }
         }
 
@@ -883,7 +855,8 @@ LSQUnit<Impl>::read(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
 
 template <class Impl>
 Fault
-LSQUnit<Impl>::write(RequestPtr req, RequestPtr sreqLow, RequestPtr sreqHigh,
+LSQUnit<Impl>::write(const RequestPtr &req,
+                     const RequestPtr &sreqLow, const RequestPtr &sreqHigh,
                      uint8_t *data, int store_idx)
 {
     assert(storeQueue[store_idx].inst);
diff --git a/src/cpu/o3/lsq_unit_impl.hh b/src/cpu/o3/lsq_unit_impl.hh
index e8e2c1853..c2750be7d 100644
--- a/src/cpu/o3/lsq_unit_impl.hh
+++ b/src/cpu/o3/lsq_unit_impl.hh
@@ -79,7 +79,6 @@ LSQUnit<Impl>::WritebackEvent::process()
     if (pkt->senderState)
         delete pkt->senderState;
 
-    delete pkt->req;
     delete pkt;
 }
 
@@ -133,7 +132,6 @@ LSQUnit<Impl>::completeDataAccess(PacketPtr pkt)
     }
 
     if (TheISA::HasUnalignedMemAcc && state->isSplit && state->isLoad) {
-        delete state->mainPkt->req;
         delete state->mainPkt;
     }
 
@@ -831,9 +829,9 @@ LSQUnit<Impl>::writebackStores()
 
         DynInstPtr inst = storeQueue[storeWBIdx].inst;
 
-        RequestPtr req = storeQueue[storeWBIdx].req;
-        RequestPtr sreqLow = storeQueue[storeWBIdx].sreqLow;
-        RequestPtr sreqHigh = storeQueue[storeWBIdx].sreqHigh;
+        RequestPtr &req = storeQueue[storeWBIdx].req;
+        const RequestPtr &sreqLow = storeQueue[storeWBIdx].sreqLow;
+        const RequestPtr &sreqHigh = storeQueue[storeWBIdx].sreqHigh;
 
         storeQueue[storeWBIdx].committed = true;
 
@@ -874,7 +872,6 @@ LSQUnit<Impl>::writebackStores()
             state->outstanding = 2;
 
             // Can delete the main request now.
-            delete req;
             req = sreqLow;
         }
 
@@ -923,11 +920,8 @@ LSQUnit<Impl>::writebackStores()
                 assert(snd_data_pkt->req->isMmappedIpr());
                 TheISA::handleIprWrite(thread, snd_data_pkt);
                 delete snd_data_pkt;
-                delete sreqLow;
-                delete sreqHigh;
             }
             delete state;
-            delete req;
             completeStore(storeWBIdx);
             incrStIdx(storeWBIdx);
         } else if (!sendStore(data_pkt)) {
@@ -1061,16 +1055,12 @@ LSQUnit<Impl>::squash(const InstSeqNum &squashed_num)
         // Must delete request now that it wasn't handed off to
         // memory.  This is quite ugly.  @todo: Figure out the proper
         // place to really handle request deletes.
-        delete storeQueue[store_idx].req;
+        storeQueue[store_idx].req.reset();
         if (TheISA::HasUnalignedMemAcc && storeQueue[store_idx].isSplit) {
-            delete storeQueue[store_idx].sreqLow;
-            delete storeQueue[store_idx].sreqHigh;
-
-            storeQueue[store_idx].sreqLow = NULL;
-            storeQueue[store_idx].sreqHigh = NULL;
+            storeQueue[store_idx].sreqLow.reset();
+            storeQueue[store_idx].sreqHigh.reset();
         }
 
-        storeQueue[store_idx].req = NULL;
         --stores;
 
         // Inefficient!