inorder: Return Address Stack bug
authorKorey Sewell <ksewell@umich.edu>
Fri, 25 Jun 2010 21:42:35 +0000 (17:42 -0400)
committerKorey Sewell <ksewell@umich.edu>
Fri, 25 Jun 2010 21:42:35 +0000 (17:42 -0400)
the nextPC was getting sent to the branch predictor not the current PC, so
the RAS was returning the wrong PC and mispredicting everything.

src/cpu/inorder/resources/bpred_unit.cc
src/cpu/inorder/resources/bpred_unit.hh

index 0a78e518ebdbd33a3d544da05fe4f2bfd2e7b319..b08a393f7f200ce7b32e05a7308b24cd1f7e0b1c 100644 (file)
@@ -66,7 +66,9 @@ BPredUnit::BPredUnit(Resource *_res, ThePipeline::Params *params)
     }
 
     for (int i=0; i < ThePipeline::MaxThreads; i++)
-       RAS[i].init(params->RASSize);
+        RAS[i].init(params->RASSize);
+
+    instSize = sizeof(TheISA::MachInst);
 }
 
 std::string
@@ -147,7 +149,7 @@ BPredUnit::takeOverFrom()
 
 
 bool
-BPredUnit::predict(DynInstPtr &inst, Addr &PC, ThreadID tid)
+BPredUnit::predict(DynInstPtr &inst, Addr &pred_PC, ThreadID tid)
 {
     // See if branch predictor predicts taken.
     // If so, get its target addr either from the BTB or the RAS.
@@ -183,14 +185,14 @@ BPredUnit::predict(DynInstPtr &inst, Addr &PC, ThreadID tid)
     } else {
         ++condPredicted;
 
-        pred_taken = BPLookup(PC, bp_history);
+        pred_taken = BPLookup(pred_PC, bp_history);
 
         DPRINTF(InOrderBPred, "[tid:%i]: Branch predictor predicted %i "
                 "for PC %#x\n",
                 tid, pred_taken, inst->readPC());
     }
 
-    PredictorHistory predict_record(inst->seqNum, PC, pred_taken, 
+    PredictorHistory predict_record(inst->seqNum, pred_PC, pred_taken,
                                     bp_history, tid);
 
     // Now lookup in the BTB or RAS.
@@ -218,10 +220,11 @@ BPredUnit::predict(DynInstPtr &inst, Addr &PC, ThreadID tid)
             ++BTBLookups;
 
             if (inst->isCall()) {
+
 #if ISA_HAS_DELAY_SLOT
-                Addr ras_pc = PC + (2 * sizeof(MachInst)); // Next Next PC
+                Addr ras_pc = pred_PC + instSize; // Next Next PC
 #else
-                Addr ras_pc = PC + sizeof(MachInst); // Next PC
+                Addr ras_pc = pred_PC; // Next PC
 #endif
 
                 RAS[tid].push(ras_pc);
@@ -243,11 +246,11 @@ BPredUnit::predict(DynInstPtr &inst, Addr &PC, ThreadID tid)
                 DPRINTF(InOrderBPred, "[tid:%i]: Setting %#x predicted"
                         " target to %#x.\n",
                         tid, inst->readPC(), target);
-            } else if (BTB.valid(PC, asid)) {
+            } else if (BTB.valid(pred_PC, asid)) {
                 ++BTBHits;
 
                 // If it's not a return, use the BTB to get the target addr.
-                target = BTB.lookup(PC, asid);
+                target = BTB.lookup(pred_PC, asid);
 
                 DPRINTF(InOrderBPred, "[tid:%i]: [asid:%i] Instruction %#x "
                         "predicted target is %#x.\n",
@@ -262,16 +265,14 @@ BPredUnit::predict(DynInstPtr &inst, Addr &PC, ThreadID tid)
 
     if (pred_taken) {
         // Set the PC and the instruction's predicted target.
-        PC = target;
-        inst->setPredTarg(target);
+        pred_PC = target;
     } else {
 #if ISA_HAS_DELAY_SLOT
         // This value will be inst->PC + 4 (nextPC)
         // Delay Slot archs need this to be inst->PC + 8 (nextNPC)
         // so we increment one more time here.
-        PC = PC + sizeof(MachInst);
+        pred_PC = pred_PC + instSize;
 #endif
-        inst->setPredTarg(PC);
     }
 
     predHist[tid].push_front(predict_record);
index 8f4ef593d63d064a9ba000e5851316fde9c29b1b..881bfcc95b0d435fdf299d781cd3d8ad287fb763 100644 (file)
@@ -83,11 +83,11 @@ class BPredUnit
      * Predicts whether or not the instruction is a taken branch, and the
      * target of the branch if it is taken.
      * @param inst The branch instruction.
-     * @param PC The predicted PC is passed back through this parameter.
+     * @param pred_PC The predicted PC is passed back through this parameter.
      * @param tid The thread id.
      * @return Returns if the branch is taken or not.
      */
-    bool predict(ThePipeline::DynInstPtr &inst, Addr &PC, ThreadID tid);
+    bool predict(ThePipeline::DynInstPtr &inst, Addr &pred_PC, ThreadID tid);
 
     // @todo: Rename this function.
     void BPUncond(void * &bp_history);
@@ -173,6 +173,7 @@ class BPredUnit
     void dump();
 
   private:
+    int instSize;
     Resource *res;
     
     struct PredictorHistory {