Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing checkEarlyBranchMisprediction() function #446

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/include/simeng/Instruction.hh
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,6 @@ class Instruction {
/** Retrieve supplied memory data. */
virtual span<const RegisterValue> getData() const = 0;

/** Early misprediction check; see if it's possible to determine whether the
* next instruction address was mispredicted without executing the
* instruction. Returns a {mispredicted, target} tuple representing whether
* the instruction was mispredicted, and the correct target address. */
virtual std::tuple<bool, uint64_t> checkEarlyBranchMisprediction() const = 0;

/** Retrieve branch type. */
virtual BranchType getBranchType() const = 0;

Expand Down
6 changes: 0 additions & 6 deletions src/include/simeng/arch/aarch64/Instruction.hh
Original file line number Diff line number Diff line change
Expand Up @@ -338,12 +338,6 @@ class Instruction : public simeng::Instruction {
/** Retrieve supplied memory data. */
span<const RegisterValue> getData() const override;

/** Early misprediction check; see if it's possible to determine whether the
* next instruction address was mispredicted without executing the
* instruction. Returns a {mispredicted, target} tuple representing whether
* the instruction was mispredicted, and the correct target address. */
std::tuple<bool, uint64_t> checkEarlyBranchMisprediction() const override;

/** Retrieve branch type. */
BranchType getBranchType() const override;

Expand Down
6 changes: 0 additions & 6 deletions src/include/simeng/arch/riscv/Instruction.hh
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,6 @@ class Instruction : public simeng::Instruction {
/** Retrieve supplied memory data. */
span<const RegisterValue> getData() const override;

/** Early misprediction check; see if it's possible to determine whether the
* next instruction address was mispredicted without executing the
* instruction. Returns a {mispredicted, target} tuple representing whether
* the instruction was mispredicted, and the correct target address. */
std::tuple<bool, uint64_t> checkEarlyBranchMisprediction() const override;

/** Retrieve branch type. */
BranchType getBranchType() const override;

Expand Down
15 changes: 0 additions & 15 deletions src/lib/arch/aarch64/Instruction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,21 +100,6 @@ span<const RegisterValue> Instruction::getData() const {
return {memoryData_.data(), memoryData_.size()};
}

std::tuple<bool, uint64_t> Instruction::checkEarlyBranchMisprediction() const {
assert(
!executed_ &&
"Early branch misprediction check shouldn't be called after execution");

if (!isBranch()) {
// Instruction isn't a branch; if predicted as taken, it will require a
// flush
return {prediction_.isTaken, instructionAddress_ + 4};
}

// Not enough information to determine this was a misprediction
return {false, 0};
}

BranchType Instruction::getBranchType() const { return branchType_; }

int64_t Instruction::getKnownOffset() const { return knownOffset_; }
Expand Down
15 changes: 0 additions & 15 deletions src/lib/arch/riscv/Instruction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,21 +95,6 @@ span<const RegisterValue> Instruction::getData() const {
return {memoryData_.data(), memoryData_.size()};
}

std::tuple<bool, uint64_t> Instruction::checkEarlyBranchMisprediction() const {
assert(
!executed_ &&
"Early branch misprediction check shouldn't be called after execution");

if (!isBranch()) {
// Instruction isn't a branch; if predicted as taken, it will require a
// flush
return {prediction_.isTaken, instructionAddress_ + 4};
}

// Not enough information to determine this was a misprediction
return {false, 0};
}

BranchType Instruction::getBranchType() const { return branchType_; }

int64_t Instruction::getKnownOffset() const { return knownOffset_; }
Expand Down
38 changes: 1 addition & 37 deletions src/lib/pipeline/DecodeUnit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,44 +48,8 @@ void DecodeUnit::tick() {
if (!microOps_.size()) break;

// Move uop to output buffer and remove from internal buffer
auto& uop = (output_.getTailSlots()[slot] = std::move(microOps_.front()));
output_.getTailSlots()[slot] = std::move(microOps_.front());
microOps_.pop_front();

// Check preliminary branch prediction results now that the instruction is
// decoded. Identifies:
// - Non-branch instructions mistakenly predicted as branches
// - Incorrect targets for immediate branches
auto [misprediction, correctAddress] = uop->checkEarlyBranchMisprediction();
if (misprediction) {
earlyFlushes_++;
shouldFlush_ = true;
pc_ = correctAddress;

if (!uop->isBranch()) {
// Non-branch incorrectly predicted as a branch; let the predictor know
predictor_.update(uop->getInstructionAddress(), false, pc_,
uop->getBranchType(), uop->getInstructionId());
}
// Remove macro-operations in microOps_ buffer after macro-operation
// decoded in this cycle
auto uopIt = microOps_.begin();
// Find first microOps_ entry not belonging to same address as flushing
// instruction
while (uopIt != microOps_.end()) {
if ((*uopIt)->getInstructionAddress() != uop->getInstructionAddress()) {
break;
} else {
uopIt++;
}
}
// Remove all entries after first macro-operation in buffer
while (uopIt != microOps_.end()) {
uopIt = microOps_.erase(uopIt);
}

// Skip processing remaining uops, as they need to be flushed
break;
}
}
}

Expand Down
40 changes: 0 additions & 40 deletions test/unit/aarch64/InstructionTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -461,38 +461,6 @@ TEST_F(AArch64InstructionTest, supplyData_dataAbort) {
EXPECT_EQ(insn.getException(), InstructionException::DataAbort);
}

// Test to check logic around early branch misprediction logic
TEST_F(AArch64InstructionTest, earlyBranchMisprediction) {
// Insn is `fdivr z1.s, p0/m, z1.s, z0.s`
Instruction insn = Instruction(arch, *fdivMetadata.get(), MicroOpInfo());
insn.setInstructionAddress(64);

// Check initial state of an instruction's branch related options
BranchPrediction pred = {false, 0};
bool matchingPred = (insn.getBranchPrediction() == pred);
EXPECT_TRUE(matchingPred);
EXPECT_FALSE(insn.wasBranchTaken());
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Unknown);
EXPECT_FALSE(insn.isBranch());
std::tuple<bool, uint64_t> tup = {false, insn.getInstructionAddress() + 4};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);

// Set prediction and ensure expected state changes / outcomes are seen
pred = {true, 0x4848};
insn.setBranchPrediction(pred);
matchingPred = (insn.getBranchPrediction() == pred);
EXPECT_TRUE(matchingPred);
EXPECT_FALSE(insn.wasBranchTaken());
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Unknown);
// Check logic of `checkEarlyBranchMisprediction` which is different for
// non-branch instructions
EXPECT_FALSE(insn.isBranch());
tup = {true, insn.getInstructionAddress() + 4};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);
}

