cpu, mem: Changing AtomicOpFunctor* for unique_ptr<AtomicOpFunctor>

This change is based on modify the way we move the AtomicOpFunctor*
through gem5 in order to mantain proper ownership of the object and
ensuring its destruction when it is no longer used.

Doing that we fix at the same time a memory leak in Request.hh
where we were assigning a new AtomicOpFunctor* without destroying the
previous one.

This change creates a new type AtomicOpFunctor_ptr as a
std::unique_ptr<AtomicOpFunctor> and move its ownership as needed. Except
for its only usage when AtomicOpFunc() is called.

Change-Id: Ic516f9d8217cb1ae1f0a19500e5da0336da9fd4f
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/20919
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>
Tested-by: kokoro <noreply+kokoro@google.com>

3 files changed, 12 insertions, 11 deletions

diff --git a/src/cpu/o3/cpu.hh b/src/cpu/o3/cpu.hh
index ac917dba9..b06182d43 100644
--- a/src/cpu/o3/cpu.hh
+++ b/src/cpu/o3/cpu.hh
@@ -713,13 +713,13 @@ class FullO3CPU : public BaseO3CPU
     /** CPU pushRequest function, forwards request to LSQ. */
     Fault pushRequest(const DynInstPtr& inst, bool isLoad, uint8_t *data,
                       unsigned int size, Addr addr, Request::Flags flags,
-                      uint64_t *res, AtomicOpFunctor *amo_op = nullptr,
+                      uint64_t *res, AtomicOpFunctorPtr amo_op = nullptr,
                       const std::vector<bool>& byteEnable =
                           std::vector<bool>())
 
     {
         return iew.ldstQueue.pushRequest(inst, isLoad, data, size, addr,
-                flags, res, amo_op, byteEnable);
+                flags, res, std::move(amo_op), byteEnable);
     }
 
     /** CPU read function, forwards read to LSQ. */
diff --git a/src/cpu/o3/lsq.hh b/src/cpu/o3/lsq.hh
index cc14ae423..6225c507d 100644
--- a/src/cpu/o3/lsq.hh
+++ b/src/cpu/o3/lsq.hh
@@ -299,7 +299,7 @@ class LSQ
         const Request::Flags _flags;
         std::vector<bool> _byteEnable;
         uint32_t _numOutstandingPackets;
-        AtomicOpFunctor *_amo_op;
+        AtomicOpFunctorPtr _amo_op;
       protected:
         LSQUnit* lsqUnit() { return &_port; }
         LSQRequest(LSQUnit* port, const DynInstPtr& inst, bool isLoad) :
@@ -318,7 +318,7 @@ class LSQ
                    const Addr& addr, const uint32_t& size,
                    const Request::Flags& flags_,
                    PacketDataPtr data = nullptr, uint64_t* res = nullptr,
-                   AtomicOpFunctor* amo_op = nullptr)
+                   AtomicOpFunctorPtr amo_op = nullptr)
             : _state(State::NotIssued), _senderState(nullptr),
             numTranslatedFragments(0),
             numInTranslationFragments(0),
@@ -326,7 +326,7 @@ class LSQ
             _res(res), _addr(addr), _size(size),
             _flags(flags_),
             _numOutstandingPackets(0),
-            _amo_op(amo_op)
+            _amo_op(std::move(amo_op))
         {
             flags.set(Flag::IsLoad, isLoad);
             flags.set(Flag::WbStore,
@@ -412,7 +412,8 @@ class LSQ
                 isAnyActiveElement(byteEnable.begin(), byteEnable.end())) {
                 auto request = std::make_shared<Request>(_inst->getASID(),
                         addr, size, _flags, _inst->masterId(),
-                        _inst->instAddr(), _inst->contextId(), _amo_op);
+                        _inst->instAddr(), _inst->contextId(),
+                        std::move(_amo_op));
                 if (!byteEnable.empty()) {
                     request->setByteEnable(byteEnable);
                 }
@@ -721,9 +722,9 @@ class LSQ
                           const Request::Flags& flags_,
                           PacketDataPtr data = nullptr,
                           uint64_t* res = nullptr,
-                          AtomicOpFunctor* amo_op = nullptr) :
+                          AtomicOpFunctorPtr amo_op = nullptr) :
             LSQRequest(port, inst, isLoad, addr, size, flags_, data, res,
-                       amo_op) {}
+                       std::move(amo_op)) {}
 
         inline virtual ~SingleDataRequest() {}
         virtual void initiateTranslation();
@@ -1032,7 +1033,7 @@ class LSQ
 
     Fault pushRequest(const DynInstPtr& inst, bool isLoad, uint8_t *data,
                       unsigned int size, Addr addr, Request::Flags flags,
-                      uint64_t *res, AtomicOpFunctor *amo_op,
+                      uint64_t *res, AtomicOpFunctorPtr amo_op,
                       const std::vector<bool>& byteEnable);
 
     /** The CPU pointer. */
diff --git a/src/cpu/o3/lsq_impl.hh b/src/cpu/o3/lsq_impl.hh
index e885e6172..c2d5e90b4 100644
--- a/src/cpu/o3/lsq_impl.hh
+++ b/src/cpu/o3/lsq_impl.hh
@@ -687,7 +687,7 @@ template<class Impl>
 Fault
 LSQ<Impl>::pushRequest(const DynInstPtr& inst, bool isLoad, uint8_t *data,
                        unsigned int size, Addr addr, Request::Flags flags,
-                       uint64_t *res, AtomicOpFunctor *amo_op,
+                       uint64_t *res, AtomicOpFunctorPtr amo_op,
                        const std::vector<bool>& byteEnable)
 {
     // This comming request can be either load, store or atomic.
@@ -717,7 +717,7 @@ LSQ<Impl>::pushRequest(const DynInstPtr& inst, bool isLoad, uint8_t *data,
                     size, flags, data, res);
         } else {
             req = new SingleDataRequest(&thread[tid], inst, isLoad, addr,
-                    size, flags, data, res, amo_op);
+                    size, flags, data, res, std::move(amo_op));
         }
         assert(req);
         if (!byteEnable.empty()) {