Skip to content

Commit

Permalink
fix: always set prediction target for BP #84
Browse files Browse the repository at this point in the history
**Description**
If the instruction is predicted as branch but it is not a branch, it
does not trigger a branch misprediction.

**Steps to Reproduce**
Run data caching image with 2 coresAfter some time an instruction is
predicted as a taken branch modifying the PCExpected BehaviorAfter we
dispatch we should rollback the next instructions and give branch
feedback to the frontend

**Actual Behavior**
No rollback happens when we notice the instruction is not a branch
  • Loading branch information
xusine authored and branylagaffe committed Nov 28, 2024
1 parent c8f01c7 commit 8f93ccb
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 7 deletions.
3 changes: 2 additions & 1 deletion components/Decoder/Instruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ ArchInstruction::doDispatchEffects()
auto bp_state = bpState();

DBG_Assert(bp_state, (<< "No branch predictor state exists, but it must"));
if (bp_state->theActualType == kNonBranch) return;
if (bp_state->thePredictedType == kNonBranch) return;
if (isBranch()) return;

// Branch predictor identified an instruction that is not a branch as a branch.
Expand All @@ -98,6 +98,7 @@ ArchInstruction::doDispatchEffects()
feedback->theActualTarget = VirtualMemoryAddress(0);
feedback->theBPState = bpState();
core()->branchFeedback(feedback);
if (core()->squashFrom(dynamic_cast<Instruction*>(this), false)) { core()->redirectFetch(pc() + 4); }
}

bool
Expand Down
12 changes: 10 additions & 2 deletions components/FetchAddressGenerate/FetchAddressGenerateImpl.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@

#include "components/uFetch/uFetchTypes.hpp"
#include <components/FetchAddressGenerate/FetchAddressGenerate.hpp>

#define FLEXUS_BEGIN_COMPONENT FetchAddressGenerate
Expand Down Expand Up @@ -141,19 +142,26 @@ class FLEXUS_COMPONENT(FetchAddressGenerate)
AGU_DBG("Getting addresses: " << max_addrs << " remaining");

FetchAddr faddr(thePC[anIndex]);
faddr.theBPState->pc = thePC[anIndex];

// Advance the PC
if (theBranchPredictor->isBranch(faddr.theAddress)) {
AGU_DBG("Predicting a Branch");
faddr.theBPState->thePredictedType = kUnconditional;
if (max_predicts == 0) {
AGU_DBG("Config set the max prediction to zero, so no prediction");
break;
}
VirtualMemoryAddress prediction = theBranchPredictor->predict(faddr.theAddress, *faddr.theBPState);
if (prediction == 0)
if (prediction == 0) {
thePC[anIndex] += 4;
else
faddr.theBPState->thePrediction = kNotTaken;
}
else {
thePC[anIndex] = prediction;
faddr.theBPState->thePrediction = kTaken;
}
faddr.theBPState->thePredictedTarget = thePC[anIndex];
AGU_DBG("Advancing PC to: " << thePC[anIndex] << " for core: " << anIndex);
AGU_DBG("Enqueing Fetch Thread[" << anIndex << "] " << faddr.theAddress);

Expand Down
2 changes: 1 addition & 1 deletion components/uArch/CoreModel/coreModelImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ class CoreImpl : public CoreModel
// Squashing & Front-end control
//==========================================================================
public:
bool squashFrom(boost::intrusive_ptr<Instruction> anInsn);
bool squashFrom(boost::intrusive_ptr<Instruction> anInsn, bool inclusive = true);
void redirectFetch(VirtualMemoryAddress anAddress);
void branchFeedback(boost::intrusive_ptr<BranchFeedback> feedback);

Expand Down
4 changes: 2 additions & 2 deletions components/uArch/CoreModel/cycle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1469,14 +1469,14 @@ CoreImpl::commit(boost::intrusive_ptr<Instruction> anInstruction)
}

bool
CoreImpl::squashFrom(boost::intrusive_ptr<Instruction> anInsn)
CoreImpl::squashFrom(boost::intrusive_ptr<Instruction> anInsn, bool inclusive)
{
if (!theSquashRequested || (anInsn->sequenceNo() <= (*theSquashInstruction)->sequenceNo())) {
theSquashRequested = true;
theSquashReason = kBranchMispredict;
theEmptyROBCause = kMispredict;
theSquashInstruction = theROB.project<0>(theROB.get<by_insn>().find(anInsn));
theSquashInclusive = true;
theSquashInclusive = inclusive;
return true;
}
return false;
Expand Down
2 changes: 1 addition & 1 deletion components/uArch/uArchInterfaces.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ struct uArch
{
DBG_Assert(false);
}
virtual bool squashFrom(boost::intrusive_ptr<Instruction> anInsn)
virtual bool squashFrom(boost::intrusive_ptr<Instruction> anInsn, bool inclusive = true)
{
DBG_Assert(false);
return false;
Expand Down

0 comments on commit 8f93ccb

Please sign in to comment.