// Test that a correct prediction (branch taken) is handled correctly
TEST_F(AArch64InstructionTest, correctPred_taken) {
// insn is `cbz x2, #0x28`
Expand All @@ -507,8 +475,6 @@ TEST_F(AArch64InstructionTest, correctPred_taken) {
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Conditional);
EXPECT_TRUE(insn.isBranch());
std::tuple<bool, uint64_t> tup = {false, 0};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);

// Test a correct prediction where branch is taken is handled correctly
pred = {true, 80 + 0x28};
Expand Down Expand Up @@ -536,8 +502,6 @@ TEST_F(AArch64InstructionTest, correctPred_notTaken) {
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Conditional);
EXPECT_TRUE(insn.isBranch());
std::tuple<bool, uint64_t> tup = {false, 0};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);

// Test a correct prediction where a branch isn't taken is handled correctly
pred = {false, 80 + 4};
Expand Down Expand Up @@ -565,8 +529,6 @@ TEST_F(AArch64InstructionTest, incorrectPred_target) {
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Conditional);
EXPECT_TRUE(insn.isBranch());
std::tuple<bool, uint64_t> tup = {false, 0};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);

// Test an incorrect prediction is handled correctly - target is wrong
pred = {true, 80 + 0x28};
Expand Down Expand Up @@ -594,8 +556,6 @@ TEST_F(AArch64InstructionTest, incorrectPred_taken) {
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Conditional);
EXPECT_TRUE(insn.isBranch());
std::tuple<bool, uint64_t> tup = {false, 0};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);

