Skip to content

Commit 4cd16f2

Browse files
authored
[BOLT][AArch64] Add more heuristics on epilogue determination (#167077)
Add more heuristics to check if a basic block is an AArch64 epilogue. We assume instructions that load from stack or adjust stack pointer as valid epilogue code sequence if and only if they immediately precede the branch instruction that ends the basic block.
1 parent e95f6fa commit 4cd16f2

File tree

5 files changed

+107
-9
lines changed

5 files changed

+107
-9
lines changed

bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,11 @@ class MCPlusBuilder {
784784

785785
virtual bool isPop(const MCInst &Inst) const { return false; }
786786

787+
/// Determine if a basic block looks like an epilogue. For now it is only
788+
/// called at the final stage of building CFG to check basic block ending
789+
/// with an indirect call that has unknown control flow attribute.
790+
virtual bool isEpilogue(const BinaryBasicBlock &BB) const { return false; }
791+
787792
/// Return true if the instruction is used to terminate an indirect branch.
788793
virtual bool isTerminateBranch(const MCInst &Inst) const {
789794
llvm_unreachable("not implemented");

bolt/lib/Core/BinaryFunction.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2167,13 +2167,10 @@ bool BinaryFunction::postProcessIndirectBranches(
21672167
continue;
21682168
}
21692169

2170-
// If this block contains an epilogue code and has an indirect branch,
2171-
// then most likely it's a tail call. Otherwise, we cannot tell for sure
2172-
// what it is and conservatively reject the function's CFG.
2173-
bool IsEpilogue = llvm::any_of(BB, [&](const MCInst &Instr) {
2174-
return BC.MIB->isLeave(Instr) || BC.MIB->isPop(Instr);
2175-
});
2176-
if (IsEpilogue) {
2170+
// If this block contains epilogue code and has an indirect branch,
2171+
// then most likely it's a tail call. Otherwise, we cannot tell for
2172+
// sure what it is and conservatively reject the function's CFG.
2173+
if (BC.MIB->isEpilogue(BB)) {
21772174
BC.MIB->convertJmpToTailCall(Instr);
21782175
BB.removeAllSuccessors();
21792176
continue;

bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,11 +164,53 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
164164

165165
bool isPush(const MCInst &Inst) const override {
166166
return isStoreToStack(Inst);
167-
};
167+
}
168168

169169
bool isPop(const MCInst &Inst) const override {
170170
return isLoadFromStack(Inst);
171-
};
171+
}
172+
173+
// We look for instructions that load from stack or make stack pointer
174+
// adjustment, and assume the basic block is an epilogue if and only if
175+
// such instructions are present and also immediately precede the branch
176+
// instruction that ends the basic block.
177+
bool isEpilogue(const BinaryBasicBlock &BB) const override {
178+
if (BB.succ_size())
179+
return false;
180+
181+
bool SeenLoadFromStack = false;
182+
bool SeenStackPointerAdjustment = false;
183+
for (const MCInst &Instr : BB) {
184+
// Skip CFI pseudo instruction.
185+
if (isCFI(Instr))
186+
continue;
187+
188+
bool IsPop = isPop(Instr);
189+
// A load from stack instruction could do SP adjustment in pre-index or
190+
// post-index form, which we can skip to check for epilogue recognition
191+
// purpose.
192+
bool IsSPAdj = (isADD(Instr) || isMOVW(Instr)) &&
193+
Instr.getOperand(0).isReg() &&
194+
Instr.getOperand(0).getReg() == AArch64::SP;
195+
SeenLoadFromStack |= IsPop;
196+
SeenStackPointerAdjustment |= IsSPAdj;
197+
198+
if (!SeenLoadFromStack && !SeenStackPointerAdjustment)
199+
continue;
200+
if (IsPop || IsSPAdj || isPAuthOnLR(Instr))
201+
continue;
202+
if (isReturn(Instr))
203+
return true;
204+
if (isBranch(Instr))
205+
break;
206+
207+
// Any previously seen load from stack or stack adjustment instruction
208+
// is definitely not part of epilogue code sequence, so reset these two.
209+
SeenLoadFromStack = false;
210+
SeenStackPointerAdjustment = false;
211+
}
212+
return SeenLoadFromStack || SeenStackPointerAdjustment;
213+
}
172214

173215
void createCall(MCInst &Inst, const MCSymbol *Target,
174216
MCContext *Ctx) override {

bolt/lib/Target/X86/X86MCPlusBuilder.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,12 @@ class X86MCPlusBuilder : public MCPlusBuilder {
219219
return getPopSize(Inst) == 0 ? false : true;
220220
}
221221

222+
bool isEpilogue(const BinaryBasicBlock &BB) const override {
223+
return ::llvm::any_of(BB, [&](const MCInst &Instr) {
224+
return isLeave(Instr) || isPop(Instr);
225+
});
226+
}
227+
222228
bool isTerminateBranch(const MCInst &Inst) const override {
223229
return Inst.getOpcode() == X86::ENDBR32 || Inst.getOpcode() == X86::ENDBR64;
224230
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Test that we will not incorrectly take the first basic block in function
2+
# `_foo` as epilogue due to the first load from stack instruction.
3+
4+
# RUN: %clang %cflags %s -o %t.so -Wl,-q
5+
# RUN: llvm-bolt %t.so -o %t.bolt --print-cfg | FileCheck %s
6+
7+
.text
8+
.global _foo
9+
.type _foo, %function
10+
_foo:
11+
ldr w8, [sp]
12+
adr x10, _jmptbl
13+
ldrsw x9, [x10, x9, lsl #2]
14+
add x10, x10, x9
15+
br x10
16+
# CHECK-NOT: x10 # TAILCALL
17+
# CHECK: x10 # UNKNOWN CONTROL FLOW
18+
mov x0, 0
19+
ret
20+
mov x0, 1
21+
ret
22+
23+
.balign 4
24+
_jmptbl:
25+
.long -16
26+
.long -8
27+
28+
.global _bar
29+
.type _bar, %function
30+
_bar:
31+
stp x29, x30, [sp, #-0x10]!
32+
mov x29, sp
33+
sub sp, sp, #0x10
34+
ldr x8, [x29, #0x30]
35+
blr x8
36+
add sp, sp, #0x10
37+
ldp x29, x30, [sp], #0x10
38+
br x2
39+
# CHECK-NOT: x2 # UNKNOWN CONTROL FLOW
40+
# CHECK: x2 # TAILCALL
41+
42+
.global _start
43+
.type _start, %function
44+
_start:
45+
ret
46+
47+
# Dummy relocation to force relocation mode
48+
.reloc 0, R_AARCH64_NONE

0 commit comments

Comments
 (0)