cpu: Fix incorrect speculative branch predictor behavior

When a branch mispredicted gem5 would squash all history after and including
the mispredicted branch.  However, the mispredicted branch is still speculative
and its history is required to rollback state if another, older, branch
mispredicts.  This leads to things like RAS corruption.

diff --git a/src/cpu/pred/2bit_local.hh b/src/cpu/pred/2bit_local.hh
index 23683cc674c242ec87d320744eac04e862580598..e008c62327431a45e51dcf2ad934e4f5f468355c 100644 (file)
--- a/src/cpu/pred/2bit_local.hh
+++ b/src/cpu/pred/2bit_local.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 ARM Limited
+ * Copyright (c) 2011, 2014 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -92,6 +92,9 @@ class LocalBP : public BPredUnit
      */
     void update(Addr branch_addr, bool taken, void *bp_history, bool squashed);
 
+    void retireSquashed(void *bp_history)
+    { assert(bp_history == NULL); }
+
     void squash(void *bp_history)
     { assert(bp_history == NULL); }
 

diff --git a/src/cpu/pred/bpred_unit.hh b/src/cpu/pred/bpred_unit.hh
index 61b375f9b4c926b6a3c15cd6b13d2b2cd72cf17d..f75ab79d548f65c7fd61ef200b8010e012f3a280 100644 (file)
--- a/src/cpu/pred/bpred_unit.hh
+++ b/src/cpu/pred/bpred_unit.hh
@@ -178,6 +178,13 @@ class BPredUnit : public SimObject
      */
     virtual void update(Addr instPC, bool taken, void *bp_history,
                         bool squashed) = 0;
+     /**
+     * Deletes the associated history with a branch, performs no predictor
+     * updates.  Used for branches that mispredict and update tables but
+     * are still speculative and later retire.
+     * @param bp_history History to delete associated with this predictor
+     */
+    virtual void retireSquashed(void *bp_history) = 0;
 
     /**
      * Updates the BTB with the target of a branch.
@@ -200,7 +207,7 @@ class BPredUnit : public SimObject
                          ThreadID _tid)
             : seqNum(seq_num), pc(instPC), bpHistory(bp_history), RASTarget(0),
               RASIndex(0), tid(_tid), predTaken(pred_taken), usedRAS(0), pushedRAS(0),
-              wasCall(0), wasReturn(0)
+              wasCall(0), wasReturn(0), wasSquashed(0)
         {}
 
         bool operator==(const PredictorHistory &entry) const {
@@ -234,7 +241,7 @@ class BPredUnit : public SimObject
         /** Whether or not the RAS was used. */
         bool usedRAS;
 
-        /* Wether or not the RAS was pushed */
+        /* Whether or not the RAS was pushed */
         bool pushedRAS;
 
         /** Whether or not the instruction was a call. */
@@ -242,6 +249,9 @@ class BPredUnit : public SimObject
 
         /** Whether or not the instruction was a return. */
         bool wasReturn;
+
+        /** Whether this instruction has already mispredicted/updated bp */
+        bool wasSquashed;
     };
 
     typedef std::deque<PredictorHistory> History;

diff --git a/src/cpu/pred/bpred_unit_impl.hh b/src/cpu/pred/bpred_unit_impl.hh
index 18e2217750caba69ef372adddae25c0e7cf99bdd..eaffb7ea1efeb7803d0b49910c8af2ed84fcd4e4 100644 (file)
--- a/src/cpu/pred/bpred_unit_impl.hh
+++ b/src/cpu/pred/bpred_unit_impl.hh
@@ -372,8 +372,12 @@ BPredUnit::update(const InstSeqNum &done_sn, ThreadID tid)
     while (!predHist[tid].empty() &&
            predHist[tid].back().seqNum <= done_sn) {
         // Update the branch predictor with the correct results.
-        update(predHist[tid].back().pc, predHist[tid].back().predTaken,
-               predHist[tid].back().bpHistory, false);
+        if (!predHist[tid].back().wasSquashed) {
+            update(predHist[tid].back().pc, predHist[tid].back().predTaken,
+                predHist[tid].back().bpHistory, false);
+        } else {
+            retireSquashed(predHist[tid].back().bpHistory);
+        }
 
         predHist[tid].pop_back();
     }
