Tom Rollet has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/51067 )

Change subject: cpu-o3: Naming cleanup for LSQRequest and Request
......................................................................

cpu-o3: Naming cleanup for LSQRequest and Request

'LSQRequest' are now referred as 'request'
'Request' are now referred as 'req'

It makes the code easier to read.
Also it makes the naming of Request consistent with the cache.

Change-Id: I8ba75b75bd8408e411300d522cc2c8582c334cf5
---
M src/cpu/o3/dyn_inst.hh
M src/cpu/o3/lsq.cc
M src/cpu/o3/lsq.hh
M src/cpu/o3/lsq_unit.cc
M src/cpu/o3/lsq_unit.hh
5 files changed, 234 insertions(+), 222 deletions(-)



diff --git a/src/cpu/o3/dyn_inst.hh b/src/cpu/o3/dyn_inst.hh
index f26ea22..45d7b12 100644
--- a/src/cpu/o3/dyn_inst.hh
+++ b/src/cpu/o3/dyn_inst.hh
@@ -400,7 +400,7 @@
      * Saved memory request (needed when the DTB address translation is
      * delayed due to a hw page table walk).
      */
-    LSQ::LSQRequest *savedReq;
+    LSQ::LSQRequest *savedRequest;

     /////////////////////// Checker //////////////////////
     // Need a copy of main request pointer to verify on writes.
diff --git a/src/cpu/o3/lsq.cc b/src/cpu/o3/lsq.cc
index 081fe32..27267b1 100644
--- a/src/cpu/o3/lsq.cc
+++ b/src/cpu/o3/lsq.cc
@@ -773,7 +773,7 @@
     ThreadID tid = cpu->contextToThread(inst->contextId());
     auto cacheLineSize = cpu->cacheLineSize();
     bool needs_burst = transferNeedsBurst(addr, size, cacheLineSize);
-    LSQRequest* req = nullptr;
+    LSQRequest* request = nullptr;

     // Atomic requests that access data across cache line boundary are
