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.

10 files changed, 26 insertions, 212 deletions

diff --git a/src/cpu/o3/comm.hh b/src/cpu/o3/comm.hh
index ab0a9ff42..a425484f5 100644
--- a/src/cpu/o3/comm.hh
+++ b/src/cpu/o3/comm.hh
@@ -229,8 +229,6 @@ struct TimeBufStruct {
     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 --git a/src/cpu/o3/commit.hh b/src/cpu/o3/commit.hh
index ba594a2d2..473e5e51d 100644
--- a/src/cpu/o3/commit.hh
+++ b/src/cpu/o3/commit.hh
@@ -185,9 +185,6 @@ class DefaultCommit
     /** 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 @@ class DefaultCommit
      */
     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 @@ class DefaultCommit
     /** 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 --git a/src/cpu/o3/commit_impl.hh b/src/cpu/o3/commit_impl.hh
index b6fdc40bb..45c231adb 100644
--- a/src/cpu/o3/commit_impl.hh
+++ b/src/cpu/o3/commit_impl.hh
@@ -1335,29 +1335,6 @@ DefaultCommit<Impl>::getInsts()
 
 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 @@ DefaultCommit<Impl>::markCompletedInsts()
 }
 
 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 --git a/src/cpu/o3/decode.hh b/src/cpu/o3/decode.hh
index 3424b1d07..006219a50 100644
--- a/src/cpu/o3/decode.hh
+++ b/src/cpu/o3/decode.hh
@@ -126,7 +126,7 @@ class DefaultDecode
     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 @@ class DefaultDecode
     /** Source of possible stalls. */
     struct Stalls {
         bool rename;
-        bool iew;
-        bool commit;
     };
 
     /** Tracks which stages are telling decode to stall. */
diff --git a/src/cpu/o3/decode_impl.hh b/src/cpu/o3/decode_impl.hh
index c66f488a5..93d04a8f7 100644
--- a/src/cpu/o3/decode_impl.hh
+++ b/src/cpu/o3/decode_impl.hh
@@ -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 @@ DefaultDecode<Impl>::resetStage()
         decodeStatus[tid] = Idle;
 
         stalls[tid].rename = false;
-        stalls[tid].iew = false;
-        stalls[tid].commit = false;
     }
 }
 
@@ -206,6 +204,17 @@ DefaultDecode<Impl>::drainSanityCheck() const
     }
 }
 
+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 @@ DefaultDecode<Impl>::checkStall(ThreadID tid) const
     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 @@ DefaultDecode<Impl>::skidInsert(ThreadID tid)
 
         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);
+
+        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 @@ DefaultDecode<Impl>::readStallSignals(ThreadID tid)
         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 @@ DefaultDecode<Impl>::checkSignalsAndUpdate(ThreadID tid)
         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 --git a/src/cpu/o3/fetch.hh b/src/cpu/o3/fetch.hh
index eba4469c0..0c1b81d86 100644
--- a/src/cpu/o3/fetch.hh
+++ b/src/cpu/o3/fetch.hh
@@ -434,9 +434,6 @@ class DefaultFetch
     /** Source of possible stalls. */
     struct Stalls {
         bool decode;
-        bool rename;
-        bool iew;
-        bool commit;
         bool drain;
     };
 
diff --git a/src/cpu/o3/fetch_impl.hh b/src/cpu/o3/fetch_impl.hh
index 98c1b6998..637e39957 100644
--- a/src/cpu/o3/fetch_impl.hh
+++ b/src/cpu/o3/fetch_impl.hh
@@ -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 @@ DefaultFetch<Impl>::resetStage()
         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 @@ DefaultFetch<Impl>::drainSanityCheck() const
 
     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 @@ DefaultFetch<Impl>::finishTranslation(Fault fault, RequestPtr mem_req)
             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 @@ DefaultFetch<Impl>::checkStall(ThreadID tid) const
     } 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);
-        ret_val = true;
     }
 
     return ret_val;
@@ -952,36 +940,6 @@ DefaultFetch<Impl>::checkSignalsAndUpdate(ThreadID tid)
         stalls[tid].decode = false;
     }
 
