derek/gem5
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

inorder: Return Address Stack bug

Browse Source 
the nextPC was getting sent to the branch predictor not the current PC, so
the RAS was returning the wrong PC and mispredicting everything.
This commit is contained in:
Korey Sewell
2010-06-25 17:42:35 -04:00
parent 6bfd766f2c
commit 868181f24d
2 changed files with 16 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
src/cpu/inorder/resources/bpred_unit.cc
View 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);

5
src/cpu/inorder/resources/bpred_unit.hh
View 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 {