// Test an incorrect prediction is handled correctly - taken is wrong
pred = {true, 100 + 0x28};
Expand Down
29 changes: 0 additions & 29 deletions test/unit/pipeline/DecodeUnitTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ TEST_F(PipelineDecodeUnitTest, TickEmpty) {
TEST_F(PipelineDecodeUnitTest, Tick) {
input.getHeadSlots()[0] = {uopPtr};

EXPECT_CALL(*uop, checkEarlyBranchMisprediction())
.WillOnce(Return(std::tuple<bool, uint64_t>(false, 0)));

decodeUnit.tick();

// Check result uop is the same as the one provided
Expand All @@ -67,32 +64,6 @@ TEST_F(PipelineDecodeUnitTest, Tick) {
EXPECT_EQ(decodeUnit.getEarlyFlushes(), 0);
}

// Tests that the decode unit requests a flush when a non-branch is mispredicted
TEST_F(PipelineDecodeUnitTest, Flush) {
input.getHeadSlots()[0] = {uopPtr};

uop->setInstructionAddress(2);

// Return branch type as unconditional by default
ON_CALL(*uop, getBranchType())
.WillByDefault(Return(BranchType::Unconditional));

EXPECT_CALL(*uop, checkEarlyBranchMisprediction())
.WillOnce(Return(std::tuple<bool, uint64_t>(true, 1)));
EXPECT_CALL(*uop, isBranch()).WillOnce(Return(false));

// Check the predictor is updated with the correct instruction address and PC
EXPECT_CALL(predictor, update(2, false, 1, BranchType::Unconditional,
uop->getInstructionId()));

decodeUnit.tick();

// Check that a flush was correctly requested
EXPECT_EQ(decodeUnit.shouldFlush(), true);
EXPECT_EQ(decodeUnit.getFlushAddress(), 1);
EXPECT_EQ(decodeUnit.getEarlyFlushes(), 1);
}

// Tests that PurgeFlushed empties the microOps queue
TEST_F(PipelineDecodeUnitTest, purgeFlushed) {
input.getHeadSlots()[0] = {uopPtr, uop2Ptr};
Expand Down
40 changes: 0 additions & 40 deletions test/unit/riscv/InstructionTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -435,38 +435,6 @@ TEST_F(RiscVInstructionTest, supplyData_dataAbort) {
EXPECT_EQ(insn.getException(), InstructionException::DataAbort);
}

// Test to check logic around early branch misprediction logic
TEST_F(RiscVInstructionTest, earlyBranchMisprediction) {
// Insn is `div a3, a3, a0`
Instruction insn = Instruction(arch, *divMetadata.get());
insn.setInstructionAddress(64);

// Check initial state of an instruction's branch related options
BranchPrediction pred = {false, 0};
bool matchingPred = (insn.getBranchPrediction() == pred);
EXPECT_TRUE(matchingPred);
EXPECT_FALSE(insn.wasBranchTaken());
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Unknown);
EXPECT_FALSE(insn.isBranch());
std::tuple<bool, uint64_t> tup = {false, insn.getInstructionAddress() + 4};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);

// Set prediction and ensure expected state changes / outcomes are seen
pred = {true, 0x4848};
insn.setBranchPrediction(pred);
matchingPred = (insn.getBranchPrediction() == pred);
EXPECT_TRUE(matchingPred);
EXPECT_FALSE(insn.wasBranchTaken());
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Unknown);
// Check logic of `checkEarlyBranchMisprediction` which is different for
// non-branch instructions
EXPECT_FALSE(insn.isBranch());
tup = {true, insn.getInstructionAddress() + 4};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);
}

// Test that a correct prediction (branch taken) is handled correctly
TEST_F(RiscVInstructionTest, correctPred_taken) {
// insn is `bgeu a5, a4, -86`
Expand All @@ -481,8 +449,6 @@ TEST_F(RiscVInstructionTest, correctPred_taken) {
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Conditional);
EXPECT_TRUE(insn.isBranch());
std::tuple<bool, uint64_t> tup = {false, 0};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);

// Test a correct prediction where branch is taken is handled correctly
pred = {true, 400 - 86};
Expand Down Expand Up @@ -511,8 +477,6 @@ TEST_F(RiscVInstructionTest, correctPred_notTaken) {
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Conditional);
EXPECT_TRUE(insn.isBranch());
std::tuple<bool, uint64_t> tup = {false, 0};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);

// Test a correct prediction where a branch isn't taken is handled correctly
// imm operand 0x28 has 4 added implicitly by dissassembler
Expand Down Expand Up @@ -542,8 +506,6 @@ TEST_F(RiscVInstructionTest, incorrectPred_target) {
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Conditional);
EXPECT_TRUE(insn.isBranch());
std::tuple<bool, uint64_t> tup = {false, 0};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);

// Test an incorrect prediction is handled correctly - target is wrong
// imm operand 0x28 has 4 added implicitly by dissassembler
Expand Down Expand Up @@ -573,8 +535,6 @@ TEST_F(RiscVInstructionTest, incorrectPred_taken) {
EXPECT_EQ(insn.getBranchAddress(), 0);
EXPECT_EQ(insn.getBranchType(), BranchType::Conditional);
EXPECT_TRUE(insn.isBranch());
std::tuple<bool, uint64_t> tup = {false, 0};
EXPECT_EQ(insn.checkEarlyBranchMisprediction(), tup);

// Test an incorrect prediction is handled correctly - taken is wrong
// imm operand 0x28 has 4 added implicitly by dissassembler
Expand Down
Loading