[bolt][aarch64] test to reproduce the issue with ldr reg, literal

[yavtuk]

ldr reg, literal instruction is limited +/- 1MB range, emitCI put the constants by the end of function and the one is out of available range.

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Alexey Moksyakov (yavtuk)

Changes

ldr reg, literal instruction is limited +/- 1MB range, emitCI put the constants by the end of function and the one is out of available range.

---

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

1 Files Affected:

• (added) bolt/test/AArch64/materialize-constant.s (+74)

diff --git a/bolt/test/AArch64/materialize-constant.s b/bolt/test/AArch64/materialize-constant.s
new file mode 100644
index 0000000000000..1c15626b09594
--- /dev/null
+++ b/bolt/test/AArch64/materialize-constant.s
@@ -0,0 +1,74 @@
+// this test checks a load literal instructions changed to movk
+
+// REQUIRES: system-linux
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
+
+# RUN: link_fdata %s %t.o %t.fdata
+# RUN: %clang %cflags -pie %t.o -o %t.exe -Wl,-q -Wl,-z,relro -Wl,-z,now
+# RUN: llvm-bolt %t.exe -o %t.bolt -data %t.fdata \
+# RUN:    --keep-nops --eliminate-unreachable=false
+# RUN: llvm-objdump --disassemble-symbols=foo %t.bolt | FileCheck %s
+
+# CHECK: mov{{.*}} w19, #0
+# CHECK-NEXT: mov{{.*}} w22, #0
+# CHECK-NEXT: movk{{.*}} w23, #0, lsl #16
+# CHECK-NEXT: movk{{.*}} w23, #100
+# CHECK-NEXT: movk{{.*}} w24, #0, lsl #16
+# CHECK-NEXT: movk{{.*}} w24, #3
+
+  .text
+  .align 4
+  .local foo
+  .type foo, %function
+foo:
+# FDATA: 1 main 0 1 foo 0 0 10
+    stp x29, x30, [sp, #-32]!
+    stp x19, x20, [sp, #16]
+    mov x29, sp
+
+    mov w19, #0 // counter = 0
+    mov w22, #0 // result = 0
+
+    ldr w23, .Llimit
+    ldr w24, .LStep
+    b .LStub
+
+.LConstants:
+  .Llimit: .word 100
+  .LStep:  .word 3
+
+.LStub:
+.rep 0x100000
+    nop
+.endr
+    b .Lmain_loop
+
+.Lmain_loop:
+    madd w22, w19, w24, w22  // result += counter * increment
+
+    add w19, w19, #1
+    cmp w19, w23
+    b.lt .Lmain_loop
+
+    mov w0, w22
+
+    b .Lreturn_point
+
+.Lreturn_point:
+    ldp x19, x20, [sp, #16]
+    ldp x29, x30, [sp], #32
+    ret
+.size foo, .-foo
+
+
+  .global main
+  .type main, %function
+main:
+  mov x0, #0
+  bl foo
+  mov     x0, 0
+  mov     w8, #93
+  svc     #0
+
+  .size main, .-main

[yavtuk]

hi folks, I've faced with the issue related to ldr reg, literal instruction inside function with constant island. after emitCI the constants are placed by the end of the function and the distance to target for ldr is out of range.

I think such cases are possibly for ADR instruction either and also after long jump pass.
I found that this can be solved by functionality which we have for X86 in simplifyROdata pass.
I wrote quick pass for checking and replace ldr -> movk.
The case is not common that why how do you think should we add the separate pass for verification such cases or extend functionality for simplifyROdata for aarch64 and move the functionality from under the option to a permanent basis?

[yozhu]

Thanks for the patch! Wondering if it would be useful to combine this "ldr->movk" transformation with some sort of ldr relaxation (as in #165787), particularly for cases where the constant island content has (dynamic) relocation and need to be fixed up?

[maksfb]

@yavtuk, great idea! If this is integrated into SimplifyRODataLoads pass then it makes sense to run it by default when it's strictly beneficial to do so. I.e., I consider replacing LDR with one or two MOVK to be a performance benefit. For more instructions in case of 64-bit load it's more complicated. #137413 is relevant to immediate generation on AArch64.

Extra step on top will be elimination of the constant island that is no longer referenced.

Running LDR-relaxation pass afterwords will catch cases where the simplification is not possible or is not the best option. We will make use of both passes.

[paschalis-mpeis]

It would be good to handle this in the suggested pass, also reporting how many times it was applied.
Eliminating any converted islands with no further users sounds like a nice addition on top!

From discussion, this doesn't appear to be a common case.
If it turns out to be more common than we now think, we could consider keeping the island close to its users, as per assembler's original intent, given it is feasible.