[llvm] [BOLT][AArch64] Fix error message for failed ADR relaxation (PR #142533)
Maksim Panchenko via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 3 01:28:42 PDT 2025
https://github.com/maksfb updated https://github.com/llvm/llvm-project/pull/142533
>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 1/4] [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
>From b9600c19b1a3f560ae7a9d543bf759ff62fc9214 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Mon, 2 Jun 2025 23:11:12 -0700
Subject: [PATCH 2/4] fixup! [BOLT][AArch64] Fix error message for failed ADR
relaxation
---
bolt/test/AArch64/r_aarch64_prelxx.s | 4 ----
1 file changed, 4 deletions(-)
diff --git a/bolt/test/AArch64/r_aarch64_prelxx.s b/bolt/test/AArch64/r_aarch64_prelxx.s
index 708b66cb5ffd0..5cbe2c50b2946 100644
--- a/bolt/test/AArch64/r_aarch64_prelxx.s
+++ b/bolt/test/AArch64/r_aarch64_prelxx.s
@@ -19,8 +19,6 @@
// CHECKPREL32-NEXT: 00:
// CHECKPREL32-NEXT: 04: {{.*}} .word 0x[[#%x,VALUE:]]
-// 4 is offset in datatable
-// 8 is addend
// CHECKPREL32: [[#DATATABLEADDR + VALUE]] <_start>:
// RUN: llvm-objdump -D %t.bolt | FileCheck %s --check-prefix=CHECKPREL64
@@ -30,8 +28,6 @@
// CHECKPREL64-NEXT: 08: {{.*}} .word 0x[[#%x,VALUE:]]
// CHECKPREL64-NEXT: 0c: {{.*}} .word 0x00000000
-// 8 is offset in datatable
-// 12 is addend
// CHECKPREL64: [[#DATATABLEADDR + VALUE]] <_start>:
.section .text
>From 3692f52cb7e8583853e7977aac6d0f5ca219cab5 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Tue, 3 Jun 2025 01:26:58 -0700
Subject: [PATCH 3/4] fixup! fixup! [BOLT][AArch64] Fix error message for
failed ADR relaxation
---
bolt/test/AArch64/adr-relaxation-fail.s | 3 +
bolt/test/AArch64/adr-relaxation.s | 30 ++++++---
bolt/test/runtime/AArch64/adrrelaxationpass.s | 62 -------------------
3 files changed, 25 insertions(+), 70 deletions(-)
delete mode 100644 bolt/test/runtime/AArch64/adrrelaxationpass.s
diff --git a/bolt/test/AArch64/adr-relaxation-fail.s b/bolt/test/AArch64/adr-relaxation-fail.s
index 320d662be4d58..5b2af96786311 100644
--- a/bolt/test/AArch64/adr-relaxation-fail.s
+++ b/bolt/test/AArch64/adr-relaxation-fail.s
@@ -4,6 +4,7 @@
# 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
+# RUN: not llvm-bolt %t.exe -o %t.bolt --strict 2>&1 | FileCheck %s
# CHECK: BOLT-ERROR: cannot relax ADR in non-simple function _start
@@ -16,6 +17,8 @@
_start:
.cfi_startproc
adr x1, foo
+ adr x2, .L1
+.L1:
br x0
ret x0
.cfi_endproc
diff --git a/bolt/test/AArch64/adr-relaxation.s b/bolt/test/AArch64/adr-relaxation.s
index 0aa3c71f29aaa..a643a62339ba3 100644
--- a/bolt/test/AArch64/adr-relaxation.s
+++ b/bolt/test/AArch64/adr-relaxation.s
@@ -3,7 +3,7 @@
# 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: llvm-bolt %t.exe -o %t.bolt --split-functions --split-strategy=random2
-# RUN: llvm-objdump -d --disassemble-symbols=_start %t.bolt | FileCheck %s
+# RUN: llvm-objdump -d -j .text %t.bolt | FileCheck %s
# RUN: llvm-objdump -d --disassemble-symbols=foo.cold.0 %t.bolt \
# RUN: | FileCheck --check-prefix=CHECK-FOO %s
@@ -13,11 +13,10 @@
.globl _start
.type _start, %function
_start:
-# CHECK: <_start>:
.cfi_startproc
+# CHECK: <_start>:
+# CHECK-NEXT: adr
adr x1, _start
-# CHECK-NOT: adrp
-# CHECK: adr
cmp x1, x11
b.hi .L1
mov x0, #0x0
@@ -38,12 +37,27 @@ foo:
mov x0, #0x0
.L2:
# CHECK-FOO: <foo.cold.0>:
- adr x1, foo
-# CHECK-FOO: adrp
+# CHECK-FOO-NEXT: adrp
# CHECK-FOO-NEXT: add
+ adr x1, foo
ret x30
.cfi_endproc
.size foo, .-foo
-## Force relocation mode.
- .reloc 0, R_AARCH64_NONE
+## bar is a non-simple function. We can still relax ADR, because it has a
+## preceding NOP.
+ .globl bar
+ .type bar, %function
+bar:
+ .cfi_startproc
+# CHECK-LABEL: <bar>:
+# CHECK-NEXT: adrp
+# CHECK-NEXT: add
+ nop
+ adr x0, foo
+ adr x1, .L3
+ br x1
+.L3:
+ ret x0
+ .cfi_endproc
+ .size bar, .-bar
diff --git a/bolt/test/runtime/AArch64/adrrelaxationpass.s b/bolt/test/runtime/AArch64/adrrelaxationpass.s
deleted file mode 100644
index fa9fb63c613dc..0000000000000
--- a/bolt/test/runtime/AArch64/adrrelaxationpass.s
+++ /dev/null
@@ -1,62 +0,0 @@
-# The second and third ADR instructions are non-local to functions
-# and must be replaced with ADRP + ADD by BOLT
-# Also since main and test are non-simple, we can't change it's length so we
-# have to replace NOP with adrp, and if there is no nop before adr in non-simple
-# function, we can't guarantee we didn't break possible jump tables, so we
-# fail in non-strict mode
-
-# REQUIRES: system-linux
-
-# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown \
-# RUN: %s -o %t.o
-# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
-# RUN: llvm-bolt %t.exe -o %t.bolt --adr-relaxation=true --strict
-# RUN: llvm-objdump --no-print-imm-hex -d --disassemble-symbols=main %t.bolt | FileCheck %s
-# RUN: %t.bolt
-# RUN: not llvm-bolt %t.exe -o %t.bolt --adr-relaxation=true \
-# RUN: 2>&1 | FileCheck %s --check-prefix CHECK-ERROR
-
- .text
- .align 4
- .global test
- .type test, %function
-test:
- adr x2, Gvar
- mov x0, xzr
- ret
- .size test, .-test
-
- .align 4
- .global main
- .type main, %function
-main:
- adr x0, .CI
- nop
- adr x1, test
- adr x2, Gvar2
- adr x3, br
-br:
- br x1
- .size main, .-main
-.CI:
- .word 0xff
-
- .data
- .align 8
- .global Gvar
-Gvar: .xword 0x0
- .global Gvar2
-Gvar2: .xword 0x42
- .balign 4
-jmptable:
- .word 0
- .word test - jmptable
-
-# CHECK: <main>:
-# CHECK-NEXT: adr x0, 0x{{[1-8a-f][0-9a-f]*}}
-# CHECK-NEXT: adrp x1, 0x{{[1-8a-f][0-9a-f]*}}
-# CHECK-NEXT: add x1, x1, #{{[1-8a-f][0-9a-f]*}}
-# CHECK-NEXT: adrp x2, 0x{{[1-8a-f][0-9a-f]*}}
-# CHECK-NEXT: add x2, x2, #{{[1-8a-f][0-9a-f]*}}
-# CHECK-NEXT: adr x3, 0x{{[1-8a-f][0-9a-f]*}}
-# CHECK-ERROR: BOLT-ERROR: Cannot relax adr in non-simple function
>From f954a9f87d74d6965b40a9f6fa8be93f2e558f0d Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Tue, 3 Jun 2025 01:28:29 -0700
Subject: [PATCH 4/4] fixup! fixup! fixup! [BOLT][AArch64] Fix error message
for failed ADR relaxation
---
bolt/test/AArch64/adr-relaxation-fail.s | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/bolt/test/AArch64/adr-relaxation-fail.s b/bolt/test/AArch64/adr-relaxation-fail.s
index 5b2af96786311..2b04bf8db697e 100644
--- a/bolt/test/AArch64/adr-relaxation-fail.s
+++ b/bolt/test/AArch64/adr-relaxation-fail.s
@@ -28,11 +28,6 @@ _start:
.type foo, %function
foo:
.cfi_startproc
- cmp x1, x11
- b.hi .L2
- mov x0, #0x0
-.L2:
- adr x1, foo
- ret x30
+ ret x0
.cfi_endproc
.size foo, .-foo
More information about the llvm-commits
mailing list