@@ -465,12 +469,15 @@ BPredUnit::squash(const InstSeqNum &squashed_sn,
 
         update((*hist_it).pc, actually_taken,
                pred_hist.front().bpHistory, true);
+        hist_it->wasSquashed = true;
+
         if (actually_taken) {
             if (hist_it->wasReturn && !hist_it->usedRAS) {
                  DPRINTF(Branch, "[tid: %i] Incorrectly predicted"
                          "  return [sn:%i] PC: %s\n", tid, hist_it->seqNum,
                          hist_it->pc);
                  RAS[tid].pop();
+                 hist_it->usedRAS = true;
             }
 
             DPRINTF(Branch,"[tid: %i] BTB Update called for [sn:%i]"
@@ -488,23 +495,16 @@ BPredUnit::squash(const InstSeqNum &squashed_sn,
                         " to: %i, target: %s.\n", tid,
                         hist_it->RASIndex, hist_it->RASTarget);
                 RAS[tid].restore(hist_it->RASIndex, hist_it->RASTarget);
-
+                hist_it->usedRAS = false;
            } else if (hist_it->wasCall && hist_it->pushedRAS) {
                  //Was a Call but predicated false. Pop RAS here
                  DPRINTF(Branch, "[tid: %i] Incorrectly predicted"
                          "  Call [sn:%i] PC: %s Popping RAS\n", tid,
                          hist_it->seqNum, hist_it->pc);
                  RAS[tid].pop();
+                 hist_it->pushedRAS = false;
            }
         }
-        DPRINTF(Branch, "[tid:%i]: Removing history for [sn:%i]"
-                " PC %s  Actually Taken: %i\n", tid, hist_it->seqNum,
-                hist_it->pc, actually_taken);
-
-        pred_hist.erase(hist_it);
-
-        DPRINTF(Branch, "[tid:%i]: predHist.size(): %i\n", tid,
-                                         predHist[tid].size());
     } else {
         DPRINTF(Branch, "[tid:%i]: [sn:%i] pred_hist empty, can't "
                 "update.\n", tid, squashed_sn);

diff --git a/src/cpu/pred/tournament.cc b/src/cpu/pred/tournament.cc
index e471d08f58b73f3abcdf0ca6426f780259edba29..c6514b6add0a2e1c49d3695349a62f53f450203b 100644 (file)
--- a/src/cpu/pred/tournament.cc
+++ b/src/cpu/pred/tournament.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 ARM Limited
+ * Copyright (c) 2011, 2014 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -346,10 +346,10 @@ TournamentBP::update(Addr branch_addr, bool taken, void *bp_history,
                 }
              }
 
+        } else {
+            // We're done with this history, now delete it.
+            delete history;
         }
-        // We're done with this history, now delete it.
-        delete history;
-
     }
 
     assert(local_history_idx < localHistoryTableSize);
@@ -357,6 +357,13 @@ TournamentBP::update(Addr branch_addr, bool taken, void *bp_history,
 
 }
 
+void
+TournamentBP::retireSquashed(void *bp_history)
+{
+    BPHistory *history = static_cast<BPHistory *>(bp_history);
+    delete history;
+}
+
 void
 TournamentBP::squash(void *bp_history)
 {

diff --git a/src/cpu/pred/tournament.hh b/src/cpu/pred/tournament.hh
index 39fff5bfb62623094831ca4b83475de17bc41e1b..88334a8c03071758615235ae820b2ffebcb2671b 100644 (file)
--- a/src/cpu/pred/tournament.hh
+++ b/src/cpu/pred/tournament.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 ARM Limited
+ * Copyright (c) 2011, 2014 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -104,6 +104,8 @@ class TournamentBP : public BPredUnit
      */
     void update(Addr branch_addr, bool taken, void *bp_history, bool squashed);
 
+    void retireSquashed(void *bp_history);
+
     /**
      * Restores the global branch history on a squash.
      * @param bp_history Pointer to the BPHistory object that has the