[lld][LoongArch] Handle extreme code model relocs according to psABI v2.30

[SixWeining]

psABI v2.30 requires the extreme code model instructions sequence (pcalau12i+addi.d+lu32i.d+lu52i.d) to be adjacent.

See #71907 and loongson-community/discussions#17 for details.

[llvmbot]

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Lu Weining (SixWeining)

Changes

Defer the compution of negativeB because adding 0x1000 to original result may yield a different negativeB value. Actually this issue was first reported by @rui314 at https://reviews.llvm.org/D138135#4568594.

Note that even with this patch, the handling of R_LARCH_PCALA64_* relocs are NOT totally correct, because current approach assumes those four instructions (pcalau12i/addi.d/lu32i.d/lu52i.d) are in the same 4K-page which is not always true. It is possible to document this assumption as a constraint in psABI in future. But at least this patch is necessary.

See #71907 and loongson-community/discussions#17 for details.

---

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

2 Files Affected:

• (modified) lld/ELF/Arch/LoongArch.cpp (+1-2)
• (modified) lld/test/ELF/loongarch-pc-aligned.s (+10-12)

diff --git a/lld/ELF/Arch/LoongArch.cpp b/lld/ELF/Arch/LoongArch.cpp
index 1c3e015efc1649a..9291ff14d674b64 100644
--- a/lld/ELF/Arch/LoongArch.cpp
+++ b/lld/ELF/Arch/LoongArch.cpp
@@ -157,10 +157,9 @@ uint64_t elf::getLoongArchPageDelta(uint64_t dest, uint64_t pc) {
   //     result = page(dest) - page(pc) + 0x1000
   uint64_t result = getLoongArchPage(dest) - getLoongArchPage(pc);
   bool negativeA = lo12(dest) > 0x7ff;
-  bool negativeB = (result & 0x8000'0000) != 0;
-
   if (negativeA)
     result += 0x1000;
+  bool negativeB = (result & 0x8000'0000) != 0;
   if (negativeA && !negativeB)
     result -= 0x10000'0000;
   else if (!negativeA && negativeB)
diff --git a/lld/test/ELF/loongarch-pc-aligned.s b/lld/test/ELF/loongarch-pc-aligned.s
index e7950400a5c8c45..742ac502e799804 100644
--- a/lld/test/ELF/loongarch-pc-aligned.s
+++ b/lld/test/ELF/loongarch-pc-aligned.s
@@ -260,31 +260,29 @@
 # EXTREME15-NEXT: lu32i.d   $t0, -349526
 # EXTREME15-NEXT: lu52i.d   $t0, $t0, -1093
 
-## FIXME: Correct %pc64_lo20 should be 0xfffff (-1) and %pc64_hi12 should be 0xfff (-1), but current values are:
-## page delta = 0x0000000000000000, page offset = 0x888
+## page delta = 0xffffffff00000000, page offset = 0x888
 ## %pc_lo12   = 0x888 = -1912
 ## %pc_hi20   = 0x00000 = 0
-## %pc64_lo20 = 0x00000 = 0
-## %pc64_hi12 = 0x00000 = 0
+## %pc64_lo20 = 0xfffff = -1
+## %pc64_hi12 = 0xfff = -1
 # RUN: ld.lld %t/extreme.o --section-start=.rodata=0x0000000012344888 --section-start=.text=0x0000000012345678 -o %t/extreme16
 # RUN: llvm-objdump -d --no-show-raw-insn %t/extreme16 | FileCheck %s --check-prefix=EXTREME16
 # EXTREME16:      addi.d $t0, $zero, -1912
 # EXTREME16-NEXT: pcalau12i $t1, 0
-# EXTREME16-NEXT: lu32i.d   $t0, 0
-# EXTREME16-NEXT: lu52i.d   $t0, $t0, 0
+# EXTREME16-NEXT: lu32i.d   $t0, -1
+# EXTREME16-NEXT: lu52i.d   $t0, $t0, -1
 
-## FIXME: Correct %pc64_lo20 should be 0x00000 (0) and %pc64_hi12 should be 0x000 (0), but current values are:
-## page delta = 0xffffffff80000000, page offset = 0x888
+## page delta = 0x0000000080000000, page offset = 0x888
 ## %pc_lo12   = 0x888 = -1912
 ## %pc_hi20   = 0x80000 = -524288
-## %pc64_lo20 = 0xfffff = -1
-## %pc64_hi12 = 0xfff = -1
+## %pc64_lo20 = 0xfffff = 0
+## %pc64_hi12 = 0xfff = 0
 # RUN: ld.lld %t/extreme.o --section-start=.rodata=0x000071238ffff888 --section-start=.text=0x0000712310000678 -o %t/extreme17
 # RUN: llvm-objdump -d --no-show-raw-insn %t/extreme17 | FileCheck %s --check-prefix=EXTREME17
 # EXTREME17:      addi.d $t0, $zero, -1912
 # EXTREME17-NEXT: pcalau12i $t1, -524288
-# EXTREME17-NEXT: lu32i.d   $t0, -1
-# EXTREME17-NEXT: lu52i.d   $t0, $t0, -1
+# EXTREME17-NEXT: lu32i.d   $t0, 0
+# EXTREME17-NEXT: lu52i.d   $t0, $t0, 0
 
 #--- a.s
 .rodata

[SixWeining]

cc @MQ-mengqing @xen0n @xry111

[SixWeining]

With this PR, lld works the same as bfd and mold on those edge cases. See the test here https://gist.github.com/SixWeining/c5df38a2a21e18a32a82f199645ea5ab.

[rui314]

It looks like bfd and mold use essentially the same logic. If you believe their logic is correct, why don't you just copy that logic too? Doing that eliminates the need to check the code to see if the new code would produce the same output as the others for all possible inputs.

[xry111]

IMO we should document the "recommended practice" for R_LARCH_PCALA64_* in the ABI doc and use the same algorithm everywhere.

[rui314]

@xry111 Agreed. That'd be very useful.

[SixWeining]

It looks like bfd and mold use essentially the same logic. If you believe their logic is correct, why don't you just copy that logic too? Doing that eliminates the need to check the code to see if the new code would produce the same output as the others for all possible inputs.

Thanks, just now I update the PR to use the same logic as bfd and mold.

[SixWeining]

IMO we should document the "recommended practice" for R_LARCH_PCALA64_* in the ABI doc and use the same algorithm everywhere.

Yes, I think that will happen in next psABI update.

[heiher]

Let's take a step forward?

[MaskRay]

Partially fix the handling

Why is this fix partial? Can it be complete?

I agree that psABI documenting the recommended algorithm will be great.

[MaskRay]

.

[SixWeining]

Partially fix the handling

Why is this fix partial? Can it be complete?

I agree that psABI documenting the recommended algorithm will be great.

The wording Partially means if those 4 instructions are not in the same 4K-page the algorithm is wrong. Of course, if the psABI documents the recommended algorithm, this would be a complete fix.

[SixWeining]

Ping.