// currently not allowed since the cache does not guarantee corresponding
@@ -786,47 +786,47 @@
     const bool htm_cmd = isLoad && (flags & Request::HTM_CMD);

     if (inst->translationStarted()) {
-        req = inst->savedReq;
-        assert(req);
+        request = inst->savedRequest;
+        assert(request);
     } else {
         if (htm_cmd) {
             assert(addr == 0x0lu);
             assert(size == 8);
-            req = new HtmCmdRequest(&thread[tid], inst, flags);
+            request = new HtmCmdRequest(&thread[tid], inst, flags);
         } else if (needs_burst) {
-            req = new SplitDataRequest(&thread[tid], inst, isLoad, addr,
+ request = new SplitDataRequest(&thread[tid], inst, isLoad, addr,
                     size, flags, data, res);
         } else {
-            req = new SingleDataRequest(&thread[tid], inst, isLoad, addr,
+ request = new SingleDataRequest(&thread[tid], inst, isLoad, addr,
                     size, flags, data, res, std::move(amo_op));
         }
-        assert(req);
-        req->_byteEnable = byte_enable;
+        assert(request);
+        request->_byteEnable = byte_enable;
         inst->setRequest();
-        req->taskId(cpu->taskId());
+        request->taskId(cpu->taskId());

// There might be fault from a previous execution attempt if this is
         // a strictly ordered load
         inst->getFault() = NoFault;

-        req->initiateTranslation();
+        request->initiateTranslation();
     }

     /* This is the place were instructions get the effAddr. */
-    if (req->isTranslationComplete()) {
-        if (req->isMemAccessRequired()) {
-            inst->effAddr = req->getVaddr();
+    if (request->isTranslationComplete()) {
+        if (request->isMemAccessRequired()) {
+            inst->effAddr = request->getVaddr();
             inst->effSize = size;
             inst->effAddrValid(true);

             if (cpu->checker) {
- inst->reqToVerify = std::make_shared<Request>(*req->request()); + inst->reqToVerify = std::make_shared<Request>(*request->req());
             }
             Fault fault;
             if (isLoad)
-                fault = read(req, inst->lqIdx);
+                fault = read(request, inst->lqIdx);
             else
-                fault = write(req, data, inst->sqIdx);
+                fault = write(request, data, inst->sqIdx);
             // inst->getFault() may have the first-fault of a
             // multi-access split request at this point.
             // Overwrite that only if we got another type of fault
@@ -848,7 +848,7 @@
 }

 void
-LSQ::SingleDataRequest::finish(const Fault &fault, const RequestPtr &req,
+LSQ::SingleDataRequest::finish(const Fault &fault, const RequestPtr &request,
         gem5::ThreadContext* tc, BaseMMU::Mode mode)
 {
     _fault.push_back(fault);
@@ -859,15 +859,15 @@
     if (_inst->isSquashed()) {
         squashTranslation();
     } else {
-        _inst->strictlyOrdered(req->isStrictlyOrdered());
+        _inst->strictlyOrdered(request->isStrictlyOrdered());

         flags.set(Flag::TranslationFinished);
         if (fault == NoFault) {
-            _inst->physEffAddr = req->getPaddr();
-            _inst->memReqFlags = req->getFlags();
-            if (req->isCondSwap()) {
+            _inst->physEffAddr = request->getPaddr();
+            _inst->memReqFlags = request->getFlags();
+            if (request->isCondSwap()) {
                 assert(_res);
-                req->setExtraData(*_res);
+                request->setExtraData(*_res);
             }
             setState(State::Request);
         } else {
@@ -884,32 +884,32 @@
         gem5::ThreadContext* tc, BaseMMU::Mode mode)
 {
     int i;
-    for (i = 0; i < _requests.size() && _requests[i] != req; i++);
-    assert(i < _requests.size());
+    for (i = 0; i < _reqs.size() && _reqs[i] != req; i++);
+    assert(i < _reqs.size());
     _fault[i] = fault;

     numInTranslationFragments--;
     numTranslatedFragments++;

     if (fault == NoFault)
-        mainReq->setFlags(req->getFlags());
+        _mainReq->setFlags(req->getFlags());

-    if (numTranslatedFragments == _requests.size()) {
+    if (numTranslatedFragments == _reqs.size()) {
         if (_inst->isSquashed()) {
             squashTranslation();
         } else {
-            _inst->strictlyOrdered(mainReq->isStrictlyOrdered());
+            _inst->strictlyOrdered(_mainReq->isStrictlyOrdered());
             flags.set(Flag::TranslationFinished);
             _inst->translationCompleted(true);

             for (i = 0; i < _fault.size() && _fault[i] == NoFault; i++);
             if (i > 0) {
-                _inst->physEffAddr = request(0)->getPaddr();
-                _inst->memReqFlags = mainReq->getFlags();
-                if (mainReq->isCondSwap()) {
+                _inst->physEffAddr = LSQRequest::req()->getPaddr();
+                _inst->memReqFlags = _mainReq->getFlags();
+                if (_mainReq->isCondSwap()) {
                     assert (i == _fault.size());
                     assert(_res);
-                    mainReq->setExtraData(*_res);
+                    _mainReq->setExtraData(*_res);
                 }
                 if (i == _fault.size()) {
                     _inst->fault = NoFault;
@@ -930,18 +930,18 @@
 void
 LSQ::SingleDataRequest::initiateTranslation()
 {
-    assert(_requests.size() == 0);
+    assert(_reqs.size() == 0);

-    addRequest(_addr, _size, _byteEnable);
+    addReq(_addr, _size, _byteEnable);

-    if (_requests.size() > 0) {
-        _requests.back()->setReqInstSeqNum(_inst->seqNum);
-        _requests.back()->taskId(_taskId);
+    if (_reqs.size() > 0) {
+        _reqs.back()->setReqInstSeqNum(_inst->seqNum);
+        _reqs.back()->taskId(_taskId);
         _inst->translationStarted(true);
         setState(State::Translation);
         flags.set(Flag::TranslationStarted);

-        _inst->savedReq = this;
+        _inst->savedRequest = this;
         sendFragmentToTranslation(0);
     } else {
         _inst->setMemAccPredicate(false);
@@ -955,9 +955,9 @@
 }

 RequestPtr
-LSQ::SplitDataRequest::mainRequest()
+LSQ::SplitDataRequest::mainReq()
 {
-    return mainReq;
+    return _mainReq;
 }

 void
@@ -969,21 +969,21 @@
     Addr final_addr = addrBlockAlign(_addr + _size, cacheLineSize);
     uint32_t size_so_far = 0;

-    mainReq = std::make_shared<Request>(base_addr,
+    _mainReq = std::make_shared<Request>(base_addr,
                 _size, _flags, _inst->requestorId(),
                 _inst->instAddr(), _inst->contextId());
-    mainReq->setByteEnable(_byteEnable);
+    _mainReq->setByteEnable(_byteEnable);

-    // Paddr is not used in mainReq. However, we will accumulate the flags
- // from the sub requests into mainReq by calling setFlags() in finish().
+    // Paddr is not used in _mainReq. However, we will accumulate the flags
+ // from the sub requests into _mainReq by calling setFlags() in finish(). // setFlags() assumes that paddr is set so flip the paddr valid bit here to // avoid a potential assert in setFlags() when we call it from finish().
-    mainReq->setPaddr(0);
+    _mainReq->setPaddr(0);

     /* Get the pre-fix, possibly unaligned. */
     auto it_start = _byteEnable.begin();
     auto it_end = _byteEnable.begin() + (next_addr - base_addr);
-    addRequest(base_addr, next_addr - base_addr,
+    addReq(base_addr, next_addr - base_addr,
                      std::vector<bool>(it_start, it_end));
     size_so_far = next_addr - base_addr;

@@ -992,7 +992,7 @@
     while (base_addr != final_addr) {
         auto it_start = _byteEnable.begin() + size_so_far;
         auto it_end = _byteEnable.begin() + size_so_far + cacheLineSize;
-        addRequest(base_addr, cacheLineSize,
+        addReq(base_addr, cacheLineSize,
                          std::vector<bool>(it_start, it_end));
         size_so_far += cacheLineSize;
         base_addr += cacheLineSize;
@@ -1002,13 +1002,13 @@
     if (size_so_far < _size) {
         auto it_start = _byteEnable.begin() + size_so_far;
         auto it_end = _byteEnable.end();
-        addRequest(base_addr, _size - size_so_far,
+        addReq(base_addr, _size - size_so_far,
                          std::vector<bool>(it_start, it_end));
     }

-    if (_requests.size() > 0) {
+    if (_reqs.size() > 0) {
         /* Setup the requests and send them to translation. */
-        for (auto& r: _requests) {
+        for (auto& r: _reqs) {
             r->setReqInstSeqNum(_inst->seqNum);
             r->taskId(_taskId);
         }
@@ -1016,12 +1016,12 @@
         _inst->translationStarted(true);
         setState(State::Translation);
         flags.set(Flag::TranslationStarted);
-        _inst->savedReq = this;
+        _inst->savedRequest = this;
         numInTranslationFragments = 0;
         numTranslatedFragments = 0;
-        _fault.resize(_requests.size());
+        _fault.resize(_reqs.size());

-        for (uint32_t i = 0; i < _requests.size(); i++) {
+        for (uint32_t i = 0; i < _reqs.size(); i++) {
             sendFragmentToTranslation(i);
         }
     } else {
@@ -1080,23 +1080,23 @@
 bool LSQ::LSQRequest::squashed() const { return _inst->isSquashed(); }

 void
-LSQ::LSQRequest::addRequest(Addr addr, unsigned size,
+LSQ::LSQRequest::addReq(Addr addr, unsigned size,
            const std::vector<bool>& byte_enable)
 {
     if (isAnyActiveElement(byte_enable.begin(), byte_enable.end())) {
-        auto request = std::make_shared<Request>(
+        auto req = std::make_shared<Request>(
                 addr, size, _flags, _inst->requestorId(),
                 _inst->instAddr(), _inst->contextId(),
                 std::move(_amo_op));
-        request->setByteEnable(byte_enable);
-        _requests.push_back(request);
+        req->setByteEnable(byte_enable);
+        _reqs.push_back(req);
     }
 }

 LSQ::LSQRequest::~LSQRequest()
 {
     assert(!isAnyOutstandingRequest());
-    _inst->savedReq = nullptr;
+    _inst->savedRequest = nullptr;

     for (auto r: _packets)
         delete r;
@@ -1112,7 +1112,7 @@
 LSQ::LSQRequest::sendFragmentToTranslation(int i)
 {
     numInTranslationFragments++;
-    _port.getMMUPtr()->translateTiming(request(i), _inst->thread->getTC(),
+    _port.getMMUPtr()->translateTiming(req(i), _inst->thread->getTC(),
             this, isLoad() ? BaseMMU::Read : BaseMMU::Write);
 }

@@ -1138,8 +1138,8 @@
         flags.set(Flag::Complete);
         /* Assemble packets. */
         PacketPtr resp = isLoad()
-            ? Packet::createRead(mainReq)
-            : Packet::createWrite(mainReq);
+            ? Packet::createRead(_mainReq)
+            : Packet::createWrite(_mainReq);
         if (isLoad())
             resp->dataStatic(_inst->memData);
         else
@@ -1158,8 +1158,8 @@
     if (_packets.size() == 0) {
         _packets.push_back(
                 isLoad()
-                    ?  Packet::createRead(request())
-                    :  Packet::createWrite(request()));
+                    ?  Packet::createRead(req())
+                    :  Packet::createWrite(req()));
         _packets.back()->dataStatic(_inst->memData);
         _packets.back()->senderState = this;

@@ -1192,7 +1192,7 @@
     if (_packets.size() == 0) {
         /* New stuff */
         if (isLoad()) {
-            _mainPacket = Packet::createRead(mainReq);
+            _mainPacket = Packet::createRead(_mainReq);
             _mainPacket->dataStatic(_inst->memData);

             // hardware transactional memory
@@ -1210,18 +1210,18 @@
                   _inst->getHtmTransactionUid());
             }
         }
- for (int i = 0; i < _requests.size() && _fault[i] == NoFault; i++) {
-            RequestPtr r = _requests[i];
-            PacketPtr pkt = isLoad() ? Packet::createRead(r)
-                                     : Packet::createWrite(r);
-            ptrdiff_t offset = r->getVaddr() - base_address;
+        for (int i = 0; i < _reqs.size() && _fault[i] == NoFault; i++) {
+            RequestPtr req = _reqs[i];
+            PacketPtr pkt = isLoad() ? Packet::createRead(req)
+                                     : Packet::createWrite(req);
+            ptrdiff_t offset = req->getVaddr() - base_address;
             if (isLoad()) {
                 pkt->dataStatic(_inst->memData + offset);
             } else {
-                uint8_t* req_data = new uint8_t[r->getSize()];
+                uint8_t* req_data = new uint8_t[req->getSize()];
                 std::memcpy(req_data,
                         _inst->memData + offset,
-                        r->getSize());
+                        req->getSize());
                 pkt->dataDynamic(req_data);
             }
             pkt->senderState = this;
@@ -1281,7 +1281,7 @@
     Cycles delay(0);
     unsigned offset = 0;

-    for (auto r: _requests) {
+    for (auto r: _reqs) {
         PacketPtr pkt =
             new Packet(r, isLoad() ? MemCmd::ReadReq : MemCmd::WriteReq);
         pkt->dataStatic(mainPkt->getPtr<uint8_t>() + offset);
@@ -1297,7 +1297,7 @@
 bool
 LSQ::SingleDataRequest::isCacheBlockHit(Addr blockAddr, Addr blockMask)
 {
- return ( (LSQRequest::_requests[0]->getPaddr() & blockMask) == blockAddr);
+    return ( (LSQRequest::_reqs[0]->getPaddr() & blockMask) == blockAddr);
 }

 /**
@@ -1319,7 +1319,7 @@
 LSQ::SplitDataRequest::isCacheBlockHit(Addr blockAddr, Addr blockMask)
 {
     bool is_hit = false;
-    for (auto &r: _requests) {
+    for (auto &r: _reqs) {
        /**
* The load-store queue handles partial faults which complicates this
         * method. Physical addresses must be compared between requests and
@@ -1375,21 +1375,21 @@
     // The virtual and physical address uses a dummy value of 0x00
     // Address translation does not really occur thus the code below

-    assert(_requests.size() == 0);
+    assert(_reqs.size() == 0);

-    addRequest(_addr, _size, _byteEnable);
+    addReq(_addr, _size, _byteEnable);

-    if (_requests.size() > 0) {
-        _requests.back()->setReqInstSeqNum(_inst->seqNum);
-        _requests.back()->taskId(_taskId);
-        _requests.back()->setPaddr(_addr);
-        _requests.back()->setInstCount(_inst->getCpuPtr()->totalInsts());
+    if (_reqs.size() > 0) {
+        _reqs.back()->setReqInstSeqNum(_inst->seqNum);
+        _reqs.back()->taskId(_taskId);
+        _reqs.back()->setPaddr(_addr);
+        _reqs.back()->setInstCount(_inst->getCpuPtr()->totalInsts());

-        _inst->strictlyOrdered(_requests.back()->isStrictlyOrdered());
+        _inst->strictlyOrdered(_reqs.back()->isStrictlyOrdered());
         _inst->fault = NoFault;
-        _inst->physEffAddr = _requests.back()->getPaddr();
-        _inst->memReqFlags = _requests.back()->getFlags();
-        _inst->savedReq = this;
+        _inst->physEffAddr = _reqs.back()->getPaddr();
+        _inst->memReqFlags = _reqs.back()->getFlags();
+        _inst->savedRequest = this;

         flags.set(Flag::TranslationStarted);
         flags.set(Flag::TranslationFinished);
@@ -1411,19 +1411,20 @@
 }

 Fault
-LSQ::read(LSQRequest* req, int load_idx)
+LSQ::read(LSQRequest* request, int load_idx)
 {
-    ThreadID tid = cpu->contextToThread(req->request()->contextId());
+    assert(request->req()->contextId() == request->contextId());
+    ThreadID tid = cpu->contextToThread(request->req()->contextId());

-    return thread.at(tid).read(req, load_idx);
+    return thread.at(tid).read(request, load_idx);
 }

 Fault
-LSQ::write(LSQRequest* req, uint8_t *data, int store_idx)
+LSQ::write(LSQRequest* request, uint8_t *data, int store_idx)
 {
-    ThreadID tid = cpu->contextToThread(req->request()->contextId());
+    ThreadID tid = cpu->contextToThread(request->req()->contextId());

-    return thread.at(tid).write(req, data, store_idx);
+    return thread.at(tid).write(request, data, store_idx);
 }

 } // namespace o3
diff --git a/src/cpu/o3/lsq.hh b/src/cpu/o3/lsq.hh
index 4dc374e..c909f54 100644
--- a/src/cpu/o3/lsq.hh
+++ b/src/cpu/o3/lsq.hh
@@ -248,7 +248,7 @@
         uint32_t _taskId;
         PacketDataPtr _data;
         std::vector<PacketPtr> _packets;
-        std::vector<RequestPtr> _requests;
+        std::vector<RequestPtr> _reqs;
         std::vector<Fault> _fault;
         uint64_t* _res;
         const Addr _addr;
@@ -257,6 +257,7 @@
         std::vector<bool> _byteEnable;
         uint32_t _numOutstandingPackets;
         AtomicOpFunctorPtr _amo_op;
+
       protected:
         LSQUnit* lsqUnit() { return &_port; }
         LSQRequest(LSQUnit* port, const DynInstPtr& inst, bool isLoad);
@@ -309,7 +310,7 @@
          * The request is only added if there is at least one active
          * element in the mask.
          */
-        void addRequest(Addr addr, unsigned size,
+        void addReq(Addr addr, unsigned size,
                 const std::vector<bool>& byte_enable);

         /** Destructor.
@@ -325,7 +326,7 @@
         void
         setContext(const ContextID& context_id)
         {
-            request()->setContext(context_id);
+            req()->setContext(context_id);
         }

         const DynInstPtr& instruction() { return _inst; }
@@ -337,7 +338,7 @@
         setVirt(Addr vaddr, unsigned size, Request::Flags flags_,
                 RequestorID requestor_id, Addr pc)
         {
-            request()->setVirt(vaddr, size, flags_, requestor_id, pc);
+            req()->setVirt(vaddr, size, flags_, requestor_id, pc);
         }

         ContextID contextId() const;
@@ -346,20 +347,16 @@
         taskId(const uint32_t& v)
         {
             _taskId = v;
-            for (auto& r: _requests)
+            for (auto& r: _reqs)
                 r->taskId(v);
         }

         uint32_t taskId() const { return _taskId; }
-        RequestPtr request(int idx = 0) { return _requests.at(idx); }

-        const RequestPtr
-        request(int idx = 0) const
-        {
-            return _requests.at(idx);
-        }
+        RequestPtr req(int idx = 0) { return _reqs.at(idx); }
+        const RequestPtr req(int idx = 0) const { return _reqs.at(idx); }

- Addr getVaddr(int idx = 0) const { return request(idx)->getVaddr(); }
+        Addr getVaddr(int idx = 0) const { return req(idx)->getVaddr(); }
         virtual void initiateTranslation() = 0;

         PacketPtr packet(int idx = 0) { return _packets.at(idx); }
@@ -372,10 +369,10 @@
         }

         virtual RequestPtr
-        mainRequest()
+        mainReq()
         {
-            assert (_requests.size() == 1);
-            return request();
+            assert (_reqs.size() == 1);
+            return req();
         }

         /**
@@ -575,7 +572,7 @@
         using LSQRequest::_flags;
         using LSQRequest::_size;
         using LSQRequest::_byteEnable;
-        using LSQRequest::_requests;
+        using LSQRequest::_reqs;
         using LSQRequest::_inst;
         using LSQRequest::_packets;
         using LSQRequest::_port;
@@ -586,7 +583,7 @@
         using LSQRequest::isLoad;
         using LSQRequest::isTranslationComplete;
         using LSQRequest::lsqUnit;
-        using LSQRequest::request;
+        using LSQRequest::req;
         using LSQRequest::sendFragmentToTranslation;
         using LSQRequest::setState;
         using LSQRequest::numInTranslationFragments;
@@ -627,7 +624,7 @@
         using LSQRequest::_addr;
         using LSQRequest::_size;
         using LSQRequest::_byteEnable;
-        using LSQRequest::_requests;
+        using LSQRequest::_reqs;
         using LSQRequest::_inst;
         using LSQRequest::_taskId;
         using LSQRequest::flags;
@@ -656,7 +653,7 @@
         using LSQRequest::_inst;
         using LSQRequest::_packets;
         using LSQRequest::_port;
-        using LSQRequest::_requests;
+        using LSQRequest::_reqs;
         using LSQRequest::_res;
         using LSQRequest::_byteEnable;
         using LSQRequest::_size;
@@ -668,14 +665,14 @@
         using LSQRequest::lsqUnit;
         using LSQRequest::numInTranslationFragments;
         using LSQRequest::numTranslatedFragments;
-        using LSQRequest::request;
+        using LSQRequest::req;
         using LSQRequest::sendFragmentToTranslation;
         using LSQRequest::setState;
         using LSQRequest::_numOutstandingPackets;

         uint32_t numFragments;
         uint32_t numReceivedPackets;
-        RequestPtr mainReq;
+        RequestPtr _mainReq;
         PacketPtr _mainPacket;

       public:
@@ -687,15 +684,15 @@
                        nullptr),
             numFragments(0),
             numReceivedPackets(0),
-            mainReq(nullptr),
+            _mainReq(nullptr),
             _mainPacket(nullptr)
         {
             flags.set(Flag::IsSplit);
         }
         virtual ~SplitDataRequest()
         {
-            if (mainReq) {
-                mainReq = nullptr;
+            if (_mainReq) {
+                _mainReq = nullptr;
             }
             if (_mainPacket) {
                 delete _mainPacket;
@@ -713,7 +710,7 @@
                 gem5::ThreadContext *thread, PacketPtr pkt);
         virtual bool isCacheBlockHit(Addr blockAddr, Addr cacheBlockMask);

-        virtual RequestPtr mainRequest();
+        virtual RequestPtr mainReq();
         virtual PacketPtr mainPacket();
         virtual std::string name() const { return "SplitDataRequest"; }
     };
@@ -899,12 +896,12 @@
     /** Executes a read operation, using the load specified at the load
      * index.
      */
-    Fault read(LSQRequest* req, int load_idx);
+    Fault read(LSQRequest* request, int load_idx);

     /** Executes a store operation, using the store specified at the store
      * index.
      */
-    Fault write(LSQRequest* req, uint8_t *data, int store_idx);
+    Fault write(LSQRequest* request, uint8_t *data, int store_idx);

     /**
      * Retry the previous send that failed.
diff --git a/src/cpu/o3/lsq_unit.cc b/src/cpu/o3/lsq_unit.cc
index c5187ea..21bd469 100644
--- a/src/cpu/o3/lsq_unit.cc
+++ b/src/cpu/o3/lsq_unit.cc
@@ -67,8 +67,8 @@
     : Event(Default_Pri, AutoDelete),
       inst(_inst), pkt(_pkt), lsqPtr(lsq_ptr)
 {
-    assert(_inst->savedReq);
-    _inst->savedReq->writebackScheduled();
+    assert(_inst->savedRequest);
+    _inst->savedRequest->writebackScheduled();
 }

 void
@@ -78,8 +78,8 @@

     lsqPtr->writeback(inst, pkt);

-    assert(inst->savedReq);
-    inst->savedReq->writebackDone();
+    assert(inst->savedRequest);
+    inst->savedRequest->writebackDone();
     delete pkt;
 }

@@ -449,11 +449,11 @@

     DynInstPtr ld_inst = iter->instruction();
     assert(ld_inst);
-    LSQRequest *req = iter->request();
+    LSQRequest *request = iter->request();

     // Check that this snoop didn't just invalidate our lock flag
     if (ld_inst->effAddrValid() &&
-        req->isCacheBlockHit(invalidate_addr, cacheBlockMask)
+        request->isCacheBlockHit(invalidate_addr, cacheBlockMask)
         && ld_inst->memReqFlags & Request::LLSC) {
ld_inst->tcBase()->getIsaPtr()->handleLockedSnoopHit(ld_inst.get());
     }
@@ -463,7 +463,7 @@
     while (++iter != loadQueue.end()) {
         ld_inst = iter->instruction();
         assert(ld_inst);
-        req = iter->request();
+        request = iter->request();
         if (!ld_inst->effAddrValid() || ld_inst->strictlyOrdered())
             continue;

@@ -471,7 +471,7 @@
                     ld_inst->seqNum, invalidate_addr);

         if (force_squash ||
-            req->isCacheBlockHit(invalidate_addr, cacheBlockMask)) {
+            request->isCacheBlockHit(invalidate_addr, cacheBlockMask)) {
             if (needsTSO) {
// If we have a TSO system, as all loads must be ordered with // all other loads, this load as well as *all* subsequent loads
@@ -484,7 +484,7 @@

                 // Mark the load for re-execution
                 ld_inst->fault = std::make_shared<ReExec>();
-                req->setStateToFault();
+                request->setStateToFault();
             } else {
DPRINTF(LSQUnit, "HitExternal Snoop for addr %#x [sn:%lli]\n",
                         pkt->getAddr(), ld_inst->seqNum);
@@ -613,8 +613,9 @@
         return load_fault;

     if (load_fault != NoFault && inst->translationCompleted() &&
- inst->savedReq->isPartialFault() && !inst->savedReq->isComplete()) {
-        assert(inst->savedReq->isSplit());
+            inst->savedRequest->isPartialFault()
+            && !inst->savedRequest->isComplete()) {
+        assert(inst->savedRequest->isSplit());
// If we have a partial fault where the mem access is not complete yet // then the cache must have been blocked. This load will be re-executed
         // when the cache gets unblocked. We will handle the fault when the
@@ -825,39 +826,38 @@
         assert(!storeWBIt->committed());

         DynInstPtr inst = storeWBIt->instruction();
-        LSQRequest* req = storeWBIt->request();
+        LSQRequest* request = storeWBIt->request();

         // Process store conditionals or store release after all previous
         // stores are completed
-        if ((req->mainRequest()->isLLSC() ||
-             req->mainRequest()->isRelease()) &&
+        if ((request->mainReq()->isLLSC() ||
+             request->mainReq()->isRelease()) &&
              (storeWBIt.idx() != storeQueue.head())) {
             DPRINTF(LSQUnit, "Store idx:%i PC:%s to Addr:%#x "
                 "[sn:%lli] is %s%s and not head of the queue\n",
                 storeWBIt.idx(), inst->pcState(),
-                req->request()->getPaddr(), inst->seqNum,
-                req->mainRequest()->isLLSC() ? "SC" : "",
-                req->mainRequest()->isRelease() ? "/Release" : "");
+                request->mainReq()->getPaddr(), inst->seqNum,
+                request->mainReq()->isLLSC() ? "SC" : "",
+                request->mainReq()->isRelease() ? "/Release" : "");
             break;
         }

         storeWBIt->committed() = true;

         assert(!inst->memData);
-        inst->memData = new uint8_t[req->_size];
+        inst->memData = new uint8_t[request->_size];

         if (storeWBIt->isAllZeros())
-            memset(inst->memData, 0, req->_size);
+            memset(inst->memData, 0, request->_size);
         else
-            memcpy(inst->memData, storeWBIt->data(), req->_size);
+            memcpy(inst->memData, storeWBIt->data(), request->_size);

-
-        req->buildPackets();
+        request->buildPackets();

         DPRINTF(LSQUnit, "D-Cache: Writing back store idx:%i PC:%s "
                 "to Addr:%#x, data:%#x [sn:%lli]\n",
                 storeWBIt.idx(), inst->pcState(),
-                req->request()->getPaddr(), (int)*(inst->memData),
+                request->mainReq()->getPaddr(), (int)*(inst->memData),
                 inst->seqNum);

         // @todo: Remove this SC hack once the memory system handles it.
@@ -867,17 +867,17 @@
             // the desired behavior when handling store conditionals.
             inst->recordResult(false);
             bool success = inst->tcBase()->getIsaPtr()->handleLockedWrite(
-                    inst.get(), req->request(), cacheBlockMask);
+                    inst.get(), request->mainReq(), cacheBlockMask);
             inst->recordResult(true);
-            req->packetSent();
+            request->packetSent();

             if (!success) {
-                req->complete();
+                request->complete();
                 // Instantly complete this store.
                 DPRINTF(LSQUnit, "Store conditional [sn:%lli] failed.  "
                         "Instantly completing it.\n",
                         inst->seqNum);
-                PacketPtr new_pkt = new Packet(*req->packet());
+                PacketPtr new_pkt = new Packet(*request->packet());
                 WritebackEvent *wb = new WritebackEvent(inst,
                         new_pkt, this);
                 cpu->schedule(wb, curTick() + 1);
@@ -890,24 +890,24 @@
             }
         }

-        if (req->request()->isLocalAccess()) {
+        if (request->mainReq()->isLocalAccess()) {
             assert(!inst->isStoreConditional());
             assert(!inst->inHtmTransactionalState());
             gem5::ThreadContext *thread = cpu->tcBase(lsqID);
-            PacketPtr main_pkt = new Packet(req->mainRequest(),
+            PacketPtr main_pkt = new Packet(request->mainReq(),
                                             MemCmd::WriteReq);
             main_pkt->dataStatic(inst->memData);
-            req->request()->localAccessor(thread, main_pkt);
+            request->mainReq()->localAccessor(thread, main_pkt);
             delete main_pkt;
             completeStore(storeWBIt);
             storeWBIt++;
             continue;
         }
         /* Send to cache */
-        req->sendPacketToCache();
+        request->sendPacketToCache();

         /* If successful, do the post send */
-        if (req->isSent()) {
+        if (request->isSent()) {
             storePostSend();
         } else {
DPRINTF(LSQUnit, "D-Cache became blocked when writing [sn:%lli], "
@@ -1103,7 +1103,7 @@

             if (!htm_fault) {
assert(dynamic_cast<ReExec*>(inst->fault.get()) != nullptr ||
-                       inst->savedReq->isPartialFault());
+                       inst->savedRequest->isPartialFault());

             } else if (!pkt->htmTransactionFailedInCache()) {
                 // Situation in which the instruction has a hardware
@@ -1282,12 +1282,12 @@
 }

 Fault
-LSQUnit::read(LSQRequest *req, int load_idx)
+LSQUnit::read(LSQRequest *request, int load_idx)
 {
-    LQEntry& load_req = loadQueue[load_idx];
-    const DynInstPtr& load_inst = load_req.instruction();
+    LQEntry& load_entry = loadQueue[load_idx];
+    const DynInstPtr& load_inst = load_entry.instruction();

-    load_req.setRequest(req);
+    load_entry.setRequest(request);
     assert(load_inst);

     assert(!load_inst->isExecuted());
@@ -1297,7 +1297,7 @@
     // only if they're at the head of the LSQ and are ready to commit
     // (at the head of the ROB too).

-    if (req->mainRequest()->isStrictlyOrdered() &&
+    if (request->mainReq()->isStrictlyOrdered() &&
         (load_idx != loadQueue.head() || !load_inst->isAtCommit())) {
         // Tell IQ/mem dep unit that this instruction will need to be
         // rescheduled eventually
@@ -1311,8 +1311,8 @@
         // 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.
-        load_req.setRequest(nullptr);
-        req->discard();
+        load_entry.setRequest(nullptr);
+        request->discard();
         return std::make_shared<GenericISA::M5PanicFault>(
             "Strictly ordered load [sn:%llx] PC %s\n",
             load_inst->seqNum, load_inst->pcState());
@@ -1321,29 +1321,30 @@
     DPRINTF(LSQUnit, "Read called, load idx: %i, store idx: %i, "
             "storeHead: %i addr: %#x%s\n",
             load_idx - 1, load_inst->sqIt._idx, storeQueue.head() - 1,
- req->mainRequest()->getPaddr(), req->isSplit() ? " split" : "");
+            request->mainReq()->getPaddr(), request->isSplit() ? " split" :
+            "");

-    if (req->mainRequest()->isLLSC()) {
+    if (request->mainReq()->isLLSC()) {
         // Disable recording the result temporarily.  Writing to misc
         // regs normally updates the result, but this is not the
         // desired behavior when handling store conditionals.
         load_inst->recordResult(false);
         load_inst->tcBase()->getIsaPtr()->handleLockedRead(load_inst.get(),
-                req->mainRequest());
+                request->mainReq());
         load_inst->recordResult(true);
     }

-    if (req->mainRequest()->isLocalAccess()) {
+    if (request->mainReq()->isLocalAccess()) {
         assert(!load_inst->memData);
         assert(!load_inst->inHtmTransactionalState());
         load_inst->memData = new uint8_t[MaxDataBytes];

         gem5::ThreadContext *thread = cpu->tcBase(lsqID);
- PacketPtr main_pkt = new Packet(req->mainRequest(), MemCmd::ReadReq); + PacketPtr main_pkt = new Packet(request->mainReq(), MemCmd::ReadReq);

         main_pkt->dataStatic(load_inst->memData);

-        Cycles delay = req->mainRequest()->localAccessor(thread, main_pkt);
+        Cycles delay = request->mainReq()->localAccessor(thread, main_pkt);

         WritebackEvent *wb = new WritebackEvent(load_inst, main_pkt, this);
         cpu->schedule(wb, cpu->clockEdge(delay));
@@ -1351,21 +1352,21 @@
     }

     // hardware transactional memory
- if (req->mainRequest()->isHTMStart() || req->mainRequest()->isHTMCommit()) + if (request->mainReq()->isHTMStart() || request->mainReq()->isHTMCommit())
     {
         // don't want to send nested transactionStarts and
         // transactionStops outside of core, e.g. to Ruby
-        if (req->mainRequest()->getFlags().isSet(Request::NO_ACCESS)) {
+        if (request->mainReq()->getFlags().isSet(Request::NO_ACCESS)) {
             Cycles delay(0);
             PacketPtr data_pkt =
-                new Packet(req->mainRequest(), MemCmd::ReadReq);
+                new Packet(request->mainReq(), MemCmd::ReadReq);

             // Allocate memory if this is the first time a load is issued.
             if (!load_inst->memData) {
                 load_inst->memData =
-                    new uint8_t[req->mainRequest()->getSize()];
+                    new uint8_t[request->mainReq()->getSize()];
                 // sanity checks espect zero in request's data
- memset(load_inst->memData, 0, req->mainRequest()->getSize()); + memset(load_inst->memData, 0, request->mainReq()->getSize());
             }

             data_pkt->dataStatic(load_inst->memData);
@@ -1396,14 +1397,14 @@
         // path but they carry no data and they shouldn't be
         // considered for forwarding
if (store_size != 0 && !store_it->instruction()->strictlyOrdered() &&
-            !(store_it->request()->mainRequest() &&
-              store_it->request()->mainRequest()->isCacheMaintenance())) {
+            !(store_it->request()->mainReq() &&
+              store_it->request()->mainReq()->isCacheMaintenance())) {
             assert(store_it->instruction()->effAddrValid());

// Check if the store data is within the lower and upper bounds of
             // addresses that the request needs.
-            auto req_s = req->mainRequest()->getVaddr();
-            auto req_e = req_s + req->mainRequest()->getSize();
+            auto req_s = request->mainReq()->getVaddr();
+            auto req_e = req_s + request->mainReq()->getSize();
             auto st_s = store_it->instruction()->effAddr;
             auto st_e = st_s + store_size;

@@ -1420,22 +1421,22 @@
             // we can forward data from the store to the load
             if (!store_it->instruction()->isAtomic() &&
                 store_has_lower_limit && store_has_upper_limit &&
-                !req->mainRequest()->isLLSC()) {
+                !request->mainReq()->isLLSC()) {

-                const auto& store_req = store_it->request()->mainRequest();
+                const auto& store_req = store_it->request()->mainReq();
                 coverage = store_req->isMasked() ?
                     AddrRangeCoverage::PartialAddrRangeCoverage :
                     AddrRangeCoverage::FullAddrRangeCoverage;
             } else if (
// This is the partial store-load forwarding case where a store
                 // has only part of the load's data and the load isn't LLSC
-                (!req->mainRequest()->isLLSC() &&
+                (!request->mainReq()->isLLSC() &&
                  ((store_has_lower_limit && lower_load_has_store_part) ||
                   (store_has_upper_limit && upper_load_has_store_part) ||
(lower_load_has_store_part && upper_load_has_store_part))) ||
                 // The load is LLSC, and the store has all or part of the
                 // load's data
-                (req->mainRequest()->isLLSC() &&
+                (request->mainReq()->isLLSC() &&
                  ((store_has_lower_limit || upper_load_has_store_part) &&
                   (store_has_upper_limit || lower_load_has_store_part))) ||
// The store entry is atomic and has all or part of the load's
@@ -1449,27 +1450,27 @@

             if (coverage == AddrRangeCoverage::FullAddrRangeCoverage) {
                 // Get shift amount for offset into the store's data.
-                int shift_amt = req->mainRequest()->getVaddr() -
+                int shift_amt = request->mainReq()->getVaddr() -
                     store_it->instruction()->effAddr;

// Allocate memory if this is the first time a load is issued.
                 if (!load_inst->memData) {
                     load_inst->memData =
-                        new uint8_t[req->mainRequest()->getSize()];
+                        new uint8_t[request->mainReq()->getSize()];
                 }
                 if (store_it->isAllZeros())
                     memset(load_inst->memData, 0,
-                            req->mainRequest()->getSize());
+                            request->mainReq()->getSize());
                 else
                     memcpy(load_inst->memData,
                         store_it->data() + shift_amt,
-                        req->mainRequest()->getSize());
+                        request->mainReq()->getSize());

                 DPRINTF(LSQUnit, "Forwarding from store idx %i to load to "
                         "addr %#x\n", store_it._idx,
-                        req->mainRequest()->getVaddr());
+                        request->mainReq()->getVaddr());

-                PacketPtr data_pkt = new Packet(req->mainRequest(),
+                PacketPtr data_pkt = new Packet(request->mainReq(),
                         MemCmd::ReadReq);
                 data_pkt->dataStatic(load_inst->memData);

@@ -1480,7 +1481,7 @@
// write set of the transaction. The write set has a stronger // property than the read set, so the load doesn't necessarily
                 // have to be there.
-                assert(!req->mainRequest()->isHTMCmd());
+                assert(!request->mainReq()->isHTMCmd());
                 if (load_inst->inHtmTransactionalState()) {
                     assert (!storeQueue[store_it._idx].completed());
                     assert (
@@ -1502,13 +1503,13 @@
                       load_inst->getHtmTransactionUid());
                 }

-                if (req->isAnyOutstandingRequest()) {
-                    assert(req->_numOutstandingPackets > 0);
+                if (request->isAnyOutstandingRequest()) {
+                    assert(request->_numOutstandingPackets > 0);
                     // There are memory requests packets in flight already.
                     // This may happen if the store was not complete the
// first time this load got executed. Signal the senderSate
                     // that response packets should be discarded.
-                    req->discard();
+                    request->discard();
                 }

WritebackEvent *wb = new WritebackEvent(load_inst, data_pkt,
@@ -1554,11 +1555,11 @@
                 // complete.
                 DPRINTF(LSQUnit, "Load-store forwarding mis-match. "
                         "Store idx %i to load addr %#x\n",
-                        store_it._idx, req->mainRequest()->getVaddr());
+                        store_it._idx, request->mainReq()->getVaddr());

                 // Must discard the request.
-                req->discard();
-                load_req.setRequest(nullptr);
+                request->discard();
+                load_entry.setRequest(nullptr);
                 return NoFault;
             }
         }
@@ -1570,12 +1571,12 @@

     // Allocate memory if this is the first time a load is issued.
     if (!load_inst->memData) {
-        load_inst->memData = new uint8_t[req->mainRequest()->getSize()];
+        load_inst->memData = new uint8_t[request->mainReq()->getSize()];
     }


     // hardware transactional memory
-    if (req->mainRequest()->isHTMCmd()) {
+    if (request->mainReq()->isHTMCmd()) {
         // this is a simple sanity check
         // the Ruby cache controller will set
         // memData to 0x0ul if successful.
@@ -1589,36 +1590,36 @@
     // and arbitrate between loads and stores.

     // if we the cache is not blocked, do cache access
-    req->buildPackets();
-    req->sendPacketToCache();
-    if (!req->isSent())
+    request->buildPackets();
+    request->sendPacketToCache();
+    if (!request->isSent())
         iewStage->blockMemInst(load_inst);

     return NoFault;
 }

 Fault
-LSQUnit::write(LSQRequest *req, uint8_t *data, int store_idx)
+LSQUnit::write(LSQRequest *request, uint8_t *data, int store_idx)
 {
     assert(storeQueue[store_idx].valid());

DPRINTF(LSQUnit, "Doing write to store idx %i, addr %#x | storeHead:%i "
             "[sn:%llu]\n",
- store_idx - 1, req->request()->getPaddr(), storeQueue.head() - 1, + store_idx - 1, request->req()->getPaddr(), storeQueue.head() - 1,
             storeQueue[store_idx].instruction()->seqNum);

-    storeQueue[store_idx].setRequest(req);
-    unsigned size = req->_size;
+    storeQueue[store_idx].setRequest(request);
+    unsigned size = request->_size;
     storeQueue[store_idx].size() = size;
     bool store_no_data =
-        req->mainRequest()->getFlags() & Request::STORE_NO_DATA;
+        request->mainReq()->getFlags() & Request::STORE_NO_DATA;
     storeQueue[store_idx].isAllZeros() = store_no_data;
     assert(size <= SQEntry::DataSize || store_no_data);

// copy data into the storeQueue only if the store request has valid data
-    if (!(req->request()->getFlags() & Request::CACHE_BLOCK_ZERO) &&
-        !req->request()->isCacheMaintenance() &&
-        !req->request()->isAtomic())
+    if (!(request->req()->getFlags() & Request::CACHE_BLOCK_ZERO) &&
+        !request->req()->isCacheMaintenance() &&
+        !request->req()->isAtomic())
         memcpy(storeQueue[store_idx].data(), data, size);

     // This function only writes the data to the store queue, so no fault
diff --git a/src/cpu/o3/lsq_unit.hh b/src/cpu/o3/lsq_unit.hh
index 9bb1124..0d2f80f 100644
--- a/src/cpu/o3/lsq_unit.hh
+++ b/src/cpu/o3/lsq_unit.hh
@@ -97,9 +97,9 @@
     {
       private:
         /** The instruction. */
-        DynInstPtr inst;
+        DynInstPtr _inst;
         /** The request. */
-        LSQRequest* req = nullptr;
+        LSQRequest* _request = nullptr;
         /** The size of the operation. */
         uint32_t _size = 0;
         /** Valid entry. */
@@ -108,20 +108,20 @@
       public:
         ~LSQEntry()
         {
-            if (req != nullptr) {
-                req->freeLSQEntry();
-                req = nullptr;
+            if (_request != nullptr) {
+                _request->freeLSQEntry();
+                _request = nullptr;
             }
         }

         void
         clear()
         {
-            inst = nullptr;
-            if (req != nullptr) {
-                req->freeLSQEntry();
+            _inst = nullptr;
+            if (_request != nullptr) {
+                _request->freeLSQEntry();
             }
-            req = nullptr;
+            _request = nullptr;
             _valid = false;
             _size = 0;
         }
@@ -130,20 +130,20 @@
         set(const DynInstPtr& new_inst)
         {
             assert(!_valid);
-            inst = new_inst;
+            _inst = new_inst;
             _valid = true;
             _size = 0;
         }

-        LSQRequest* request() { return req; }
-        void setRequest(LSQRequest* r) { req = r; }
-        bool hasRequest() { return req != nullptr; }
+        LSQRequest* request() { return _request; }
+        void setRequest(LSQRequest* r) { _request = r; }
+        bool hasRequest() { return _request != nullptr; }
         /** Member accessors. */
         /** @{ */
         bool valid() const { return _valid; }
         uint32_t& size() { return _size; }
         const uint32_t& size() const { return _size; }
-        const DynInstPtr& instruction() const { return inst; }
+        const DynInstPtr& instruction() const { return _inst; }
         /** @} */
     };

@@ -443,7 +443,7 @@
     ThreadID lsqID;
   public:
     /** The store queue. */
-    CircularQueue<SQEntry> storeQueue;
+    StoreQueue storeQueue;

     /** The load queue. */
     LoadQueue loadQueue;
@@ -539,10 +539,10 @@

   public:
     /** Executes the load at the given index. */
-    Fault read(LSQRequest *req, int load_idx);
+    Fault read(LSQRequest *request, int load_idx);

     /** Executes the store at the given index. */
-    Fault write(LSQRequest *req, uint8_t *data, int store_idx);
+    Fault write(LSQRequest *request, uint8_t *data, int store_idx);

     /** Returns the index of the head load instruction. */
     int getLoadHead() { return loadQueue.head(); }
@@ -560,8 +560,6 @@
   public:
     typedef typename CircularQueue<LQEntry>::iterator LQIterator;
     typedef typename CircularQueue<SQEntry>::iterator SQIterator;
-    typedef CircularQueue<LQEntry> LQueue;
-    typedef CircularQueue<SQEntry> SQueue;
 };

 } // namespace o3

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/51067
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I8ba75b75bd8408e411300d522cc2c8582c334cf5
Gerrit-Change-Number: 51067
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Rollet <tom.rol...@huawei.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to