diff options
author | Arthur Perais <arthur.perais@inria.fr> | 2016-12-21 15:07:16 -0600 |
---|---|---|
committer | Arthur Perais <arthur.perais@inria.fr> | 2016-12-21 15:07:16 -0600 |
commit | 497cc2d373d1559aaae0263635b88f670fd239cd (patch) | |
tree | 7ad8f55f589fe1e04bc3bc3764794c83e13f1788 /src/cpu/pred/bi_mode.cc | |
parent | 34065f8d5f51e165b56d12a6d88092332809f0b9 (diff) | |
download | gem5-497cc2d373d1559aaae0263635b88f670fd239cd.tar.xz |
cpu: disallow speculative update of branch predictor tables (o3)
The Minor and o3 cpu models share the branch prediction
code. Minor relies on the BPredUnit::squash() function
to update the branch predictor tables on a branch mispre-
diction. This is fine because Minor executes in-order, so
the update is on the correct path. However, this causes the
branch predictor to be updated on out-of-order branch
mispredictions when using the o3 model, which should not
be the case.
This patch guards against speculative update of the branch
prediction tables. On a branch misprediction, BPredUnit::squash()
calls BpredUnit::update(..., squashed = true). The underlying
branch predictor tests against the value of squashed. If it is
true, it restores any speculatively updated internal state
it might have (e.g., global/local branch history), then returns.
If false, it updates its prediction tables. Previously, exist-
ing predictors did not test against the "squashed" parameter.
To accomodate for this change, the Minor model must now call
BPredUnit::squash() then BPredUnit::update(..., squashed = false)
on branch mispredictions. Before, calling BpredUnit::squash()
performed the prediction tables update.
The effect is a slight MPKI improvement when using the o3
model. A further patch should perform the same modifications
for the indirect target predictor and BTB (less critical).
Signed-off-by: Jason Lowe-Power <jason@lowepower.com>
Diffstat (limited to 'src/cpu/pred/bi_mode.cc')
-rw-r--r-- | src/cpu/pred/bi_mode.cc | 103 |
1 files changed, 47 insertions, 56 deletions
diff --git a/src/cpu/pred/bi_mode.cc b/src/cpu/pred/bi_mode.cc index 48e798d96..355d0d8f7 100644 --- a/src/cpu/pred/bi_mode.cc +++ b/src/cpu/pred/bi_mode.cc @@ -162,78 +162,69 @@ void BiModeBP::update(ThreadID tid, Addr branchAddr, bool taken, void *bpHistory, bool squashed) { - if (bpHistory) { - BPHistory *history = static_cast<BPHistory*>(bpHistory); + assert(bpHistory); - unsigned choiceHistoryIdx = ((branchAddr >> instShiftAmt) - & choiceHistoryMask); - unsigned globalHistoryIdx = (((branchAddr >> instShiftAmt) - ^ history->globalHistoryReg) - & globalHistoryMask); + BPHistory *history = static_cast<BPHistory*>(bpHistory); - assert(choiceHistoryIdx < choicePredictorSize); - assert(globalHistoryIdx < globalPredictorSize); + // We do not update the counters speculatively on a squash. + // We just restore the global history register. + if (squashed) { + globalHistoryReg[tid] = (history->globalHistoryReg << 1) | taken; + return; + } - if (history->takenUsed) { - // if the taken array's prediction was used, update it - if (taken) { - takenCounters[globalHistoryIdx].increment(); - } else { - takenCounters[globalHistoryIdx].decrement(); - } + unsigned choiceHistoryIdx = ((branchAddr >> instShiftAmt) + & choiceHistoryMask); + unsigned globalHistoryIdx = (((branchAddr >> instShiftAmt) + ^ history->globalHistoryReg) + & globalHistoryMask); + + assert(choiceHistoryIdx < choicePredictorSize); + assert(globalHistoryIdx < globalPredictorSize); + + if (history->takenUsed) { + // if the taken array's prediction was used, update it + if (taken) { + takenCounters[globalHistoryIdx].increment(); } else { - // if the not-taken array's prediction was used, update it - if (taken) { - notTakenCounters[globalHistoryIdx].increment(); - } else { - notTakenCounters[globalHistoryIdx].decrement(); - } + takenCounters[globalHistoryIdx].decrement(); } - - if (history->finalPred == taken) { - /* If the final prediction matches the actual branch's - * outcome and the choice predictor matches the final - * outcome, we update the choice predictor, otherwise it - * is not updated. While the designers of the bi-mode - * predictor don't explicity say why this is done, one - * can infer that it is to preserve the choice predictor's - * bias with respect to the branch being predicted; afterall, - * the whole point of the bi-mode predictor is to identify the - * atypical case when a branch deviates from its bias. - */ - if (history->finalPred == history->takenUsed) { - if (taken) { - choiceCounters[choiceHistoryIdx].increment(); - } else { - choiceCounters[choiceHistoryIdx].decrement(); - } - } + } else { + // if the not-taken array's prediction was used, update it + if (taken) { + notTakenCounters[globalHistoryIdx].increment(); } else { - // always update the choice predictor on an incorrect prediction + notTakenCounters[globalHistoryIdx].decrement(); + } + } + + if (history->finalPred == taken) { + /* If the final prediction matches the actual branch's + * outcome and the choice predictor matches the final + * outcome, we update the choice predictor, otherwise it + * is not updated. While the designers of the bi-mode + * predictor don't explicity say why this is done, one + * can infer that it is to preserve the choice predictor's + * bias with respect to the branch being predicted; afterall, + * the whole point of the bi-mode predictor is to identify the + * atypical case when a branch deviates from its bias. + */ + if (history->finalPred == history->takenUsed) { if (taken) { choiceCounters[choiceHistoryIdx].increment(); } else { choiceCounters[choiceHistoryIdx].decrement(); } } - - if (squashed) { - if (taken) { - globalHistoryReg[tid] = (history->globalHistoryReg << 1) | 1; - } else { - globalHistoryReg[tid] = (history->globalHistoryReg << 1); - } - globalHistoryReg[tid] &= historyRegisterMask; + } else { + // always update the choice predictor on an incorrect prediction + if (taken) { + choiceCounters[choiceHistoryIdx].increment(); } else { - delete history; + choiceCounters[choiceHistoryIdx].decrement(); } } -} -void -BiModeBP::retireSquashed(ThreadID tid, void *bp_history) -{ - BPHistory *history = static_cast<BPHistory*>(bp_history); delete history; } |