[llvm] Revert "[BOLT][AArch64] Transform cmpbr ~> cmp + br when inversion not possible (#185731)" (PR #202309)
Alexandros Lamprineas via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 8 03:12:19 PDT 2026
https://github.com/labrinea created https://github.com/llvm/llvm-project/pull/202309
This reverts commit 6b13656fd8386f979e061cc97e32b607ee3fcdf4.
>From 438dc785b4f16eca2dbe51170f5aef5feda91951 Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Mon, 8 Jun 2026 10:41:10 +0100
Subject: [PATCH] Revert "[BOLT][AArch64] Transform cmpbr ~> cmp + br when
inversion not possible (#185731)"
This reverts commit 6b13656fd8386f979e061cc97e32b607ee3fcdf4.
---
bolt/include/bolt/Core/BinaryFunction.h | 4 +-
bolt/include/bolt/Core/MCPlusBuilder.h | 11 +-
bolt/include/bolt/Passes/LongJmp.h | 4 +-
bolt/include/bolt/Utils/CommandLineOpts.h | 4 -
bolt/lib/Core/BinaryFunction.cpp | 7 +-
bolt/lib/Passes/BinaryPasses.cpp | 16 +-
bolt/lib/Passes/LongJmp.cpp | 27 +---
.../Target/AArch64/AArch64MCPlusBuilder.cpp | 90 ++---------
bolt/lib/Target/AArch64/CMakeLists.txt | 6 +-
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 6 +-
bolt/lib/Target/X86/X86MCPlusBuilder.cpp | 6 +-
bolt/lib/Utils/CommandLineOpts.cpp | 6 -
.../AArch64/compare-and-branch-inversion.S | 118 +++++----------
bolt/unittests/Core/CMakeLists.txt | 1 -
bolt/unittests/Core/MCPlusBuilder.cpp | 140 ++----------------
15 files changed, 81 insertions(+), 365 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index 8a789421cde0f..0fdfcc5d76597 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -64,8 +64,6 @@ class DWARFUnit;
namespace bolt {
-class DataflowInfoManager;
-
using InputOffsetToAddressMapTy = std::unordered_multimap<uint64_t, uint64_t>;
/// Types of macro-fusion alignment corrections.
@@ -2454,7 +2452,7 @@ class BinaryFunction {
/// while the second successor - false/fall-through branch.
///
/// When we reverse the branch condition, the CFG is updated accordingly.
- void fixBranches(DataflowInfoManager *DIM = nullptr);
+ void fixBranches();
/// Mark function as finalized. No further optimizations are permitted.
void setFinalized() { CurrentState = State::CFG_Finalized; }
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index 69e45ca32a9b7..76b4a5fe778c0 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -52,7 +52,6 @@ namespace bolt {
class BinaryBasicBlock;
class BinaryContext;
class BinaryFunction;
-class DataflowInfoManager;
/// Different types of indirect branches encountered during disassembly.
enum class IndirectBranchType : char {
@@ -476,8 +475,7 @@ class MCPlusBuilder {
}
/// Check whether this conditional branch can be reversed
- virtual bool isReversibleBranch(const MCInst &Inst,
- DataflowInfoManager *DIM = nullptr) const {
+ virtual bool isReversibleBranch(const MCInst &Inst) const {
assert(!isUnsupportedInstruction(Inst) && isConditionalBranch(Inst) &&
"Instruction is not known conditional branch");
@@ -2138,11 +2136,8 @@ class MCPlusBuilder {
}
/// Reverses the branch condition in Inst and update its taken target to TBB.
- /// Assumes that the branch is reversible.
- virtual void
- reverseBranchCondition(BinaryBasicBlock *Parent, MCInst &Inst,
- const MCSymbol *TBB, MCContext *Ctx,
- DataflowInfoManager *DIM = nullptr) const {
+ virtual void reverseBranchCondition(MCInst &Inst, const MCSymbol *TBB,
+ MCContext *Ctx) const {
llvm_unreachable("not implemented");
}
diff --git a/bolt/include/bolt/Passes/LongJmp.h b/bolt/include/bolt/Passes/LongJmp.h
index ccdebf9e1ed0a..4633d30104d43 100644
--- a/bolt/include/bolt/Passes/LongJmp.h
+++ b/bolt/include/bolt/Passes/LongJmp.h
@@ -14,8 +14,6 @@
namespace llvm {
namespace bolt {
-class DataflowInfoManager;
-
/// LongJmp is veneer-insertion pass originally written for AArch64 that
/// compensates for its short-range branches, typically done during linking. We
/// pull this pass inside BOLT because here we can do a better job at stub
@@ -76,7 +74,7 @@ class LongJmpPass : public BinaryFunctionPass {
/// Relax all internal function branches including those between fragments.
/// Assume that fragments are placed in different sections but are within
/// 128MB of each other.
- void relaxLocalBranches(BinaryFunction &BF, DataflowInfoManager *DIM);
+ void relaxLocalBranches(BinaryFunction &BF);
/// -- Layout estimation methods --
/// Try to do layout before running the emitter, by looking at BinaryFunctions
diff --git a/bolt/include/bolt/Utils/CommandLineOpts.h b/bolt/include/bolt/Utils/CommandLineOpts.h
index 56f6048bac36c..994e352e16218 100644
--- a/bolt/include/bolt/Utils/CommandLineOpts.h
+++ b/bolt/include/bolt/Utils/CommandLineOpts.h
@@ -124,10 +124,6 @@ extern llvm::cl::opt<bool> UpdateDebugSections;
// dbgs() for output within DEBUG().
extern llvm::cl::opt<unsigned> Verbosity;
-// Option to control whether liveness analysis should be used by
-// FixupBranches and LongJmpPass. Needed for branch inversion on AArch64.
-extern llvm::cl::opt<bool> LivenessAnalysis;
-
/// Return true if we should process all functions in the binary.
bool processAllFunctions();
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 90dc6f0c31c21..0e538fa48907a 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -3632,7 +3632,7 @@ bool BinaryFunction::validateCFG() const {
return true;
}
-void BinaryFunction::fixBranches(DataflowInfoManager *DIM) {
+void BinaryFunction::fixBranches() {
assert(isSimple() && "Expected function with valid CFG.");
auto &MIB = BC.MIB;
@@ -3691,7 +3691,7 @@ void BinaryFunction::fixBranches(DataflowInfoManager *DIM) {
// Reverse branch condition and swap successors.
auto swapSuccessors = [&]() {
- if (!MIB->isReversibleBranch(*CondBranch, DIM)) {
+ if (!MIB->isReversibleBranch(*CondBranch)) {
if (opts::Verbosity) {
BC.outs() << "BOLT-INFO: unable to swap successors in " << *this
<< '\n';
@@ -3701,8 +3701,7 @@ void BinaryFunction::fixBranches(DataflowInfoManager *DIM) {
std::swap(TSuccessor, FSuccessor);
BB->swapConditionalSuccessors();
auto L = BC.scopeLock();
- MIB->reverseBranchCondition(BB, *CondBranch, TSuccessor->getLabel(),
- Ctx, DIM);
+ MIB->reverseBranchCondition(*CondBranch, TSuccessor->getLabel(), Ctx);
return true;
};
diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index c853249ba3772..89737b906e82b 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -11,10 +11,8 @@
//===----------------------------------------------------------------------===//
#include "bolt/Passes/BinaryPasses.h"
-#include "bolt/Core/BinaryFunctionCallGraph.h"
#include "bolt/Core/FunctionLayout.h"
#include "bolt/Core/ParallelUtilities.h"
-#include "bolt/Passes/DataflowInfoManager.h"
#include "bolt/Passes/ReorderAlgorithm.h"
#include "bolt/Passes/ReorderFunctions.h"
#include "bolt/Utils/CommandLineOpts.h"
@@ -546,22 +544,12 @@ bool ReorderBasicBlocks::modifyFunctionLayout(BinaryFunction &BF,
}
Error FixupBranches::runOnFunctions(BinaryContext &BC) {
- std::unique_ptr<BinaryFunctionCallGraph> CG;
- std::unique_ptr<RegAnalysis> RA;
- std::unique_ptr<DataflowInfoManager> DIM;
-
- if (opts::LivenessAnalysis) {
- CG = std::make_unique<BinaryFunctionCallGraph>(buildCallGraph(BC));
- RA = std::make_unique<RegAnalysis>(BC, &BC.getBinaryFunctions(), CG.get());
- }
for (auto &It : BC.getBinaryFunctions()) {
BinaryFunction &Function = It.second;
if (!BC.shouldEmit(Function) || !Function.isSimple())
continue;
- if (opts::LivenessAnalysis)
- DIM = std::make_unique<DataflowInfoManager>(Function, RA.get(), nullptr);
- Function.fixBranches(DIM.get());
+ Function.fixBranches();
}
return Error::success();
}
@@ -972,7 +960,7 @@ uint64_t SimplifyConditionalTailCalls::fixTailCalls(BinaryFunction &BF) {
uint64_t Count = 0;
if (CondSucc != BB) {
// Patch the new target address into the conditional branch.
- MIB->reverseBranchCondition(PredBB, *CondBranch, CalleeSymbol, Ctx);
+ MIB->reverseBranchCondition(*CondBranch, CalleeSymbol, Ctx);
// Since we reversed the condition on the branch we need to change
// the target for the unconditional branch or add a unconditional
// branch to the old target. This has to be done manually since
diff --git a/bolt/lib/Passes/LongJmp.cpp b/bolt/lib/Passes/LongJmp.cpp
index ef7292aed55aa..f085500fccbd3 100644
--- a/bolt/lib/Passes/LongJmp.cpp
+++ b/bolt/lib/Passes/LongJmp.cpp
@@ -11,9 +11,7 @@
//===----------------------------------------------------------------------===//
#include "bolt/Passes/LongJmp.h"
-#include "bolt/Core/BinaryFunctionCallGraph.h"
#include "bolt/Core/ParallelUtilities.h"
-#include "bolt/Passes/DataflowInfoManager.h"
#include "bolt/Utils/CommandLineOpts.h"
#include "llvm/Support/MathExtras.h"
@@ -658,8 +656,7 @@ Error LongJmpPass::relax(BinaryFunction &Func, bool &Modified) {
return Error::success();
}
-void LongJmpPass::relaxLocalBranches(BinaryFunction &BF,
- DataflowInfoManager *DIM) {
+void LongJmpPass::relaxLocalBranches(BinaryFunction &BF) {
BinaryContext &BC = BF.getBinaryContext();
auto &MIB = BC.MIB;
@@ -829,7 +826,7 @@ void LongJmpPass::relaxLocalBranches(BinaryFunction &BF,
// If the other successor is a fall-through, invert the condition code.
BinaryBasicBlock *NextBB =
BF->getLayout().getBasicBlockAfter(BB, /*IgnoreSplits*/ false);
- bool IsReversibleBranch = MIB->isReversibleBranch(Inst, DIM);
+ bool IsReversibleBranch = MIB->isReversibleBranch(Inst);
bool ShouldReverseBranch = BB->getConditionalSuccessor(false) == NextBB;
// Create a trampoline basic block for the fall-through target of the
@@ -847,8 +844,7 @@ void LongJmpPass::relaxLocalBranches(BinaryFunction &BF,
if (ShouldReverseBranch && IsReversibleBranch) {
BB->swapConditionalSuccessors();
auto L = BC.scopeLock();
- MIB->reverseBranchCondition(BB, Inst, NextBB->getLabel(), BC.Ctx.get(),
- DIM);
+ MIB->reverseBranchCondition(Inst, NextBB->getLabel(), BC.Ctx.get());
} else {
auto L = BC.scopeLock();
MIB->replaceBranchTarget(Inst, TrampolineBB->getLabel(), BC.Ctx.get());
@@ -933,23 +929,12 @@ Error LongJmpPass::runOnFunctions(BinaryContext &BC) {
opts::SplitStrategy != opts::SplitFunctionsStrategy::CDSplit) &&
"LongJmp cannot work with functions split in more than two fragments");
- std::unique_ptr<BinaryFunctionCallGraph> CG;
- std::unique_ptr<RegAnalysis> RA;
- std::unique_ptr<DataflowInfoManager> DIM;
-
- if (opts::LivenessAnalysis) {
- CG = std::make_unique<BinaryFunctionCallGraph>(buildCallGraph(BC));
- RA = std::make_unique<RegAnalysis>(BC, &BC.getBinaryFunctions(), CG.get());
- }
-
if (opts::CompactCodeModel) {
BC.outs()
<< "BOLT-INFO: relaxing branches for compact code model (<128MB)\n";
ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
- if (opts::LivenessAnalysis)
- DIM = std::make_unique<DataflowInfoManager>(BF, RA.get(), nullptr);
- relaxLocalBranches(BF, DIM.get());
+ relaxLocalBranches(BF);
};
ParallelUtilities::PredicateTy SkipPredicate =
@@ -974,14 +959,12 @@ Error LongJmpPass::runOnFunctions(BinaryContext &BC) {
tentativeLayout(BC, Sorted);
updateStubGroups();
for (BinaryFunction *Func : Sorted) {
- if (opts::LivenessAnalysis)
- DIM = std::make_unique<DataflowInfoManager>(*Func, RA.get(), nullptr);
if (auto E = relax(*Func, Modified))
return Error(std::move(E));
// Don't ruin non-simple functions, they can't afford to have the layout
// changed.
if (Modified && Func->isSimple())
- Func->fixBranches(DIM.get());
+ Func->fixBranches();
}
} while (Modified);
BC.outs() << "BOLT-INFO: Inserted " << NumHotStubs
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index ecb7bac9d7796..b4a9dff9d25b2 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -22,7 +22,6 @@
#include "bolt/Core/BinaryFunction.h"
#include "bolt/Core/MCInstUtils.h"
#include "bolt/Core/MCPlusBuilder.h"
-#include "bolt/Passes/DataflowInfoManager.h"
#include "llvm/BinaryFormat/ELF.h"
#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCDisassembler/MCDisassembler.h"
@@ -2046,25 +2045,6 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
exit(1);
}
- unsigned getInvertedCC(unsigned Opcode) const {
- // clang-format off
- switch (Opcode) {
- default:
- llvm_unreachable("Failed to invert condition code");
- return Opcode;
- // Compare register with immediate and branch.
- case AArch64::CBGTWri: return AArch64CC::LE;
- case AArch64::CBGTXri: return AArch64CC::LE;
- case AArch64::CBLTWri: return AArch64CC::GE;
- case AArch64::CBLTXri: return AArch64CC::GE;
- case AArch64::CBHIWri: return AArch64CC::LS;
- case AArch64::CBHIXri: return AArch64CC::LS;
- case AArch64::CBLOWri: return AArch64CC::HS;
- case AArch64::CBLOXri: return AArch64CC::HS;
- }
- // clang-format on
- }
-
unsigned getInvertedBranchOpcode(unsigned Opcode) const {
// clang-format off
switch (Opcode) {
@@ -2191,78 +2171,38 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
}
}
- bool isReversibleBranch(const MCInst &Inst,
- DataflowInfoManager *DIM = nullptr) const override {
+ bool isReversibleBranch(const MCInst &Inst) const override {
if (isCompAndBranch(Inst)) {
- bool MayClobberFlags =
- DIM ? DIM->getLivenessAnalysis().getLiveIn(Inst).test(getFlagsReg())
- : true;
unsigned InvertedOpcode = getInvertedBranchOpcode(Inst.getOpcode());
- if (needsImmDec(InvertedOpcode) && Inst.getOperand(1).getImm() == 0 &&
- MayClobberFlags)
+ if (needsImmDec(InvertedOpcode) && Inst.getOperand(1).getImm() == 0)
return false;
- if (needsImmInc(InvertedOpcode) && Inst.getOperand(1).getImm() == 63 &&
- MayClobberFlags)
+ if (needsImmInc(InvertedOpcode) && Inst.getOperand(1).getImm() == 63)
return false;
}
return MCPlusBuilder::isReversibleBranch(Inst);
}
- void
- reverseBranchCondition(BinaryBasicBlock *Parent, MCInst &Inst,
- const MCSymbol *TBB, MCContext *Ctx,
- DataflowInfoManager *DIM = nullptr) const override {
- assert(isReversibleBranch(Inst, DIM) && "Irreversible branch");
+ void reverseBranchCondition(MCInst &Inst, const MCSymbol *TBB,
+ MCContext *Ctx) const override {
+ if (!isReversibleBranch(Inst)) {
+ errs() << "BOLT-ERROR: Cannot reverse branch " << Inst << "\n";
+ exit(1);
+ }
if (isTB(Inst) || isCB(Inst) || isCompAndBranch(Inst)) {
- bool ImmediateOutOfBounds = false;
unsigned InvertedOpcode = getInvertedBranchOpcode(Inst.getOpcode());
- assert(InvertedOpcode != 0 && "Invalid branch instruction");
+ Inst.setOpcode(InvertedOpcode);
+ assert(Inst.getOpcode() != 0 && "Invalid branch instruction");
// The FEAT_CMPBR compare-and-branch instructions cannot encode all
// the possible condition codes, therefore we either have to adjust
// the immediate value by +-1, or to swap the register operands
// when reversing the branch condition.
if (needsRegSwap(InvertedOpcode))
std::swap(Inst.getOperand(0), Inst.getOperand(1));
- else if (needsImmDec(InvertedOpcode)) {
- if (Inst.getOperand(1).getImm() == 0)
- ImmediateOutOfBounds = true;
- else
- Inst.getOperand(1).setImm(Inst.getOperand(1).getImm() - 1);
- } else if (needsImmInc(InvertedOpcode)) {
- if (Inst.getOperand(1).getImm() == 63)
- ImmediateOutOfBounds = true;
- else
- Inst.getOperand(1).setImm(Inst.getOperand(1).getImm() + 1);
- }
- if (ImmediateOutOfBounds) {
- auto is32BitVariant = [](unsigned Opcode) {
- switch (Opcode) {
- default:
- return false;
- case AArch64::CBGTWri:
- case AArch64::CBLTWri:
- case AArch64::CBHIWri:
- case AArch64::CBLOWri:
- return true;
- }
- };
- InstructionListType Code;
- MCInstBuilder Cmp =
- is32BitVariant(InvertedOpcode)
- ? MCInstBuilder(AArch64::SUBSWri).addReg(AArch64::WZR)
- : MCInstBuilder(AArch64::SUBSXri).addReg(AArch64::XZR);
- Cmp.addReg(Inst.getOperand(0).getReg())
- .addImm(Inst.getOperand(1).getImm())
- .addImm(0);
- Code.emplace_back(std::move(Cmp));
- Code.emplace_back(MCInstBuilder(AArch64::Bcc)
- .addImm(getInvertedCC(Inst.getOpcode()))
- .addExpr(MCSymbolRefExpr::create(TBB, *Ctx)));
- Parent->replaceInstruction(Parent->findInstruction(&Inst), Code);
- return;
- }
- Inst.setOpcode(InvertedOpcode);
+ else if (needsImmDec(InvertedOpcode))
+ Inst.getOperand(1).setImm(Inst.getOperand(1).getImm() - 1);
+ else if (needsImmInc(InvertedOpcode))
+ Inst.getOperand(1).setImm(Inst.getOperand(1).getImm() + 1);
} else if (Inst.getOpcode() == AArch64::Bcc) {
Inst.getOperand(0).setImm(AArch64CC::getInvertedCondCode(
static_cast<AArch64CC::CondCode>(Inst.getOperand(0).getImm())));
diff --git a/bolt/lib/Target/AArch64/CMakeLists.txt b/bolt/lib/Target/AArch64/CMakeLists.txt
index e28ed0bd66ba1..1e171748aece6 100644
--- a/bolt/lib/Target/AArch64/CMakeLists.txt
+++ b/bolt/lib/Target/AArch64/CMakeLists.txt
@@ -29,11 +29,7 @@ add_llvm_library(LLVMBOLTTargetAArch64
AArch64CommonTableGen
)
-target_link_libraries(LLVMBOLTTargetAArch64 PRIVATE
- LLVMBOLTCore
- LLVMBOLTPasses
- LLVMBOLTUtils
- )
+target_link_libraries(LLVMBOLTTargetAArch64 PRIVATE LLVMBOLTCore LLVMBOLTUtils)
include_directories(
${LLVM_MAIN_SRC_DIR}/lib/Target/AArch64
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index b954a392c4f26..d1a0572277874 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -162,10 +162,8 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
}
}
- void
- reverseBranchCondition(BinaryBasicBlock *Parent, MCInst &Inst,
- const MCSymbol *TBB, MCContext *Ctx,
- DataflowInfoManager *DIM = nullptr) const override {
+ void reverseBranchCondition(MCInst &Inst, const MCSymbol *TBB,
+ MCContext *Ctx) const override {
auto Opcode = getInvertedBranchOpcode(Inst.getOpcode());
Inst.setOpcode(Opcode);
replaceBranchTarget(Inst, TBB, Ctx);
diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
index ecc491f1afeb0..923de64be58c8 100644
--- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
+++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
@@ -2806,10 +2806,8 @@ class X86MCPlusBuilder : public MCPlusBuilder {
Inst.addOperand(MCOperand::createImm(CC));
}
- void
- reverseBranchCondition(BinaryBasicBlock *Parent, MCInst &Inst,
- const MCSymbol *TBB, MCContext *Ctx,
- DataflowInfoManager *DIM = nullptr) const override {
+ void reverseBranchCondition(MCInst &Inst, const MCSymbol *TBB,
+ MCContext *Ctx) const override {
unsigned InvCC = getInvertedCondCode(getCondCode(Inst));
assert(InvCC != X86::COND_INVALID && "invalid branch instruction");
Inst.getOperand(Info->get(Inst.getOpcode()).NumOperands - 1).setImm(InvCC);
diff --git a/bolt/lib/Utils/CommandLineOpts.cpp b/bolt/lib/Utils/CommandLineOpts.cpp
index 9b18f4043756c..ee34b7075ee31 100644
--- a/bolt/lib/Utils/CommandLineOpts.cpp
+++ b/bolt/lib/Utils/CommandLineOpts.cpp
@@ -323,12 +323,6 @@ cl::opt<unsigned>
cl::init(0), cl::ZeroOrMore, cl::cat(BoltCategory),
cl::sub(cl::SubCommand::getAll()));
-cl::opt<bool> LivenessAnalysis(
- "liveness-analysis",
- cl::desc("use liveness analysis in FixupBranches and LongJmpPass"
- "(needed for branch inversion on AArch64)"),
- cl::init(false), cl::cat(BoltCategory));
-
bool processAllFunctions() {
if (opts::AggregateOnly)
return false;
diff --git a/bolt/test/AArch64/compare-and-branch-inversion.S b/bolt/test/AArch64/compare-and-branch-inversion.S
index 0ea084e904101..28167416c31cb 100644
--- a/bolt/test/AArch64/compare-and-branch-inversion.S
+++ b/bolt/test/AArch64/compare-and-branch-inversion.S
@@ -1,24 +1,18 @@
# This test checks that branch inversion works when reordering blocks which
# contain short range conditional branches. Handles edge cases, like when
# the immediate value is the upper or lower allowed value in which case the
-# transformation bails. If liveness analysis proves that the condition flags
-# are dead we can replace the branch with cmp + b.cc
+# transformation bails.
# REQUIRES: system-linux, asserts
# RUN: %clang %cflags -march=armv9-a+cmpbr -Wl,-q %s -o %t
# RUN: link_fdata --no-lbr %s %t %t.fdata
# RUN: llvm-strip --strip-unneeded %t
-#
# RUN: llvm-bolt -v=1 %t -o %t.bolt --data %t.fdata --reorder-blocks=ext-tsp --compact-code-model \
-# RUN: | FileCheck %s --check-prefix=BOLT-INFO-NO-LIVENESS
-# RUN: llvm-objdump -d %t.bolt | FileCheck %s --check-prefix=COMMON --check-prefix=NO-LIVENESS
-#
-# RUN: llvm-bolt -v=1 %t -o %t.bolt --data %t.fdata --reorder-blocks=ext-tsp --compact-code-model \
-# RUN: --liveness-analysis | FileCheck %s --check-prefix=BOLT-INFO-LIVENESS
-# RUN: llvm-objdump -d %t.bolt | FileCheck %s --check-prefix=COMMON --check-prefix=LIVENESS
+# RUN: | FileCheck %s --check-prefix=BOLT-INFO
+# RUN: llvm-objdump -d %t.bolt | FileCheck %s
-# COMMON: Disassembly of section .text:
+# CHECK: Disassembly of section .text:
.globl immediate_increment
.type immediate_increment, %function
@@ -35,12 +29,12 @@ immediate_increment:
mov x0, #2
ret
-# COMMON: <immediate_increment>:
-# COMMON-NEXT: {{.*}} cblt x0, #0x1, 0x[[ADDR0:[0-9a-f]+]] <{{.*}}>
-# COMMON-NEXT: {{.*}} mov x0, #0x2 // =2
-# COMMON-NEXT: {{.*}} ret
-# COMMON-NEXT: [[ADDR0]]: {{.*}} mov x0, #0x1 // =1
-# COMMON-NEXT: {{.*}} ret
+# CHECK: <immediate_increment>:
+# CHECK-NEXT: {{.*}} cblt x0, #0x1, 0x[[ADDR0:[0-9a-f]+]] <{{.*}}>
+# CHECK-NEXT: {{.*}} mov x0, #0x2 // =2
+# CHECK-NEXT: {{.*}} ret
+# CHECK-NEXT: [[ADDR0]]: {{.*}} mov x0, #0x1 // =1
+# CHECK-NEXT: {{.*}} ret
.globl immediate_decrement
.type immediate_decrement, %function
@@ -57,12 +51,12 @@ immediate_decrement:
mov x0, #2
ret
-# COMMON: <immediate_decrement>:
-# COMMON-NEXT: {{.*}} cbhi x0, #0x0, 0x[[ADDR1:[0-9a-f]+]] <{{.*}}>
-# COMMON-NEXT: {{.*}} mov x0, #0x2 // =2
-# COMMON-NEXT: {{.*}} ret
-# COMMON-NEXT: [[ADDR1]]: {{.*}} mov x0, #0x1 // =1
-# COMMON-NEXT: {{.*}} ret
+# CHECK: <immediate_decrement>:
+# CHECK-NEXT: {{.*}} cbhi x0, #0x0, 0x[[ADDR1:[0-9a-f]+]] <{{.*}}>
+# CHECK-NEXT: {{.*}} mov x0, #0x2 // =2
+# CHECK-NEXT: {{.*}} ret
+# CHECK-NEXT: [[ADDR1]]: {{.*}} mov x0, #0x1 // =1
+# CHECK-NEXT: {{.*}} ret
.globl register_swap
.type register_swap, %function
@@ -79,77 +73,37 @@ register_swap:
mov x0, #2
ret
-# COMMON: <register_swap>:
-# COMMON-NEXT: {{.*}} cbgt x1, x0, 0x[[ADDR2:[0-9a-f]+]] <{{.*}}>
-# COMMON-NEXT: {{.*}} mov x0, #0x2 // =2
-# COMMON-NEXT: {{.*}} ret
-# COMMON-NEXT: [[ADDR2]]: {{.*}} mov x0, #0x1 // =1
-# COMMON-NEXT: {{.*}} ret
+# CHECK: <register_swap>:
+# CHECK-NEXT: {{.*}} cbgt x1, x0, 0x[[ADDR2:[0-9a-f]+]] <{{.*}}>
+# CHECK-NEXT: {{.*}} mov x0, #0x2 // =2
+# CHECK-NEXT: {{.*}} ret
+# CHECK-NEXT: [[ADDR2]]: {{.*}} mov x0, #0x1 // =1
+# CHECK-NEXT: {{.*}} ret
- .globl immediate_overflow
- .type immediate_overflow, %function
-immediate_overflow:
+ .globl irreversible
+ .type irreversible, %function
+irreversible:
.entry3:
-# FDATA: 1 immediate_overflow #.entry3# 10
+# FDATA: 1 irreversible #.entry3# 10
cbgt x0, #63, .exit3
.cold3:
-# FDATA: 1 immediate_overflow #.cold3# 1
+# FDATA: 1 irreversible #.cold3# 1
mov x0, #1
ret
.exit3:
-# FDATA: 1 immediate_overflow #.exit3# 10
- mov x0, #2
- ret
-
-# BOLT-INFO-NO-LIVENESS: unable to swap successors in immediate_overflow
-#
-# Without liveness the blocks get reordered, but since the branch is
-# irreversible an additional unconditional branch is emitted.
-# This codegen is suboptimal yet correct.
-#
-# NO-LIVENESS: <immediate_overflow>:
-# NO-LIVENESS-NEXT: {{.*}} cbgt x0, #0x3f, 0x[[ADDR3:[0-9a-f]+]] <{{.*}}>
-# NO-LIVENESS-NEXT: {{.*}} b 0x[[ADDR4:[0-9a-f]+]] <{{.*}}>
-# NO-LIVENESS-NEXT: [[ADDR3]]: {{.*}} mov x0, #0x2 // =2
-# NO-LIVENESS-NEXT: {{.*}} ret
-# NO-LIVENESS-NEXT: [[ADDR4]]: {{.*}} mov x0, #0x1 // =1
-# NO-LIVENESS-NEXT: {{.*}} ret
-
-# LIVENESS: <immediate_overflow>:
-# LIVENESS-NEXT: {{.*}} cmp x0, #0x3f
-# LIVENESS-NEXT: {{.*}} b.le 0x[[ADDR5:[0-9a-f]+]] <{{.*}}>
-# LIVENESS-NEXT: {{.*}} mov x0, #0x2 // =2
-# LIVENESS-NEXT: {{.*}} ret
-# LIVENESS-NEXT: [[ADDR5]]: {{.*}} mov x0, #0x1 // =1
-# LIVENESS-NEXT: {{.*}} ret
-
- .globl irreversible
- .type irreversible, %function
-irreversible:
-.entry4:
-# FDATA: 1 irreversible #.entry4# 10
- cmp x0, #63
- cbgt x0, #63, .exit4
-.cold4:
-# FDATA: 1 irreversible #.cold4# 1
- csel x0, x1, x2, le
- ret
-.exit4:
-# FDATA: 1 irreversible #.exit4# 10
+# FDATA: 1 irreversible #.exit3# 10
mov x0, #2
ret
-# BOLT-INFO-NO-LIVENESS: unable to swap successors in irreversible
-# BOLT-INFO-LIVENESS: unable to swap successors in irreversible
+# BOLT-INFO: unable to swap successors in irreversible
-# COMMON: <irreversible>:
-# COMMON-NEXT: {{.*}} cmp x0, #0x3f
-# COMMON-NEXT: {{.*}} cbgt x0, #0x3f, 0x[[ADDR6:[0-9a-f]+]] <{{.*}}>
-# COMMON-NEXT: {{.*}} b 0x[[ADDR7:[0-9a-f]+]] <{{.*}}>
-# COMMON-NEXT: [[ADDR6]]: {{.*}} mov x0, #0x2 // =2
-# COMMON-NEXT: {{.*}} ret
-# COMMON-NEXT: [[ADDR7]]: {{.*}} csel x0, x1, x2, le
-# COMMON-NEXT: {{.*}} ret
+# CHECK: <irreversible>:
+# CHECK-NEXT: {{.*}} cbgt x0, #0x3f, 0x[[ADDR3:[0-9a-f]+]] <{{.*}}>
+# CHECK-NEXT: {{.*}} b 0x[[ADDR4:[0-9a-f]+]] <{{.*}}>
+# CHECK-NEXT: [[ADDR3]]: {{.*}} mov x0, #0x2 // =2
+# CHECK-NEXT: {{.*}} ret
+# CHECK-NEXT: [[ADDR4]]: {{.*}} mov x0, #0x1 // =1
+# CHECK-NEXT: {{.*}} ret
## Force relocation mode.
.reloc 0, R_AARCH64_NONE
diff --git a/bolt/unittests/Core/CMakeLists.txt b/bolt/unittests/Core/CMakeLists.txt
index b755afc8a66da..297dec7449202 100644
--- a/bolt/unittests/Core/CMakeLists.txt
+++ b/bolt/unittests/Core/CMakeLists.txt
@@ -24,7 +24,6 @@ target_link_libraries(CoreTests
PRIVATE
LLVMBOLTCore
LLVMBOLTRewrite
- LLVMBOLTPasses
LLVMBOLTProfile
LLVMBOLTUtils
)
diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp
index 5ea242d65e533..e67460fe2a6a6 100644
--- a/bolt/unittests/Core/MCPlusBuilder.cpp
+++ b/bolt/unittests/Core/MCPlusBuilder.cpp
@@ -18,9 +18,6 @@
#include "bolt/Core/BinaryBasicBlock.h"
#include "bolt/Core/BinaryFunction.h"
-#include "bolt/Core/BinaryFunctionCallGraph.h"
-#include "bolt/Passes/BinaryPasses.h"
-#include "bolt/Passes/DataflowInfoManager.h"
#include "bolt/Rewrite/RewriteInstance.h"
#include "llvm/BinaryFormat/ELF.h"
#include "llvm/DebugInfo/DWARF/DWARFContext.h"
@@ -236,8 +233,8 @@ TEST_P(MCPlusBuilderTester, AArch64_ReverseCompAndBranch) {
.addExpr(MCSymbolRefExpr::create(
TargetBB->getLabel(), *BC->Ctx.get()));
ASSERT_TRUE(BC->MIB->isReversibleBranch(NeedsImmInc));
- BC->MIB->reverseBranchCondition(/*ParentBB*/ nullptr, NeedsImmInc,
- TargetBB->getLabel(), BC->Ctx.get());
+ BC->MIB->reverseBranchCondition(NeedsImmInc, TargetBB->getLabel(),
+ BC->Ctx.get());
ASSERT_EQ(NeedsImmInc.getOpcode(), AArch64::CBLTXri);
ASSERT_EQ(NeedsImmInc.getOperand(1).getImm(), 1);
@@ -250,8 +247,8 @@ TEST_P(MCPlusBuilderTester, AArch64_ReverseCompAndBranch) {
.addExpr(MCSymbolRefExpr::create(
TargetBB->getLabel(), *BC->Ctx.get()));
ASSERT_TRUE(BC->MIB->isReversibleBranch(NeedsImmDec));
- BC->MIB->reverseBranchCondition(/*ParentBB*/ nullptr, NeedsImmDec,
- TargetBB->getLabel(), BC->Ctx.get());
+ BC->MIB->reverseBranchCondition(NeedsImmDec, TargetBB->getLabel(),
+ BC->Ctx.get());
ASSERT_EQ(NeedsImmDec.getOpcode(), AArch64::CBHIXri);
ASSERT_EQ(NeedsImmDec.getOperand(1).getImm(), 0);
@@ -264,8 +261,8 @@ TEST_P(MCPlusBuilderTester, AArch64_ReverseCompAndBranch) {
.addExpr(MCSymbolRefExpr::create(
TargetBB->getLabel(), *BC->Ctx.get()));
ASSERT_TRUE(BC->MIB->isReversibleBranch(CompRegNeedsRegSwap));
- BC->MIB->reverseBranchCondition(/*ParentBB*/ nullptr, CompRegNeedsRegSwap,
- TargetBB->getLabel(), BC->Ctx.get());
+ BC->MIB->reverseBranchCondition(CompRegNeedsRegSwap, TargetBB->getLabel(),
+ BC->Ctx.get());
ASSERT_EQ(CompRegNeedsRegSwap.getOpcode(), AArch64::CBGTXrr);
ASSERT_EQ(CompRegNeedsRegSwap.getOperand(0).getReg(), AArch64::X1);
ASSERT_EQ(CompRegNeedsRegSwap.getOperand(1).getReg(), AArch64::X0);
@@ -279,8 +276,8 @@ TEST_P(MCPlusBuilderTester, AArch64_ReverseCompAndBranch) {
.addExpr(MCSymbolRefExpr::create(
TargetBB->getLabel(), *BC->Ctx.get()));
ASSERT_TRUE(BC->MIB->isReversibleBranch(CompByteNeedsRegSwap));
- BC->MIB->reverseBranchCondition(/*ParentBB*/ nullptr, CompByteNeedsRegSwap,
- TargetBB->getLabel(), BC->Ctx.get());
+ BC->MIB->reverseBranchCondition(CompByteNeedsRegSwap, TargetBB->getLabel(),
+ BC->Ctx.get());
ASSERT_EQ(CompByteNeedsRegSwap.getOpcode(), AArch64::CBBHSWrr);
ASSERT_EQ(CompByteNeedsRegSwap.getOperand(0).getReg(), AArch64::W1);
ASSERT_EQ(CompByteNeedsRegSwap.getOperand(1).getReg(), AArch64::W0);
@@ -294,8 +291,8 @@ TEST_P(MCPlusBuilderTester, AArch64_ReverseCompAndBranch) {
.addExpr(MCSymbolRefExpr::create(
TargetBB->getLabel(), *BC->Ctx.get()));
ASSERT_TRUE(BC->MIB->isReversibleBranch(CompHalfNeedsRegSwap));
- BC->MIB->reverseBranchCondition(/*ParentBB*/ nullptr, CompHalfNeedsRegSwap,
- TargetBB->getLabel(), BC->Ctx.get());
+ BC->MIB->reverseBranchCondition(CompHalfNeedsRegSwap, TargetBB->getLabel(),
+ BC->Ctx.get());
ASSERT_EQ(CompHalfNeedsRegSwap.getOpcode(), AArch64::CBHHIWrr);
ASSERT_EQ(CompHalfNeedsRegSwap.getOperand(0).getReg(), AArch64::W1);
ASSERT_EQ(CompHalfNeedsRegSwap.getOperand(1).getReg(), AArch64::W0);
@@ -321,123 +318,6 @@ TEST_P(MCPlusBuilderTester, AArch64_ReverseCompAndBranch) {
ASSERT_FALSE(BC->MIB->isReversibleBranch(Overflows));
}
-TEST_P(MCPlusBuilderTester, AArch64_ReverseCompAndBranch_Underflows) {
- if (GetParam() != Triple::aarch64)
- GTEST_SKIP();
-
- BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true);
- BinaryBasicBlock *EntryBB = BF->addBasicBlock();
- BinaryBasicBlock *FallThroughBB = BF->addBasicBlock();
- BinaryBasicBlock *TargetBB = BF->addBasicBlock();
- BF->addEntryPoint(*EntryBB);
- EntryBB->addSuccessor(TargetBB);
- EntryBB->addSuccessor(FallThroughBB);
-
- // Inversion requires expansion, immediate value underflows.
- // cblt x0, #0, target ~> cmp x0, #0
- // b.ge target
- auto I =
- EntryBB->addInstruction(MCInstBuilder(AArch64::CBLTXri)
- .addReg(AArch64::X0)
- .addImm(0)
- .addExpr(MCSymbolRefExpr::create(
- TargetBB->getLabel(), *BC->Ctx.get())));
- BinaryFunctionCallGraph CG(buildCallGraph(*BC.get()));
- RegAnalysis RA(*BC.get(), &BC->getBinaryFunctions(), &CG);
- DataflowInfoManager DIM(*BF, &RA, nullptr);
-
- ASSERT_TRUE(BC->MIB->isReversibleBranch(*I, &DIM));
- BC->MIB->reverseBranchCondition(EntryBB, *I, TargetBB->getLabel(),
- BC->Ctx.get(), &DIM);
- I = EntryBB->begin();
- ASSERT_EQ(I->getOpcode(), AArch64::SUBSXri);
- ASSERT_EQ(I->getOperand(0).getReg(), AArch64::XZR);
- ASSERT_EQ(I->getOperand(1).getReg(), AArch64::X0);
- ASSERT_EQ(I->getOperand(2).getImm(), 0);
- ASSERT_EQ(I->getOperand(3).getImm(), 0);
- I++;
- ASSERT_EQ(I->getOpcode(), AArch64::Bcc);
- ASSERT_EQ(I->getOperand(0).getImm(), AArch64CC::GE);
-}
-
-TEST_P(MCPlusBuilderTester, AArch64_ReverseCompAndBranch_Overflows) {
- if (GetParam() != Triple::aarch64)
- GTEST_SKIP();
-
- BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true);
- BinaryBasicBlock *EntryBB = BF->addBasicBlock();
- BinaryBasicBlock *FallThroughBB = BF->addBasicBlock();
- BinaryBasicBlock *TargetBB = BF->addBasicBlock();
- BF->addEntryPoint(*EntryBB);
- EntryBB->addSuccessor(TargetBB);
- EntryBB->addSuccessor(FallThroughBB);
-
- // Inversion requires expansion, immediate value overflows.
- // cbhi w0, #63, target ~> cmp w0, #63
- // b.ls target
- auto I =
- EntryBB->addInstruction(MCInstBuilder(AArch64::CBHIWri)
- .addReg(AArch64::W0)
- .addImm(63)
- .addExpr(MCSymbolRefExpr::create(
- TargetBB->getLabel(), *BC->Ctx.get())));
- BinaryFunctionCallGraph CG(buildCallGraph(*BC.get()));
- RegAnalysis RA(*BC.get(), &BC->getBinaryFunctions(), &CG);
- DataflowInfoManager DIM(*BF, &RA, nullptr);
-
- ASSERT_TRUE(BC->MIB->isReversibleBranch(*I, &DIM));
- BC->MIB->reverseBranchCondition(EntryBB, *I, TargetBB->getLabel(),
- BC->Ctx.get(), &DIM);
- I = EntryBB->begin();
- ASSERT_EQ(I->getOpcode(), AArch64::SUBSWri);
- ASSERT_EQ(I->getOperand(0).getReg(), AArch64::WZR);
- ASSERT_EQ(I->getOperand(1).getReg(), AArch64::W0);
- ASSERT_EQ(I->getOperand(2).getImm(), 63);
- ASSERT_EQ(I->getOperand(3).getImm(), 0);
- I++;
- ASSERT_EQ(I->getOpcode(), AArch64::Bcc);
- ASSERT_EQ(I->getOperand(0).getImm(), AArch64CC::LS);
-}
-
-TEST_P(MCPlusBuilderTester, AArch64_IsReversibleBranch_LiveCondFlags) {
- if (GetParam() != Triple::aarch64)
- GTEST_SKIP();
-
- BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true);
- BinaryBasicBlock *EntryBB = BF->addBasicBlock();
- BinaryBasicBlock *FallThroughBB = BF->addBasicBlock();
- BinaryBasicBlock *TargetBB = BF->addBasicBlock();
- BF->addEntryPoint(*EntryBB);
- EntryBB->addSuccessor(TargetBB);
- EntryBB->addSuccessor(FallThroughBB);
-
- // cmp x0, #63
- EntryBB->addInstruction(MCInstBuilder(AArch64::SUBSXri)
- .addReg(AArch64::XZR)
- .addReg(AArch64::X0)
- .addImm(63)
- .addImm(0));
- // cbgt x0, #63, target
- auto I =
- EntryBB->addInstruction(MCInstBuilder(AArch64::CBGTXri)
- .addReg(AArch64::X0)
- .addImm(63)
- .addExpr(MCSymbolRefExpr::create(
- TargetBB->getLabel(), *BC->Ctx.get())));
- // csel x0, x1, x2, le
- FallThroughBB->addInstruction(MCInstBuilder(AArch64::CSELXr)
- .addReg(AArch64::X0)
- .addReg(AArch64::X1)
- .addReg(AArch64::X2)
- .addImm(13));
-
- BinaryFunctionCallGraph CG(buildCallGraph(*BC.get()));
- RegAnalysis RA(*BC.get(), &BC->getBinaryFunctions(), &CG);
- DataflowInfoManager DIM(*BF, &RA, nullptr);
-
- ASSERT_FALSE(BC->MIB->isReversibleBranch(*I, &DIM));
-}
-
TEST_P(MCPlusBuilderTester, AArch64_CmpJE) {
if (GetParam() != Triple::aarch64)
GTEST_SKIP();
More information about the llvm-commits
mailing list