Hello Gabor Dozsa,
I'd like you to do a code review. Please visit
https://gem5-review.googlesource.com/c/public/gem5/+/19174
to review the following change.
Change subject: cpu-o3: Fix too strict assert condition in writeback()
......................................................................
cpu-o3: Fix too strict assert condition in writeback()
The assert() in the LSQ writeback() only allowed ReExec faults.
However, a SplitRequest which completed the translation in
PartialFault state (i.e. any but the very first cacheline
translation failed) may end up here. The assert() condition is
extended accordingly.
The patch also removes the superfluous/unused Complete/Squashed
states from the LSQ request. (The completion of the request is
recorded in the flags still.)
Change-Id: Ie575f4d3b4d5295585828ad8c7d3f4c7c1fe15d0
Signed-off-by: Gabor Dozsa <[email protected]>
Reviewed-by: Giacomo Gabrielli <[email protected]>
---
M src/cpu/o3/lsq.hh
M src/cpu/o3/lsq_impl.hh
M src/cpu/o3/lsq_unit_impl.hh
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/cpu/o3/lsq.hh b/src/cpu/o3/lsq.hh
index 6f78201..4701a8c 100644
--- a/src/cpu/o3/lsq.hh
+++ b/src/cpu/o3/lsq.hh
@@ -223,8 +223,6 @@
NotIssued,
Translation,
Request,
- Complete,
- Squashed,
Fault,
PartialFault,
};
diff --git a/src/cpu/o3/lsq_impl.hh b/src/cpu/o3/lsq_impl.hh
index bd66d45..c5bf955 100644
--- a/src/cpu/o3/lsq_impl.hh
+++ b/src/cpu/o3/lsq_impl.hh
@@ -985,7 +985,6 @@
{
assert(_numOutstandingPackets == 1);
auto state = dynamic_cast<LSQSenderState*>(pkt->senderState);
- setState(State::Complete);
flags.set(Flag::Complete);
state->outstanding--;
assert(pkt == _packets.front());
@@ -1005,7 +1004,6 @@
numReceivedPackets++;
state->outstanding--;
if (numReceivedPackets == _packets.size()) {
- setState(State::Complete);
flags.set(Flag::Complete);
/* Assemble packets. */
PacketPtr resp = isLoad()
diff --git a/src/cpu/o3/lsq_unit_impl.hh b/src/cpu/o3/lsq_unit_impl.hh
index b71ed7f..c2483d5 100644
--- a/src/cpu/o3/lsq_unit_impl.hh
+++ b/src/cpu/o3/lsq_unit_impl.hh
@@ -968,8 +968,10 @@
// the access as this discards the current fault.
// If we have an outstanding fault, the fault should only be of
- // type ReExec.
- assert(dynamic_cast<ReExec*>(inst->fault.get()) != nullptr);
+ // type ReExec or - in case of a SplitRequest - a partial
+ // translation fault
+ assert(dynamic_cast<ReExec*>(inst->fault.get()) != nullptr ||
+ inst->savedReq->isPartialFault());
DPRINTF(LSQUnit, "Not completing instruction [sn:%lli] access "
"due to pending fault.\n", inst->seqNum);
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/19174
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: Ie575f4d3b4d5295585828ad8c7d3f4c7c1fe15d0
Gerrit-Change-Number: 19174
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Gabrielli <[email protected]>
Gerrit-Reviewer: Gabor Dozsa <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
