[PATCH] D118346: [ORC] Support ADR_PREL_PG_HI21 & ADD_ABS_LO12_NC for aarch64

[Stefan Gränitz via Phabricator via llvm-commits]

sgraenitz added a comment.

Thanks for the patch. I can't dig into the details deeper right now, but if the test issues remain I do so next week.



================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp:107
 private:
-  static Expected<aarch64::EdgeKind_aarch64>
-  getRelocationKind(const uint32_t Type) {
+  Expected<aarch64::EdgeKind_aarch64> getRelocationKind(const uint32_t Type) {
     using namespace aarch64;
----------------
Unrelated change?


================
Comment at: llvm/test/ExecutionEngine/JITLink/AArch64/ELF_aarch64_adrp.s:8
+# Test Case 1: R_AARCH64_ADR_PREL_PG_HI21
+# jitlink-check: decode_operand(main, 1) = (((str + 0) & 0xFFFFFFFFFFFFF000) - (main & 0xFFFFFFFFFFFFF000))[32:12]
+#
----------------
lhames wrote:
> dongAxis1944 wrote:
> > Hello, since I do not write raw assembly by hand, this case might be optimized.
> > 
> > 1.  The addend of this case is 0, I do not know how to set it to the other value;
> > 2.  The target and str are on the same page, so "decode_operand(main, 1)" is always zero, should I put the two symbols on different pages?
> This doesn't pass on my machine. Is it passing for you locally?
> 
> 1. I think you could do it by referencing an offset from str? (That's how I'd do it on MachO, but I haven't checked on ELF).
> 
> 2. You could use a fill directive to insert some filler in a way that would guarantee that str would be on a different page. That's worth testing too (both the "same page" and "different page" cases.
> 
> Your testcase doesn't actually need to run, so I'd make main separate and name each instruction to emphasize the test;
> 
> ```
> # RUN: rm -rf %t && mkdir -p %t
> # RUN: llvm-mc -triple=aarch64-unknown-linux-gnu -relax-relocations=false -position-independent -filetype=obj -o %t/elf_aarch64_adrp.o %s
> # RUN: llvm-jitlink -noexec -check %s %t/elf_aarch64_adrp.o
> 
>         .text
> 
> # Add a no-op main as an entry-point.
>         .globl    main
>         .p2align        2
> main:
>         ret
> 
> # Test R_AARCH64_ADR_PREL_PG_HI21
> # jitlink-check: decode_operand(test_page21, 1) = str[32:12] - test_page21[32:12]
>         .globl  test_page21
>         .p2align        2
>         .type test_page21, at function
> test_page21:
>         adrp    x0, str
>         .size   test_page21, .-test_page21
> 
> # Test R_AARCH64_ADD_ABS_LO12_NC
> # jitlink-check: decode_operand(test_pageoff12, 2) = str[11:0]
>         .globl  test_pageoff12
>         .p2align        2
>         .type test_pageoff12, at function
> test_pageoff12:
>         add     x0, x0, :lo12:str
>         .size   test_pageoff12, .-test_pageoff12
> 
>         .globl  str
>         .type   str, at object
> str:
>         .asciz  "hello world"
>         .size   str, 12
> ```
Same here. The current test is failing for me with:
```
Error evaluating expression 'decode_operand(main, 1) = (((str + 0) & 0xFFFFFFFFFFFFF000) - (main & 0xFFFFFFFFFFFFF000))[32:12]': Couldn't decode instruction at 'main'
```

I still see the same `Couldn't decode instruction` error with Lang's proposed test. Not sure what is the issue, but it seems related to the `str` operand in `adrp x0, str`. It doesn't show up with an immediate value like `#0x1000`.

> Test Plan:
> check-llvm

BTW from your build root you should be able to run the single test this way:
```
> bin/llvm-lit -vv --filter=ELF_aarch64_adrp test
```


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

https://reviews.llvm.org/D118346