From 44e4514465c3c571c3b29667a6f6afa7ead9c493 Mon Sep 17 00:00:00 2001 From: Maksim Panchenko Date: Fri, 13 Sep 2024 20:31:38 -0700 Subject: [PATCH 1/3] [BOLT][AArch64] Reduce the number of ADR relaxations If ADR instruction references the same function, we can skip relaxation even if the function is split, but ADR is in the main fragment. --- bolt/lib/Passes/ADRRelaxationPass.cpp | 7 +++--- bolt/test/AArch64/adr-relaxation.s | 31 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 bolt/test/AArch64/adr-relaxation.s diff --git a/bolt/lib/Passes/ADRRelaxationPass.cpp b/bolt/lib/Passes/ADRRelaxationPass.cpp index 256034a841c70..52811edcb8273 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 0000000000000..55b7d80edaab4 --- /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 From f3174034e9d6f057e3f56c0bc934bc3a5f4f4ff0 Mon Sep 17 00:00:00 2001 From: Maksim Panchenko Date: Tue, 8 Oct 2024 13:18:11 -0700 Subject: [PATCH 2/3] fixup! [BOLT][AArch64] Reduce the number of ADR relaxations --- bolt/test/AArch64/adr-relaxation.s | 38 ++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/bolt/test/AArch64/adr-relaxation.s b/bolt/test/AArch64/adr-relaxation.s index 55b7d80edaab4..0206c30a85643 100644 --- a/bolt/test/AArch64/adr-relaxation.s +++ b/bolt/test/AArch64/adr-relaxation.s @@ -1,31 +1,49 @@ -## 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. +## Check that llvm-bolt will not unnecessarily relax ADR instruction. # 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-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 --disassemble-symbols=foo.cold.0 %t.bolt \ +# RUN: | FileCheck --check-prefix=CHECK-FOO %s -# CHECK-NOT: adrp - +## ADR below references its containing function that is split. But ADR is always +## in the main fragment, thus there is no need to relax it. .text .globl _start .type _start, %function _start: +# CHECK: <_start>: .cfi_startproc adr x1, _start +# CHECK-NOT: adrp +# CHECK: adr cmp x1, x11 b.hi .L1 - mov x0, #0x0 - .L1: ret x30 - .cfi_endproc .size _start, .-_start + +## In foo, ADR is in the split fragment but references the main one. Thus, it +## needs to be relaxed into ADRP + ADD. + .globl foo + .type foo, %function +foo: + .cfi_startproc + cmp x1, x11 + b.hi .L2 + mov x0, #0x0 +.L2: +# CHECK-FOO: : + adr x1, foo +# CHECK-FOO: adrp +# CHECK-FOO-NEXT: add + ret x30 + .cfi_endproc +.size foo, .-foo + ## Force relocation mode. .reloc 0, R_AARCH64_NONE From 759f0d6e18eeede4e84524c3480d2599e541da7d Mon Sep 17 00:00:00 2001 From: Maksim Panchenko Date: Tue, 8 Oct 2024 13:24:38 -0700 Subject: [PATCH 3/3] fixup! fixup! [BOLT][AArch64] Reduce the number of ADR relaxations --- bolt/test/AArch64/adr-relaxation.s | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/bolt/test/AArch64/adr-relaxation.s b/bolt/test/AArch64/adr-relaxation.s index 0206c30a85643..0aa3c71f29aaa 100644 --- a/bolt/test/AArch64/adr-relaxation.s +++ b/bolt/test/AArch64/adr-relaxation.s @@ -9,21 +9,21 @@ ## ADR below references its containing function that is split. But ADR is always ## in the main fragment, thus there is no need to relax it. - .text + .text .globl _start .type _start, %function _start: # CHECK: <_start>: - .cfi_startproc + .cfi_startproc adr x1, _start # CHECK-NOT: adrp # CHECK: adr - cmp x1, x11 - b.hi .L1 - mov x0, #0x0 + cmp x1, x11 + b.hi .L1 + mov x0, #0x0 .L1: - ret x30 - .cfi_endproc + ret x30 + .cfi_endproc .size _start, .-_start @@ -32,17 +32,17 @@ _start: .globl foo .type foo, %function foo: - .cfi_startproc - cmp x1, x11 - b.hi .L2 - mov x0, #0x0 + .cfi_startproc + cmp x1, x11 + b.hi .L2 + mov x0, #0x0 .L2: # CHECK-FOO: : adr x1, foo # CHECK-FOO: adrp # CHECK-FOO-NEXT: add - ret x30 - .cfi_endproc + ret x30 + .cfi_endproc .size foo, .-foo ## Force relocation mode.