Cold-Air-Networks/mirror-llvm-project
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 7

Revert "[BOLT][AArch64] Transform cmpbr ~> cmp + br when inversion not possible (#185731)" (#202309)

Browse source 
This reverts commit 6b13656fd8.

We have identified various bugs hence reverting:

* relaxLocalBranches() should account for BB growth and adjust subsequent BB offsets in fragment
* multiple parallel workers are sharing the same allocator in DataflowInfoManager
* liveness is run lazily, potentially after the CFG has been modified
This commit is contained in:
Alexandros Lamprineas 2026-06-08 11:24:03 +01:00 committed by GitHub
parent dab2bdb974
commit c1c0fb32dd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 83 additions and 367 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
bolt/include/bolt/Core/BinaryFunction.h
View file

@ -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 @@ public:
/// 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; }

11
bolt/include/bolt/Core/MCPlusBuilder.h
View file

@ -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 @@ public:
}
/// 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 @@ public:
}
/// 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");
}

4
bolt/include/bolt/Passes/LongJmp.h
View file

@ -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

4
bolt/include/bolt/Utils/CommandLineOpts.h
View file

@ -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();

7
bolt/lib/Core/BinaryFunction.cpp
View file

@ -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;
};

16
bolt/lib/Passes/BinaryPasses.cpp
View file

@ -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

27
bolt/lib/Passes/LongJmp.cpp
View file

@ -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

90
bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
View file

@ -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 @@ public:
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 @@ public:
}
}
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())));

6
bolt/lib/Target/AArch64/CMakeLists.txt
View file

@ -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

6
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
View file

@ -162,10 +162,8 @@ public:
}
}
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);

6
bolt/lib/Target/X86/X86MCPlusBuilder.cpp
View file

@ -2806,10 +2806,8 @@ public:
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);

6
bolt/lib/Utils/CommandLineOpts.cpp
View file

@ -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;

122
bolt/test/AArch64/compare-and-branch-inversion.S
View file

@ -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
.globl immediate_overflow
.type immediate_overflow, %function
immediate_overflow:
.entry3:
# FDATA: 1 immediate_overflow #.entry3# 10
cbgt x0, #63, .exit3
.cold3:
# FDATA: 1 immediate_overflow #.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
# 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 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
.entry3:
# FDATA: 1 irreversible #.entry3# 10
cbgt x0, #63, .exit3
.cold3:
# FDATA: 1 irreversible #.cold3# 1
mov x0, #1
ret
.exit4:
# FDATA: 1 irreversible #.exit4# 10
.exit3:
# 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

1
bolt/unittests/Core/CMakeLists.txt
View file

@ -24,7 +24,6 @@ target_link_libraries(CoreTests
PRIVATE
LLVMBOLTCore
LLVMBOLTRewrite
LLVMBOLTPasses
LLVMBOLTProfile
LLVMBOLTUtils
)

140
bolt/unittests/Core/MCPlusBuilder.cpp
View file

@ -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();