-    if (fromRename->renameBlock[tid]) {
-        stalls[tid].rename = true;
-    }
-
-    if (fromRename->renameUnblock[tid]) {
-        assert(stalls[tid].rename);
-        assert(!fromRename->renameBlock[tid]);
-        stalls[tid].rename = false;
-    }
-
-    if (fromIEW->iewBlock[tid]) {
-        stalls[tid].iew = true;
-    }
-
-    if (fromIEW->iewUnblock[tid]) {
-        assert(stalls[tid].iew);
-        assert(!fromIEW->iewBlock[tid]);
-        stalls[tid].iew = false;
-    }
-
-    if (fromCommit->commitBlock[tid]) {
-        stalls[tid].commit = true;
-    }
-
-    if (fromCommit->commitUnblock[tid]) {
-        assert(stalls[tid].commit);
-        assert(!fromCommit->commitBlock[tid]);
-        stalls[tid].commit = false;
-    }
-
     // Check squash signals from commit.
     if (fromCommit->commitInfo[tid].squash) {
 
@@ -1013,16 +971,6 @@ DefaultFetch<Impl>::checkSignalsAndUpdate(ThreadID tid)
         branchPred->update(fromCommit->commitInfo[tid].doneSeqNum, tid);
     }
 
-    // Check ROB squash signals from commit.
-    if (fromCommit->commitInfo[tid].robSquashing) {
-        DPRINTF(Fetch, "[tid:%u]: ROB is still squashing.\n", tid);
-
-        // Continue to squash.
-        fetchStatus[tid] = Squashing;
-
-        return true;
-    }
-
     // Check squash signals from decode.
     if (fromDecode->decodeInfo[tid].squash) {
         DPRINTF(Fetch, "[tid:%u]: Squashing instructions due to squash "
diff --git a/src/cpu/o3/iew.hh b/src/cpu/o3/iew.hh
index 3b752ac99..77403b499 100644
--- a/src/cpu/o3/iew.hh
+++ b/src/cpu/o3/iew.hh
@@ -270,9 +270,6 @@ class DefaultIEW
      */
     unsigned validInstsFromRename();
 
-    /** Reads the stall signals. */
-    void readStallSignals(ThreadID tid);
-
     /** Checks if any of the stall conditions are currently true. */
     bool checkStall(ThreadID tid);
 
@@ -346,14 +343,6 @@ class DefaultIEW
      */
     bool wroteToTimeBuffer;
 
-    /** Source of possible stalls. */
-    struct Stalls {
-        bool commit;
-    };
-
-    /** Stages that are telling IEW to stall. */
-    Stalls stalls[Impl::MaxThreads];
-
     /** Debug function to print instructions that are issued this cycle. */
     void printAvailableInsts();
 
diff --git a/src/cpu/o3/iew_impl.hh b/src/cpu/o3/iew_impl.hh
index cf2d5be5e..0a4e147c4 100644
--- a/src/cpu/o3/iew_impl.hh
+++ b/src/cpu/o3/iew_impl.hh
@@ -104,13 +104,12 @@ DefaultIEW<Impl>::DefaultIEW(O3CPU *_cpu, DerivO3CPUParams *params)
 
     for (ThreadID tid = 0; tid < numThreads; tid++) {
         dispatchStatus[tid] = Running;
-        stalls[tid].commit = false;
         fetchRedirect[tid] = false;
     }
 
     updateLSQNextCycle = false;
 
-    skidBufferMax = (3 * (renameToIEWDelay * params->renameWidth)) + issueWidth;
+    skidBufferMax = (renameToIEWDelay + 1) * params->renameWidth;
 }
 
 template <class Impl>
@@ -434,7 +433,6 @@ DefaultIEW<Impl>::takeOverFrom()
 
     for (ThreadID tid = 0; tid < numThreads; tid++) {
         dispatchStatus[tid] = Running;
-        stalls[tid].commit = false;
         fetchRedirect[tid] = false;
     }
 
@@ -761,26 +759,12 @@ DefaultIEW<Impl>::resetEntries()
 }
 
 template <class Impl>
