Tom Rollet has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/50733 )
Change subject: cpu-o3: remove LSQSenderState
......................................................................
cpu-o3: remove LSQSenderState
The LSQSenderState that was attached to Request was not useful.
All the fields were either a duplicate of information in the
LSQRequest or totally unused.
The LSQRequest class now inherits from Packet::SenderState and is
attached to the Packet that are sent to memory. We do not need
anymore the indirection Packet->SenderState->LSQRequest.
This helps making the code clearer as it was sometimes hard to
follow the difference between what the LSQRequest and
LSQSenserState was doing
(ex: number of outstanding requests in the memory).
Change-Id: I5b21e007e6d183c6aa79c27c1787ca56dcbc3fb0
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/50733
Reviewed-by: Bobby R. Bruce <bbr...@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbr...@ucdavis.edu>
Tested-by: kokoro <noreply+kok...@google.com>
---
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
4 files changed, 103 insertions(+), 217 deletions(-)
Approvals:
Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/cpu/o3/lsq.cc b/src/cpu/o3/lsq.cc
index babfa92..61823df 100644
--- a/src/cpu/o3/lsq.cc
+++ b/src/cpu/o3/lsq.cc
@@ -64,16 +64,6 @@
namespace o3
{
-LSQ::LSQSenderState::LSQSenderState(LSQRequest *request, bool is_load) :
- _request(request), isLoad(is_load), needWB(is_load)
-{}
-
-ContextID
-LSQ::LSQSenderState::contextId()
-{
- return inst->contextId();
-}
-
LSQ::DcachePort::DcachePort(LSQ *_lsq, CPU *_cpu) :
RequestPort(_cpu->name() + ".dcache_port", _cpu), lsq(_lsq), cpu(_cpu)
{}
@@ -402,8 +392,8 @@
void
LSQ::completeDataAccess(PacketPtr pkt)
{
- auto senderState = dynamic_cast<LSQSenderState*>(pkt->senderState);
- thread[cpu->contextToThread(senderState->contextId())]
+ LSQRequest *request = dynamic_cast<LSQRequest*>(pkt->senderState);
+ thread[cpu->contextToThread(request->contextId())]
.completeDataAccess(pkt);
}
@@ -414,10 +404,10 @@
DPRINTF(LSQ, "Got error packet back for address: %#X\n",
pkt->getAddr());
- auto senderState = dynamic_cast<LSQSenderState*>(pkt->senderState);
- panic_if(!senderState, "Got packet back with unknown sender state\n");
+ LSQRequest *request = dynamic_cast<LSQRequest*>(pkt->senderState);
+ panic_if(!request, "Got packet back with unknown sender state\n");
-
thread[cpu->contextToThread(senderState->contextId())].recvTimingResp(pkt);
+ thread[cpu->contextToThread(request->contextId())].recvTimingResp(pkt);
if (pkt->isInvalidate()) {
// This response also contains an invalidate; e.g. this can be the
case
@@ -439,7 +429,7 @@
}
}
// Update the LSQRequest state (this may delete the request)
- senderState->request()->packetReplied();
+ request->packetReplied();
return true;
}
@@ -1041,14 +1031,15 @@
LSQ::LSQRequest::LSQRequest(
LSQUnit *port, const DynInstPtr& inst, bool isLoad) :
- _state(State::NotIssued), _senderState(nullptr),
+ _state(State::NotIssued),
_port(*port), _inst(inst), _data(nullptr),
_res(nullptr), _addr(0), _size(0), _flags(0),
_numOutstandingPackets(0), _amo_op(nullptr)
{
flags.set(Flag::IsLoad, isLoad);
- flags.set(Flag::WbStore,
- _inst->isStoreConditional() || _inst->isAtomic());
+ flags.set(Flag::WriteBackToRegister,
+ _inst->isStoreConditional() || _inst->isAtomic() ||
+ _inst->isLoad());
flags.set(Flag::IsAtomic, _inst->isAtomic());
install();
}
@@ -1057,7 +1048,7 @@
LSQUnit *port, const DynInstPtr& inst, bool isLoad,
const Addr& addr, const uint32_t& size, const Request::Flags&
flags_,
PacketDataPtr data, uint64_t* res, AtomicOpFunctorPtr amo_op)
- : _state(State::NotIssued), _senderState(nullptr),
+ : _state(State::NotIssued),
numTranslatedFragments(0),
numInTranslationFragments(0),
_port(*port), _inst(inst), _data(data),
@@ -1067,8 +1058,9 @@
_amo_op(std::move(amo_op))
{
flags.set(Flag::IsLoad, isLoad);
- flags.set(Flag::WbStore,
- _inst->isStoreConditional() || _inst->isAtomic());
+ flags.set(Flag::WriteBackToRegister,
+ _inst->isStoreConditional() || _inst->isAtomic() ||
+ _inst->isLoad());
flags.set(Flag::IsAtomic, _inst->isAtomic());
install();
}
@@ -1105,13 +1097,17 @@
{
assert(!isAnyOutstandingRequest());
_inst->savedReq = nullptr;
- if (_senderState)
- delete _senderState;
for (auto r: _packets)
delete r;
};
+ContextID
+LSQ::LSQRequest::contextId() const
+{
+ return _inst->contextId();
+}
+
void
LSQ::LSQRequest::sendFragmentToTranslation(int i)
{
@@ -1124,9 +1120,7 @@
LSQ::SingleDataRequest::recvTimingResp(PacketPtr pkt)
{
assert(_numOutstandingPackets == 1);
- auto state = dynamic_cast<LSQSenderState*>(pkt->senderState);
flags.set(Flag::Complete);
- state->outstanding--;
assert(pkt == _packets.front());
_port.completeDataAccess(pkt);
return true;
@@ -1135,13 +1129,11 @@
bool
LSQ::SplitDataRequest::recvTimingResp(PacketPtr pkt)
{
- auto state = dynamic_cast<LSQSenderState*>(pkt->senderState);
uint32_t pktIdx = 0;
while (pktIdx < _packets.size() && pkt != _packets[pktIdx])
pktIdx++;
assert(pktIdx < _packets.size());
numReceivedPackets++;
- state->outstanding--;
if (numReceivedPackets == _packets.size()) {
flags.set(Flag::Complete);
/* Assemble packets. */
@@ -1152,7 +1144,7 @@
resp->dataStatic(_inst->memData);
else
resp->dataStatic(_data);
- resp->senderState = _senderState;
+ resp->senderState = this;
_port.completeDataAccess(resp);
delete resp;
}
@@ -1162,7 +1154,6 @@
void
LSQ::SingleDataRequest::buildPackets()
{
- assert(_senderState);
/* Retries do not create new packets. */
if (_packets.size() == 0) {
_packets.push_back(
@@ -1170,7 +1161,7 @@
? Packet::createRead(request())
: Packet::createWrite(request()));
_packets.back()->dataStatic(_inst->memData);
- _packets.back()->senderState = _senderState;
+ _packets.back()->senderState = this;
// hardware transactional memory
// If request originates in a transaction (not necessarily a
HtmCmd),
@@ -1233,7 +1224,7 @@
r->getSize());
pkt->dataDynamic(req_data);
}
- pkt->senderState = _senderState;
+ pkt->senderState = this;
_packets.push_back(pkt);
// hardware transactional memory
diff --git a/src/cpu/o3/lsq.hh b/src/cpu/o3/lsq.hh
index f843e64..4dc374e 100644
--- a/src/cpu/o3/lsq.hh
+++ b/src/cpu/o3/lsq.hh
@@ -76,48 +76,6 @@
{
public:
class LSQRequest;
- /** Derived class to hold any sender state the LSQ needs. */
- class LSQSenderState : public Packet::SenderState
- {
- protected:
- /** The senderState needs to know the LSQRequest who owns it. */
- LSQRequest* _request;
-
- /** Default constructor. */
- LSQSenderState(LSQRequest* request, bool is_load);
-
- public:
- /** Instruction which initiated the access to memory. */
- DynInstPtr inst;
- /** The main packet from a split load, used during writeback. */
- PacketPtr mainPkt = nullptr;
- /** A second packet from a split store that needs sending. */
- PacketPtr pendingPacket = nullptr;
- /** Number of outstanding packets to complete. */
- uint8_t outstanding = 0;
- /** Whether or not it is a load. */
- bool isLoad = false;
- /** Whether or not the instruction will need to writeback. */
- bool needWB = false;
- /** Whether or not this access is split in two. */
- bool isSplit = false;
- /** Whether or not there is a packet that needs sending. */
- bool pktToSend = false;
- /** Has the request been deleted?
- * LSQ entries can be squashed before the response comes back. in
that
- * case the SenderState knows.
- */
- bool deleted = false;
- ContextID contextId();
-
- /** Completes a packet and returns whether the access is finished.
*/
- bool isComplete() { return outstanding == 0; }
- void deleteRequest() { deleted = true; }
- bool alive() { return !deleted; }
- LSQRequest* request() { return _request; }
- virtual void complete() = 0;
- void writebackDone() { _request->writebackDone(); }
- };
/**
* DcachePort class for the load/store queue.
@@ -164,12 +122,12 @@
* This class holds the information about a memory operation. It lives
* from initiateAcc to resource deallocation at commit or squash.
* LSQRequest objects are owned by the LQ/SQ Entry in the LSQUnit that
- * holds the operation. It is also used by the LSQSenderState. In
addition,
- * the LSQRequest is a TranslationState, therefore, upon squash, there
must
- * be a defined ownership transferal in case the LSQ resources are
- * deallocated before the TLB is done using the TranslationState. If
that
- * happens, the LSQRequest will be self-owned, and responsible to
detect
- * that its services are no longer required and self-destruct.
+ * holds the operation. In addition, the LSQRequest is a
TranslationState,
+ * therefore, upon squash, there must be a defined ownership transferal
+ * in case the LSQ resources are deallocated before the TLB is done
using
+ * the TranslationState.
+ * If that happens, the LSQRequest will be self-owned, and responsible
to
+ * detect that its services are no longer required and self-destruct.
*
* Lifetime of a LSQRequest:
* +--------------------+
@@ -215,8 +173,8 @@
* | | +--------------------+
* | |
* | | +--------------------+
- * | | | senderState owns |
- * | +->| onRecvTimingResp |
+ * | | | self owned |
+ * | +->| on recvTimingResp |
* | | free resources |
* | +--------------------+
* |
@@ -228,7 +186,7 @@
*
*
*/
- class LSQRequest : public BaseMMU::Translation
+ class LSQRequest : public BaseMMU::Translation, public
Packet::SenderState
{
protected:
typedef uint32_t FlagsStorage;
@@ -237,8 +195,11 @@
enum Flag : FlagsStorage
{
IsLoad = 0x00000001,
- /** True if this is a store/atomic that writes registers (SC).
*/
- WbStore = 0x00000002,
+ /** True if this request needs to writeBack to register.
+ * Will be set in case of load or a store/atomic
+ * that writes registers (SC)
+ */
+ WriteBackToRegister = 0x00000002,
Delayed = 0x00000004,
IsSplit = 0x00000008,
/** True if any translation has been sent to TLB. */
@@ -272,14 +233,11 @@
PartialFault,
};
State _state;
- LSQSenderState* _senderState;
void setState(const State& newState) { _state = newState; }
uint32_t numTranslatedFragments;
uint32_t numInTranslationFragments;
- /** LQ/SQ entry idx. */
- uint32_t _entryIdx;
void markDelayed() override { flags.set(Flag::Delayed); }
bool isDelayed() { return flags.isSet(Flag::Delayed); }
@@ -324,17 +282,6 @@
bool squashed() const override;
- /**
- * Test if the LSQRequest has been released, i.e. self-owned.
- * An LSQRequest manages itself when the resources on the LSQ are
freed
- * but the translation is still going on and the LSQEntry was
freed.
- */
- bool
- isReleased()
- {
- return flags.isSet(Flag::LSQEntryFreed) ||
- flags.isSet(Flag::Discarded);
- }
/** Release the LSQRequest.
* Notify the sender state that the request it points to is not
valid
@@ -352,9 +299,6 @@
if (!isAnyOutstandingRequest()) {
delete this;
} else {
- if (_senderState) {
- _senderState->deleteRequest();
- }
flags.set(reason);
}
}
@@ -396,6 +340,8 @@
request()->setVirt(vaddr, size, flags_, requestor_id, pc);
}
+ ContextID contextId() const;
+
void
taskId(const uint32_t& v)
{
@@ -432,33 +378,6 @@
return request();
}
- void
- senderState(LSQSenderState* st)
- {
- _senderState = st;
- for (auto& pkt: _packets) {
- if (pkt)
- pkt->senderState = st;
- }
- }
-
- const LSQSenderState*
- senderState() const
- {
- return _senderState;
- }
-
- /**
- * Mark senderState as discarded. This will cause to discard
response
- * packets from the cache.
- */
- void
- discardSenderState()
- {
- assert(_senderState);
- _senderState->deleteRequest();
- }
-
/**
* Test if there is any in-flight translation or mem access request
*/
@@ -471,11 +390,29 @@
!flags.isSet(Flag::WritebackDone));
}
+ /**
+ * Test if the LSQRequest has been released, i.e. self-owned.
+ * An LSQRequest manages itself when the resources on the LSQ are
freed
+ * but the translation is still going on and the LSQEntry was
freed.
+ */
+ bool
+ isReleased()
+ {
+ return flags.isSet(Flag::LSQEntryFreed) ||
+ flags.isSet(Flag::Discarded);
+ }
+
bool
isSplit() const
{
return flags.isSet(Flag::IsSplit);
}
+
+ bool
+ needWBToRegister() const
+ {
+ return flags.isSet(Flag::WriteBackToRegister);
+ }
/** @} */
virtual bool recvTimingResp(PacketPtr pkt) = 0;
virtual void sendPacketToCache() = 0;
@@ -644,7 +581,6 @@
using LSQRequest::_port;
using LSQRequest::_res;
using LSQRequest::_taskId;
- using LSQRequest::_senderState;
using LSQRequest::_state;
using LSQRequest::flags;
using LSQRequest::isLoad;
@@ -723,7 +659,6 @@
using LSQRequest::_requests;
using LSQRequest::_res;
using LSQRequest::_byteEnable;
- using LSQRequest::_senderState;
using LSQRequest::_size;
using LSQRequest::_state;
using LSQRequest::_taskId;
diff --git a/src/cpu/o3/lsq_unit.cc b/src/cpu/o3/lsq_unit.cc
index 8b7bca5..c5187ea 100644
--- a/src/cpu/o3/lsq_unit.cc
+++ b/src/cpu/o3/lsq_unit.cc
@@ -92,25 +92,21 @@
bool
LSQUnit::recvTimingResp(PacketPtr pkt)
{
- auto senderState = dynamic_cast<LSQSenderState*>(pkt->senderState);
- LSQRequest* req = senderState->request();
- assert(req != nullptr);
+ LSQRequest *request = dynamic_cast<LSQRequest*>(pkt->senderState);
+ assert(request != nullptr);
bool ret = true;
/* Check that the request is still alive before any further action. */
- if (senderState->alive()) {
- ret = req->recvTimingResp(pkt);
- } else {
- senderState->outstanding--;
+ if (!request->isReleased()) {
+ ret = request->recvTimingResp(pkt);
}
return ret;
-
}
void
LSQUnit::completeDataAccess(PacketPtr pkt)
{
- LSQSenderState *state = dynamic_cast<LSQSenderState
*>(pkt->senderState);
- DynInstPtr inst = state->inst;
+ LSQRequest *request = dynamic_cast<LSQRequest *>(pkt->senderState);
+ DynInstPtr inst = request->instruction();
// hardware transactional memory
// sanity check
@@ -167,13 +163,9 @@
cpu->ppDataAccessComplete->notify(std::make_pair(inst, pkt));
- /* Notify the sender state that the access is complete (for ownership
- * tracking). */
- state->complete();
-
assert(!cpu->switchedOut());
if (!inst->isSquashed()) {
- if (state->needWB) {
+ if (request->needWBToRegister()) {
// Only loads, store conditionals and atomics perform the
writeback
// after receving the response from the memory
assert(inst->isLoad() || inst->isStoreConditional() ||
@@ -181,20 +173,19 @@
// hardware transactional memory
if (pkt->htmTransactionFailedInCache()) {
-
state->request()->mainPacket()->setHtmTransactionFailedInCache(
+ request->mainPacket()->setHtmTransactionFailedInCache(
pkt->getHtmTransactionFailedInCacheRC() );
}
- writeback(inst, state->request()->mainPacket());
+ writeback(inst, request->mainPacket());
if (inst->isStore() || inst->isAtomic()) {
- auto ss = dynamic_cast<SQSenderState*>(state);
- ss->writebackDone();
- completeStore(ss->idx);
+ request->writebackDone();
+ completeStore(request->instruction()->sqIt);
}
} else if (inst->isStore()) {
// This is a regular store (i.e., not store conditionals and
// atomics), so it can complete without writing back
- completeStore(dynamic_cast<SQSenderState*>(state)->idx);
+ completeStore(request->instruction()->sqIt);
}
}
}
@@ -397,6 +388,8 @@
storeQueue.advance_tail();
store_inst->sqIdx = storeQueue.tail();
+ store_inst->sqIt = storeQueue.getIterator(store_inst->sqIdx);
+
store_inst->lqIdx = loadQueue.tail() + 1;
assert(store_inst->lqIdx > 0);
store_inst->lqIt = loadQueue.end();
@@ -859,18 +852,6 @@
memcpy(inst->memData, storeWBIt->data(), req->_size);
- if (req->senderState() == nullptr) {
- SQSenderState *state = new SQSenderState(storeWBIt);
- state->isLoad = false;
- state->needWB = false;
- state->inst = inst;
-
- req->senderState(state);
- if (inst->isStoreConditional() || inst->isAtomic()) {
- /* Only store conditionals and atomics need a writeback. */
- state->needWB = true;
- }
- }
req->buildPackets();
DPRINTF(LSQUnit, "D-Cache: Writing back store idx:%i PC:%s "
@@ -1221,7 +1202,7 @@
bool ret = true;
bool cache_got_blocked = false;
- auto state = dynamic_cast<LSQSenderState*>(data_pkt->senderState);
+ LSQRequest *request = dynamic_cast<LSQRequest*>(data_pkt->senderState);
if (!lsq->cacheBlocked() &&
lsq->cachePortAvailable(isLoad)) {
@@ -1238,22 +1219,21 @@
isStoreBlocked = false;
}
lsq->cachePortBusy(isLoad);
- state->outstanding++;
- state->request()->packetSent();
+ request->packetSent();
} else {
if (cache_got_blocked) {
lsq->cacheBlocked(true);
++stats.blockedByCache;
}
if (!isLoad) {
- assert(state->request() == storeWBIt->request());
+ assert(request == storeWBIt->request());
isStoreBlocked = true;
}
- state->request()->packetNotSent();
+ request->packetNotSent();
}
DPRINTF(LSQUnit, "Memory request (pkt: %s) from inst [sn:%llu] was"
" %ssent (cache is blocked: %d, cache_got_blocked: %d)\n",
- data_pkt->print(), state->inst->seqNum,
+ data_pkt->print(), request->instruction()->seqNum,
ret ? "": "not ", lsq->cacheBlocked(), cache_got_blocked);
return ret;
}
@@ -1528,7 +1508,7 @@
// 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->discardSenderState();
+ req->discard();
}
WritebackEvent *wb = new WritebackEvent(load_inst,
data_pkt,
@@ -1609,14 +1589,6 @@
// and arbitrate between loads and stores.
// if we the cache is not blocked, do cache access
- if (req->senderState() == nullptr) {
- LQSenderState *state = new LQSenderState(
- loadQueue.getIterator(load_idx));
- state->isLoad = true;
- state->inst = load_inst;
- state->isSplit = req->isSplit();
- req->senderState(state);
- }
req->buildPackets();
req->sendPacketToCache();
if (!req->isSent())
diff --git a/src/cpu/o3/lsq_unit.hh b/src/cpu/o3/lsq_unit.hh
index 5b31d20..9bb1124 100644
--- a/src/cpu/o3/lsq_unit.hh
+++ b/src/cpu/o3/lsq_unit.hh
@@ -91,7 +91,6 @@
public:
static constexpr auto MaxDataBytes = MaxVecRegLenInBytes;
- using LSQSenderState = LSQ::LSQSenderState;
using LSQRequest = LSQ::LSQRequest;
private:
class LSQEntry
@@ -405,43 +404,6 @@
/** Pointer to the dcache port. Used only for sending. */
RequestPort *dcachePort;
- /** Particularisation of the LSQSenderState to the LQ. */
- class LQSenderState : public LSQSenderState
- {
- using LSQSenderState::alive;
- public:
- LQSenderState(typename LoadQueue::iterator idx_)
- : LSQSenderState(idx_->request(), true), idx(idx_) { }
-
- /** The LQ index of the instruction. */
- typename LoadQueue::iterator idx;
- //virtual LSQRequest* request() { return idx->request(); }
- virtual void
- complete()
- {
- //if (alive())
- // idx->request()->senderState(nullptr);
- }
- };
-
- /** Particularisation of the LSQSenderState to the SQ. */
- class SQSenderState : public LSQSenderState
- {
- using LSQSenderState::alive;
- public:
- SQSenderState(typename StoreQueue::iterator idx_)
- : LSQSenderState(idx_->request(), false), idx(idx_) { }
- /** The SQ index of the instruction. */
- typename StoreQueue::iterator idx;
- //virtual LSQRequest* request() { return idx->request(); }
- virtual void
- complete()
- {
- //if (alive())
- // idx->request()->senderState(nullptr);
- }
- };
-
/** Writeback event, specifically for when stores forward data to
loads. */
class WritebackEvent : public Event
{
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50733
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: I5b21e007e6d183c6aa79c27c1787ca56dcbc3fb0
Gerrit-Change-Number: 50733
Gerrit-PatchSet: 5
Gerrit-Owner: Tom Rollet <tom.rol...@huawei.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
Gerrit-Reviewer: Tom Rollet <tom.rol...@huawei.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
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
