changeset 867b536a68be in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=867b536a68be
description:
cpu: Fix o3 front-end pipeline interlock behavior
The o3 pipeline interlock/stall logic is incorrect. o3 unnecessicarily
stalled
fetch and decode due to later stages in the pipeline. In general, a
stage
should usually only consider if it is stalled by the adjacent,
downstream stage.
Forcing stalls due to later stages creates and results in bubbles in the
pipeline. Additionally, o3 stalled the entire frontend (fetch, decode,
rename)
on a branch mispredict while the ROB is being serially walked to update
the
RAT (robSquashing). Only should have stalled at rename.
diffstat:
src/cpu/o3/comm.hh | 2 -
src/cpu/o3/commit.hh | 11 --------
src/cpu/o3/commit_impl.hh | 40 -----------------------------
src/cpu/o3/decode.hh | 4 +--
src/cpu/o3/decode_impl.hh | 55 +++++++++++-----------------------------
src/cpu/o3/fetch.hh | 3 --
src/cpu/o3/fetch_impl.hh | 64 ++++------------------------------------------
src/cpu/o3/iew.hh | 11 --------
src/cpu/o3/iew_impl.hh | 23 +---------------
src/cpu/o3/rename_impl.hh | 25 +----------------
10 files changed, 26 insertions(+), 212 deletions(-)
diffs (truncated from 525 to 300 lines):
diff -r 5b6279635c49 -r 867b536a68be src/cpu/o3/comm.hh
--- a/src/cpu/o3/comm.hh Wed Sep 03 07:42:33 2014 -0400
+++ b/src/cpu/o3/comm.hh Wed Sep 03 07:42:34 2014 -0400
@@ -229,8 +229,6 @@
bool renameUnblock[Impl::MaxThreads];
bool iewBlock[Impl::MaxThreads];
bool iewUnblock[Impl::MaxThreads];
- bool commitBlock[Impl::MaxThreads];
- bool commitUnblock[Impl::MaxThreads];
};
#endif //__CPU_O3_COMM_HH__
diff -r 5b6279635c49 -r 867b536a68be src/cpu/o3/commit.hh
--- a/src/cpu/o3/commit.hh Wed Sep 03 07:42:33 2014 -0400
+++ b/src/cpu/o3/commit.hh Wed Sep 03 07:42:34 2014 -0400
@@ -185,9 +185,6 @@
/** Sets the pointer to the IEW stage. */
void setIEWStage(IEW *iew_stage);
- /** Skid buffer between rename and commit. */
- std::queue<DynInstPtr> skidBuffer;
-
/** The pointer to the IEW stage. Used solely to ensure that
* various events (traps, interrupts, syscalls) do not occur until
* all stores have written back.
@@ -251,11 +248,6 @@
*/
void setNextStatus();
- /** Checks if the ROB is completed with squashing. This is for the case
- * where the ROB can take multiple cycles to complete squashing.
- */
- bool robDoneSquashing();
-
/** Returns if any of the threads have the number of ROB entries changed
* on this cycle. Used to determine if the number of free ROB entries needs
* to be sent back to previous stages.
@@ -321,9 +313,6 @@
/** Gets instructions from rename and inserts them into the ROB. */
void getInsts();
- /** Insert all instructions from rename into skidBuffer */
- void skidInsert();
-
/** Marks completed instructions using information sent from IEW. */
void markCompletedInsts();
diff -r 5b6279635c49 -r 867b536a68be src/cpu/o3/commit_impl.hh
--- a/src/cpu/o3/commit_impl.hh Wed Sep 03 07:42:33 2014 -0400
+++ b/src/cpu/o3/commit_impl.hh Wed Sep 03 07:42:34 2014 -0400
@@ -1335,29 +1335,6 @@
template <class Impl>
void
-DefaultCommit<Impl>::skidInsert()
-{
- DPRINTF(Commit, "Attempting to any instructions from rename into "
- "skidBuffer.\n");
-
- for (int inst_num = 0; inst_num < fromRename->size; ++inst_num) {
- DynInstPtr inst = fromRename->insts[inst_num];
-
- if (!inst->isSquashed()) {
- DPRINTF(Commit, "Inserting PC %s [sn:%i] [tid:%i] into ",
- "skidBuffer.\n", inst->pcState(), inst->seqNum,
- inst->threadNumber);
- skidBuffer.push(inst);
- } else {
- DPRINTF(Commit, "Instruction PC %s [sn:%i] [tid:%i] was "
- "squashed, skipping.\n",
- inst->pcState(), inst->seqNum, inst->threadNumber);
- }
- }
-}
-
-template <class Impl>
-void
DefaultCommit<Impl>::markCompletedInsts()
{
// Grab completed insts out of the IEW instruction queue, and mark
@@ -1380,23 +1357,6 @@
}
template <class Impl>
-bool
-DefaultCommit<Impl>::robDoneSquashing()
-{
- list<ThreadID>::iterator threads = activeThreads->begin();
- list<ThreadID>::iterator end = activeThreads->end();
-
- while (threads != end) {
- ThreadID tid = *threads++;
-
- if (!rob->isDoneSquashing(tid))
- return false;
- }
-
- return true;
-}
-
-template <class Impl>
void
DefaultCommit<Impl>::updateComInstStats(DynInstPtr &inst)
{
diff -r 5b6279635c49 -r 867b536a68be src/cpu/o3/decode.hh
--- a/src/cpu/o3/decode.hh Wed Sep 03 07:42:33 2014 -0400
+++ b/src/cpu/o3/decode.hh Wed Sep 03 07:42:34 2014 -0400
@@ -126,7 +126,7 @@
void drainSanityCheck() const;
/** Has the stage drained? */
- bool isDrained() const { return true; }
+ bool isDrained() const;
/** Takes over from another CPU's thread. */
void takeOverFrom() { resetStage(); }
@@ -249,8 +249,6 @@
/** Source of possible stalls. */
struct Stalls {
bool rename;
- bool iew;
- bool commit;
};
/** Tracks which stages are telling decode to stall. */
diff -r 5b6279635c49 -r 867b536a68be src/cpu/o3/decode_impl.hh
--- a/src/cpu/o3/decode_impl.hh Wed Sep 03 07:42:33 2014 -0400
+++ b/src/cpu/o3/decode_impl.hh Wed Sep 03 07:42:34 2014 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012 ARM Limited
+ * Copyright (c) 2012, 2014 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -95,8 +95,6 @@
decodeStatus[tid] = Idle;
stalls[tid].rename = false;
- stalls[tid].iew = false;
- stalls[tid].commit = false;
}
}
@@ -206,6 +204,17 @@
}
}
+template <class Impl>
+bool
+DefaultDecode<Impl>::isDrained() const
+{
+ for (ThreadID tid = 0; tid < numThreads; ++tid) {
+ if (!insts[tid].empty() || !skidBuffer[tid].empty())
+ return false;
+ }
+ return true;
+}
+
template<class Impl>
bool
DefaultDecode<Impl>::checkStall(ThreadID tid) const
@@ -215,12 +224,6 @@
if (stalls[tid].rename) {
DPRINTF(Decode,"[tid:%i]: Stall fom Rename stage detected.\n", tid);
ret_val = true;
- } else if (stalls[tid].iew) {
- DPRINTF(Decode,"[tid:%i]: Stall fom IEW stage detected.\n", tid);
- ret_val = true;
- } else if (stalls[tid].commit) {
- DPRINTF(Decode,"[tid:%i]: Stall fom Commit stage detected.\n", tid);
- ret_val = true;
}
return ret_val;
@@ -395,10 +398,10 @@
assert(tid == inst->threadNumber);
- DPRINTF(Decode,"Inserting [sn:%lli] PC: %s into decode skidBuffer
%i\n",
- inst->seqNum, inst->pcState(), inst->threadNumber);
+ skidBuffer[tid].push(inst);
- skidBuffer[tid].push(inst);
+ DPRINTF(Decode,"Inserting [tid:%d][sn:%lli] PC: %s into decode
skidBuffer %i\n",
+ inst->threadNumber, inst->seqNum, inst->pcState(),
skidBuffer[tid].size());
}
// @todo: Eventually need to enforce this by not letting a thread
@@ -483,24 +486,6 @@
assert(stalls[tid].rename);
stalls[tid].rename = false;
}
-
- if (fromIEW->iewBlock[tid]) {
- stalls[tid].iew = true;
- }
-
- if (fromIEW->iewUnblock[tid]) {
- assert(stalls[tid].iew);
- stalls[tid].iew = false;
- }
-
- if (fromCommit->commitBlock[tid]) {
- stalls[tid].commit = true;
- }
-
- if (fromCommit->commitUnblock[tid]) {
- assert(stalls[tid].commit);
- stalls[tid].commit = false;
- }
}
template <class Impl>
@@ -529,16 +514,6 @@
return true;
}
- // Check ROB squash signals from commit.
- if (fromCommit->commitInfo[tid].robSquashing) {
- DPRINTF(Decode, "[tid:%u]: ROB is still squashing.\n", tid);
-
- // Continue to squash.
- decodeStatus[tid] = Squashing;
-
- return true;
- }
-
if (checkStall(tid)) {
return block(tid);
}
diff -r 5b6279635c49 -r 867b536a68be src/cpu/o3/fetch.hh
--- a/src/cpu/o3/fetch.hh Wed Sep 03 07:42:33 2014 -0400
+++ b/src/cpu/o3/fetch.hh Wed Sep 03 07:42:34 2014 -0400
@@ -434,9 +434,6 @@
/** Source of possible stalls. */
struct Stalls {
bool decode;
- bool rename;
- bool iew;
- bool commit;
bool drain;
};
diff -r 5b6279635c49 -r 867b536a68be src/cpu/o3/fetch_impl.hh
--- a/src/cpu/o3/fetch_impl.hh Wed Sep 03 07:42:33 2014 -0400
+++ b/src/cpu/o3/fetch_impl.hh Wed Sep 03 07:42:34 2014 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010-2013 ARM Limited
+ * Copyright (c) 2010-2014 ARM Limited
* All rights reserved.
*
* The license below extends only to copyright in the software and shall
@@ -354,9 +354,6 @@
memReq[tid] = NULL;
stalls[tid].decode = false;
- stalls[tid].rename = false;
- stalls[tid].iew = false;
- stalls[tid].commit = false;
stalls[tid].drain = false;
fetchBufferPC[tid] = 0;
@@ -435,10 +432,6 @@
for (ThreadID i = 0; i < numThreads; ++i) {
assert(!memReq[i]);
- assert(!stalls[i].decode);
- assert(!stalls[i].rename);
- assert(!stalls[i].iew);
- assert(!stalls[i].commit);
assert(fetchStatus[i] == Idle || stalls[i].drain);
}
@@ -680,7 +673,11 @@
fetchStatus[tid] = IcacheWaitResponse;
}
} else {
- if (!(numInst < fetchWidth)) {
+ // Don't send an instruction to decode if it can't handle it.
+ // Asynchronous nature of this function's calling means we have to
+ // check 2 signals to see if decode is stalled.
+ if (!(numInst < fetchWidth) || stalls[tid].decode ||
+ fromDecode->decodeBlock[tid]) {
assert(!finishTranslationEvent.scheduled());
finishTranslationEvent.setFault(fault);
finishTranslationEvent.setReq(mem_req);
@@ -802,15 +799,6 @@
} else if (stalls[tid].decode) {
DPRINTF(Fetch,"[tid:%i]: Stall from Decode stage detected.\n",tid);
ret_val = true;
- } else if (stalls[tid].rename) {
- DPRINTF(Fetch,"[tid:%i]: Stall from Rename stage detected.\n",tid);
- ret_val = true;
- } else if (stalls[tid].iew) {
- DPRINTF(Fetch,"[tid:%i]: Stall from IEW stage detected.\n",tid);
- ret_val = true;
- } else if (stalls[tid].commit) {
- DPRINTF(Fetch,"[tid:%i]: Stall from Commit stage detected.\n",tid);
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
