From 96f9372a816c784d043e553cc086c8b567482c76 Mon Sep 17 00:00:00 2001 From: Arthur Perais Date: Tue, 1 Jun 2021 11:30:58 +0200 Subject: [PATCH] cpu-o3: Prevent a mistarget from sending execution on an incorrect path This fixes the unlikely but possible following case : - Assume cond/uncond direct branch A jumping to next branch (PC + 4 in ARM). From the point of view of the PCState object, the instruction is not branching (PCState::branching() will return false since it tests whether nextPC != PC + 4 for ARM). This gets cached in the BTB. - Assume another cond branch B that is predicted taken but uses the PCState object of the first branch A from the BTB due to a partial tag match (BTB is not fully tagged). - At decode, the mistarget will be detected because the target given by the BTB does not match the target encoded in the instruction B. However, to determine what PC to send to fetch, decode looks at inst->pcState().branching(), which returns false because the PCState object has PC X, and nextPC X + 4 (ARM case). Therefore, Decode sends the fallthrough address of branch B, despite it being predicted taken. If the prediction is correct, Exec will not realize that the target is wrong since it is the Decode stage's job. Jira Issue: https://gem5.atlassian.net/browse/GEM5-947 Change-Id: Ia3b960bb660bdfd3c348988d6532735fa3268990 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/46260 Reviewed-by: Nathanael Premillieu Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- src/cpu/o3/decode.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/cpu/o3/decode.cc b/src/cpu/o3/decode.cc index 993ab73385..c568f5d79f 100644 --- a/src/cpu/o3/decode.cc +++ b/src/cpu/o3/decode.cc @@ -290,11 +290,18 @@ Decode::squash(const DynInstPtr &inst, ThreadID tid) toFetch->decodeInfo[tid].squash = true; toFetch->decodeInfo[tid].doneSeqNum = inst->seqNum; toFetch->decodeInfo[tid].nextPC = inst->branchTarget(); - toFetch->decodeInfo[tid].branchTaken = inst->pcState().branching(); + + // Looking at inst->pcState().branching() + // may yield unexpected results if the branch + // was predicted taken but aliased in the BTB + // with a branch jumping to the next instruction (mistarget) + // Using PCState::branching() will send execution on the + // fallthrough and this will not be caught at execution (since + // branch was correctly predicted taken) + toFetch->decodeInfo[tid].branchTaken = inst->readPredTaken() | + inst->isUncondCtrl(); + toFetch->decodeInfo[tid].squashInst = inst; - if (toFetch->decodeInfo[tid].mispredictInst->isUncondCtrl()) { - toFetch->decodeInfo[tid].branchTaken = true; - } InstSeqNum squash_seq_num = inst->seqNum;