-void
-DefaultIEW<Impl>::readStallSignals(ThreadID tid)
-{
-    if (fromCommit->commitBlock[tid]) {
-        stalls[tid].commit = true;
-    }
-
-    if (fromCommit->commitUnblock[tid]) {
-        assert(stalls[tid].commit);
-        stalls[tid].commit = false;
-    }
-}
-
-template <class Impl>
 bool
 DefaultIEW<Impl>::checkStall(ThreadID tid)
 {
     bool ret_val(false);
 
-    if (stalls[tid].commit) {
+    if (fromCommit->commitInfo[tid].robSquashing) {
         DPRINTF(IEW,"[tid:%i]: Stall from Commit stage detected.\n",tid);
         ret_val = true;
     } else if (instQueue.isFull(tid)) {
@@ -802,8 +786,6 @@ DefaultIEW<Impl>::checkSignalsAndUpdate(ThreadID tid)
     // If status was Squashing
     //     check if squashing is not high.  Switch to running this cycle.
 
-    readStallSignals(tid);
-
     if (fromCommit->commitInfo[tid].squash) {
         squash(tid);
 
@@ -824,7 +806,6 @@ DefaultIEW<Impl>::checkSignalsAndUpdate(ThreadID tid)
         dispatchStatus[tid] = Squashing;
         emptyRenameInsts(tid);
         wroteToTimeBuffer = true;
-        return;
     }
 
     if (checkStall(tid)) {
diff --git a/src/cpu/o3/rename_impl.hh b/src/cpu/o3/rename_impl.hh
index 49abb0055..04a9020d7 100644
--- a/src/cpu/o3/rename_impl.hh
+++ b/src/cpu/o3/rename_impl.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2012 ARM Limited
+ * Copyright (c) 2010-2012, 2014 ARM Limited
  * Copyright (c) 2013 Advanced Micro Devices, Inc.
  * All rights reserved.
  *
@@ -77,7 +77,7 @@ DefaultRename<Impl>::DefaultRename(O3CPU *_cpu, DerivO3CPUParams *params)
              renameWidth, static_cast<int>(Impl::MaxWidth));
 
     // @todo: Make into a parameter.
-    skidBufferMax = (2 * (decodeToRenameDelay * params->decodeWidth)) + renameWidth;
+    skidBufferMax = (decodeToRenameDelay + 1) * params->decodeWidth;
 }
 
 template <class Impl>
@@ -247,7 +247,6 @@ DefaultRename<Impl>::resetStage()
         emptyROB[tid] = true;
 
         stalls[tid].iew = false;
-        stalls[tid].commit = false;
         serializeInst[tid] = NULL;
 
         instsInProgress[tid] = 0;
@@ -1200,15 +1199,6 @@ DefaultRename<Impl>::readStallSignals(ThreadID 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>
@@ -1220,9 +1210,6 @@ DefaultRename<Impl>::checkStall(ThreadID tid)
     if (stalls[tid].iew) {
         DPRINTF(Rename,"[tid:%i]: Stall from IEW stage detected.\n", tid);
         ret_val = true;
-    } else if (stalls[tid].commit) {
-        DPRINTF(Rename,"[tid:%i]: Stall from Commit stage detected.\n", tid);
-        ret_val = true;
     } else if (calcFreeROBEntries(tid) <= 0) {
         DPRINTF(Rename,"[tid:%i]: Stall: ROB has 0 free entries.\n", tid);
         ret_val = true;
@@ -1302,14 +1289,6 @@ DefaultRename<Impl>::checkSignalsAndUpdate(ThreadID tid)
         return true;
     }
 
-    if (fromCommit->commitInfo[tid].robSquashing) {
-        DPRINTF(Rename, "[tid:%u]: ROB is still squashing.\n", tid);
-
-        renameStatus[tid] = Squashing;
-
-        return true;
-    }
-
     if (checkStall(tid)) {
         return block(tid);
     }