[BOLT][AArch64] Reduce the number of ADR relaxations

[maksfb]

If ADR instruction references the same function, we can skip relaxation even if the function is split, but ADR is in the main fragment.

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Maksim Panchenko (maksfb)

Changes

If ADR instruction references the same function, we can skip relaxation even if the function is split, but ADR is in the main fragment.

---

Full diff: https://github.com/llvm/llvm-project/pull/111577.diff

2 Files Affected:

• (modified) bolt/lib/Passes/ADRRelaxationPass.cpp (+4-3)
• (added) bolt/test/AArch64/adr-relaxation.s (+31)

diff --git a/bolt/lib/Passes/ADRRelaxationPass.cpp b/bolt/lib/Passes/ADRRelaxationPass.cpp
index 256034a841c706..52811edcb82731 100644
--- a/bolt/lib/Passes/ADRRelaxationPass.cpp
+++ b/bolt/lib/Passes/ADRRelaxationPass.cpp
@@ -56,13 +56,14 @@ void ADRRelaxationPass::runOnFunction(BinaryFunction &BF) {
           continue;
       }
 
-      // Don't relax adr if it points to the same function and it is not split
-      // and BF initial size is < 1MB.
+      // Don't relax ADR if it points to the same function and is in the main
+      // fragment and BF initial size is < 1MB.
       const unsigned OneMB = 0x100000;
       if (BF.getSize() < OneMB) {
         BinaryFunction *TargetBF = BC.getFunctionForSymbol(Symbol);
-        if (TargetBF == &BF && !BF.isSplit())
+        if (TargetBF == &BF && !BB.isSplit())
           continue;
+
         // No relaxation needed if ADR references a basic block in the same
         // fragment.
         if (BinaryBasicBlock *TargetBB = BF.getBasicBlockForLabel(Symbol))
diff --git a/bolt/test/AArch64/adr-relaxation.s b/bolt/test/AArch64/adr-relaxation.s
new file mode 100644
index 00000000000000..55b7d80edaab49
--- /dev/null
+++ b/bolt/test/AArch64/adr-relaxation.s
@@ -0,0 +1,31 @@
+## Check that llvm-bolt will unnecessarily relax ADR instruction.
+## ADR below references containing function that is split. But ADR is always
+## in the main fragment, thus there is no need to relax it.
+
+# 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=randomN \
+# RUN:   2>&1 | FileCheck %s
+# RUN: llvm-objdump -d --disassemble-symbols=_start %t.bolt | FileCheck %s
+
+# CHECK-NOT: adrp
+
+	.text
+  .globl _start
+  .type _start, %function
+_start:
+	.cfi_startproc
+  adr x1, _start
+	cmp	x1, x11
+	b.hi	.L1
+
+	mov	x0, #0x0
+
+.L1:
+	ret	x30
+
+	.cfi_endproc
+.size _start, .-_start
+
+## Force relocation mode.
+  .reloc 0, R_AARCH64_NONE

[dcci]

Thanks for working on this. Some comments/questions.

[dcci]

you mean "will not relax" ?

[dcci]

can we dump the whole function (or some context?) that way we can make the test more robust.

[dcci]

This looks fine to me. Can we add a negative test? (e.g. one where the func is not in the main fragment to show the adrp is still there?)

[yota9]

Thanks!

[yota9]

Indentation

[dcci]

LGTM