[PATCH] D82816: [LLD][PowerPC] Implement R_PPC64_REL24_NOTOC calls, callee also has no TOC

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Thanks for the patch. I haven't looked into the detail closely. Some suggestions about the test inlined.



================
Comment at: lld/test/ELF/ppc64-reloc-rel24notoc-externcall-samedso-callee-st-other0.s:3
+# RUN: echo 'SECTIONS { \
+# RUN:       .text_low 0x10010158: { *(.text_low) } \
+# RUN:       }' > %t.script
----------------
How about 0x10000000 or 0x10010000?

2 space indentation is sufficient.


================
Comment at: lld/test/ELF/ppc64-reloc-rel24notoc-externcall-samedso-callee-st-other0.s:8
+# RUN: llvm-mc -filetype=obj -triple=powerpc64le %s -o %t.o
+# RUN: echo '.globl  callee; \
+# RUN:  callee:; \
----------------
Extra space after `.globl`


================
Comment at: lld/test/ELF/ppc64-reloc-rel24notoc-externcall-samedso-callee-st-other0.s:29
+# SYMBOL: 1: 0000000010010164     0 NOTYPE  LOCAL  DEFAULT [<other: 0x20>]   2 caller1
+# SYMBOL: 2: 00000000100101a0     0 NOTYPE  LOCAL  DEFAULT [<other: 0x20>]   2 caller2
+# SYMBOL: 3: 0000000010010158     0 NOTYPE  LOCAL  HIDDEN                   1 callee
----------------
Add `-NEXT:` if applicable


================
Comment at: lld/test/ELF/ppc64-reloc-rel24notoc-externcall-samedso-callee-st-other0.s:33
+
+# CHECK: 0000000010010158 <callee>
+
----------------
```
# CHECK-LABEL: <callee>:
# CHECK:         10010180:       bl 0x10010158
```

The idea is that functions should use `-LABEL:` and its covered instructions can be indented by two columns (to make it clear it is in the function). The next line 


================
Comment at: lld/test/ELF/ppc64-reloc-rel24notoc-externcall-samedso-callee-st-other0.s:46
+caller1:
+        .localentry     caller1, 1
+        mflr 0
----------------
8 space indentation is too wasteful. 2 is sufficient.


================
Comment at: lld/test/ELF/ppc64-reloc-rel24notoc-externcall-samedso-callee-st-other0.s:49
+        std 30, -16(1)
+        std 0, 16(1)
+        stdu 1, -48(1)
----------------
Remove unneeded instructions to make the tested part stand out.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82816/new/

https://reviews.llvm.org/D82816