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.

diff --git a/src/cpu/o3/comm.hh b/src/cpu/o3/comm.hh
index ab0a9ff4228a105eaab4802835097c4b14037b96..a425484f5775237f1c43e593781f70ef0855ae89 100644 (file)
--- 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 ba594a2d2987ecbbab4cea684a21667e856d93d5..473e5e51d8523b79b85a631359236e3f8c772636 100644 (file)
--- 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 b6fdc40bbb2b482afa9ea10ce0914092dce091b3..45c231adb72325b727a57b4f05e988e33ba830ac 100644 (file)
--- a/src/cpu/o3/commit_impl.hh
+++ b/src/cpu/o3/commit_impl.hh
@@ -1333,29 +1333,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()
@@ -1379,23 +1356,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 3424b1d074e47618bf4c30dfee344076b2497bd9..006219a50376dd9f613b00231dfccdb2b7748ab4 100644 (file)
--- 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 c66f488a5c65e06ace144218e30eb295ae342fb9..93d04a8f732b4f448acbe9f3606862292fd9af79 100644 (file)
--- 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 eba4469c0a01e56d95006bb2a06eb40eaee6584e..0c1b81d86eb3ec7eac71cfa82443fadac91f81d4 100644 (file)
--- 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 98c1b699886fc4934723d45c4eb3ca376c405b62..637e39957d363ea8b8e84431feed9a72c698b649 100644 (file)
--- 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 3b752ac9926fc8437713c98ed15d803389a142b6..77403b499525f6ab411df4f06db702ca22e588da 100644 (file)
--- 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 cf2d5be5e886dbb41290170a4c3aaee299b6c4dc..0a4e147c4073dbbc4ec4be25096afbd2c9cf14d0 100644 (file)
--- 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;
     }
 
@@ -760,27 +758,13 @@ DefaultIEW<Impl>::resetEntries()
     ldstQueue.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 49abb005583ced098577b3dd872de04dd7ecc3de..04a9020d7b5671ac248197bfe18e234eca236859 100644 (file)
--- 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);
     }