changeset f54586c894e3 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=f54586c894e3
description:
        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.

diffstat:

 src/cpu/pred/2bit_local.hh      |   5 ++++-
 src/cpu/pred/bpred_unit.hh      |  14 ++++++++++++--
 src/cpu/pred/bpred_unit_impl.hh |  22 +++++++++++-----------
 src/cpu/pred/tournament.cc      |  15 +++++++++++----
 src/cpu/pred/tournament.hh      |   4 +++-
 5 files changed, 41 insertions(+), 19 deletions(-)

diffs (182 lines):

diff -r 12e3be8203a5 -r f54586c894e3 src/cpu/pred/2bit_local.hh
--- a/src/cpu/pred/2bit_local.hh        Wed Sep 03 07:42:35 2014 -0400
+++ b/src/cpu/pred/2bit_local.hh        Wed Sep 03 07:42:36 2014 -0400
@@ -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 @@
      */
     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 -r 12e3be8203a5 -r f54586c894e3 src/cpu/pred/bpred_unit.hh
--- a/src/cpu/pred/bpred_unit.hh        Wed Sep 03 07:42:35 2014 -0400
+++ b/src/cpu/pred/bpred_unit.hh        Wed Sep 03 07:42:36 2014 -0400
@@ -178,6 +178,13 @@
      */
     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 @@
                          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 @@
         /** 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 @@
 
         /** 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 -r 12e3be8203a5 -r f54586c894e3 src/cpu/pred/bpred_unit_impl.hh
--- a/src/cpu/pred/bpred_unit_impl.hh   Wed Sep 03 07:42:35 2014 -0400
+++ b/src/cpu/pred/bpred_unit_impl.hh   Wed Sep 03 07:42:36 2014 -0400
@@ -372,8 +372,12 @@
     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 @@
 
         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 @@
                         " 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 -r 12e3be8203a5 -r f54586c894e3 src/cpu/pred/tournament.cc
--- a/src/cpu/pred/tournament.cc        Wed Sep 03 07:42:35 2014 -0400
+++ b/src/cpu/pred/tournament.cc        Wed Sep 03 07:42:36 2014 -0400
@@ -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 @@
                 }
              }
 
+        } 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);
@@ -358,6 +358,13 @@
 }
 
 void
+TournamentBP::retireSquashed(void *bp_history)
+{
+    BPHistory *history = static_cast<BPHistory *>(bp_history);
+    delete history;
+}
+
+void
 TournamentBP::squash(void *bp_history)
 {
     BPHistory *history = static_cast<BPHistory *>(bp_history);
diff -r 12e3be8203a5 -r f54586c894e3 src/cpu/pred/tournament.hh
--- a/src/cpu/pred/tournament.hh        Wed Sep 03 07:42:35 2014 -0400
+++ b/src/cpu/pred/tournament.hh        Wed Sep 03 07:42:36 2014 -0400
@@ -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 @@
      */
     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
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to