[llvm] [BOLT][AArch64] Fix error message for failed ADR relaxation (PR #142533)
Maksim Panchenko via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 2 22:57:17 PDT 2025
https://github.com/maksfb created https://github.com/llvm/llvm-project/pull/142533
Do not recommend the strict mode to the user when ADR relaxation fails on a non-simple function, i.e. a function with unknown CFG.
We cannot rely on relocations to reconstruct compiler-generated jump tables for AArch64, hence strict mode does not work as intended.
>From bbcae272287fa00f21bd27fa4032694b40a32997 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Mon, 2 Jun 2025 22:08:38 -0700
Subject: [PATCH] [BOLT][AArch64] Fix error message for failed ADR relaxation
Do not recommend the strict mode to the user when ADR relaxation fails
on a non-simple function, i.e. a function with unknown CFG.
We cannot rely on relocations to reconstruct compiler-generated jump
tables for AArch64, hence strict mode does not work as intended.
---
bolt/lib/Passes/ADRRelaxationPass.cpp | 8 ++----
bolt/test/AArch64/adr-relaxation-fail.s | 35 ++++++++++++++++++++++++
bolt/test/AArch64/r_aarch64_prelxx.s | 9 +++---
bolt/test/AArch64/test-indirect-branch.s | 11 ++++++--
4 files changed, 52 insertions(+), 11 deletions(-)
create mode 100644 bolt/test/AArch64/adr-relaxation-fail.s
diff --git a/bolt/lib/Passes/ADRRelaxationPass.cpp b/bolt/lib/Passes/ADRRelaxationPass.cpp
index 4b37a061ac12d..c3954c94a7f92 100644
--- a/bolt/lib/Passes/ADRRelaxationPass.cpp
+++ b/bolt/lib/Passes/ADRRelaxationPass.cpp
@@ -81,17 +81,15 @@ void ADRRelaxationPass::runOnFunction(BinaryFunction &BF) {
It = BB.eraseInstruction(std::prev(It));
} else if (std::next(It) != BB.end() && BC.MIB->isNoop(*std::next(It))) {
BB.eraseInstruction(std::next(It));
- } else if (!opts::StrictMode && !BF.isSimple()) {
+ } else if (!BF.isSimple()) {
// If the function is not simple, it may contain a jump table undetected
// by us. This jump table may use an offset from the branch instruction
// to land in the desired place. If we add new instructions, we
// invalidate this offset, so we have to rely on linker-inserted NOP to
// replace it with ADRP, and abort if it is not present.
auto L = BC.scopeLock();
- BC.errs() << formatv(
- "BOLT-ERROR: Cannot relax adr in non-simple function "
- "{0}. Use --strict option to override\n",
- BF.getOneName());
+ BC.errs() << "BOLT-ERROR: cannot relax ADR in non-simple function "
+ << BF << '\n';
PassFailed = true;
return;
}
diff --git a/bolt/test/AArch64/adr-relaxation-fail.s b/bolt/test/AArch64/adr-relaxation-fail.s
new file mode 100644
index 0000000000000..320d662be4d58
--- /dev/null
+++ b/bolt/test/AArch64/adr-relaxation-fail.s
@@ -0,0 +1,35 @@
+## Check that llvm-bolt generates a proper error message when ADR instruction
+## cannot be relaxed.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
+# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -static
+# RUN: not llvm-bolt %t.exe -o %t.bolt 2>&1 | FileCheck %s
+
+# CHECK: BOLT-ERROR: cannot relax ADR in non-simple function _start
+
+## The function contains unknown control flow and llvm-bolt fails to recover
+## CFG. As BOLT has to preserve the function layout, the ADR instruction cannot
+## be relaxed into ADRP+ADD.
+ .text
+ .globl _start
+ .type _start, %function
+_start:
+ .cfi_startproc
+ adr x1, foo
+ br x0
+ ret x0
+ .cfi_endproc
+ .size _start, .-_start
+
+ .globl foo
+ .type foo, %function
+foo:
+ .cfi_startproc
+ cmp x1, x11
+ b.hi .L2
+ mov x0, #0x0
+.L2:
+ adr x1, foo
+ ret x30
+ .cfi_endproc
+ .size foo, .-foo
diff --git a/bolt/test/AArch64/r_aarch64_prelxx.s b/bolt/test/AArch64/r_aarch64_prelxx.s
index 73bf8387d3634..708b66cb5ffd0 100644
--- a/bolt/test/AArch64/r_aarch64_prelxx.s
+++ b/bolt/test/AArch64/r_aarch64_prelxx.s
@@ -12,7 +12,7 @@
// CHECKPREL-NEXT: R_AARCH64_PREL32 {{.*}} _start + 4
// CHECKPREL-NEXT: R_AARCH64_PREL64 {{.*}} _start + 8
-// RUN: llvm-bolt %t.exe -o %t.bolt --strict
+// RUN: llvm-bolt %t.exe -o %t.bolt
// RUN: llvm-objdump -D %t.bolt | FileCheck %s --check-prefix=CHECKPREL32
// CHECKPREL32: [[#%x,DATATABLEADDR:]] <datatable>:
@@ -21,7 +21,7 @@
// 4 is offset in datatable
// 8 is addend
-// CHECKPREL32: [[#DATATABLEADDR + 4 - 8 + VALUE]] <_start>:
+// CHECKPREL32: [[#DATATABLEADDR + VALUE]] <_start>:
// RUN: llvm-objdump -D %t.bolt | FileCheck %s --check-prefix=CHECKPREL64
// CHECKPREL64: [[#%x,DATATABLEADDR:]] <datatable>:
@@ -32,14 +32,15 @@
// 8 is offset in datatable
// 12 is addend
-// CHECKPREL64: [[#DATATABLEADDR + 8 - 12 + VALUE]] <_start>:
+// CHECKPREL64: [[#DATATABLEADDR + VALUE]] <_start>:
.section .text
.align 4
.globl _start
.type _start, %function
_start:
- adr x0, datatable
+ adrp x0, datatable
+ add x0, x0, :lo12:datable
mov x0, #0
ret
diff --git a/bolt/test/AArch64/test-indirect-branch.s b/bolt/test/AArch64/test-indirect-branch.s
index b99737ee97acc..a9fa7737d3d2d 100644
--- a/bolt/test/AArch64/test-indirect-branch.s
+++ b/bolt/test/AArch64/test-indirect-branch.s
@@ -7,8 +7,8 @@
// RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown -mattr=+pauth %s -o %t.o
// RUN: %clang %cflags --target=aarch64-unknown-linux %t.o -o %t.exe -Wl,-q
-// RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg --strict --debug-only=mcplus \
-// RUN: -v=1 2>&1 | FileCheck %s
+// RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg --debug-only=mcplus -v=1 2>&1 \
+// RUN: | FileCheck %s
// Pattern 1: there is no shift amount after the 'add' instruction.
//
@@ -45,6 +45,7 @@ _start:
.type test1, %function
test1:
mov x1, #0
+ nop
adr x3, datatable
add x3, x3, x1, lsl #2
ldr w2, [x3]
@@ -56,6 +57,11 @@ test1_1:
ret
test1_2:
ret
+ .size test1, .-test1
+
+// Temporary workaround for PC-relative relocations from datatable leaking into
+// test2 function and creating phantom entry points.
+.skip 0x100
// Pattern 2
// CHECK: BOLT-DEBUG: failed to match indirect branch: nop/adr instead of adrp/add
@@ -65,6 +71,7 @@ test2:
nop
adr x3, jump_table
ldrh w3, [x3, x1, lsl #1]
+ nop
adr x1, test2_0
add x3, x1, w3, sxth #2
br x3
More information about the llvm-commits
mailing list