[PATCH] D27609: Fix R_AARCH64_MOVW_UABS_G3 relocation

[Yichao Yu via Phabricator via llvm-commits]

yuyichao added inline comments.


================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:480-481
     // from bits 11:3 of X
-    *TargetPtr |= ((Result & 0xff8) << (10 - 3));
+    TargetValue |= ((Result & 0xff8) << (10 - 3));
+    support::ulittle32_t::ref{TargetPtr} = TargetValue;
     break;
----------------
davide wrote:
> any reason why `(10 - 3)` is not spelled `7` (here and everywhere else)
I believe it's because these are the last bits of the source (3) and the destination (10). I'm fine with either but I think the `10 - 3` is reasonably clear what it is doing along with the comment above each of these.


================
Comment at: test/ExecutionEngine/RuntimeDyld/AArch64/ELF_ARM64_relocations.s:1-2
+# RUN: llvm-mc -triple=arm64-none-linux-gnu -filetype=obj -o %T/reloc.o %s
+# RUN: llvm-rtdyld -triple=arm64-none-linux-gnu -verify -dummy-extern f=0x0123456789abcdef -check=%s %T/reloc.o
+
----------------
davide wrote:
> We need tests for both little and big endian, no?
Sure. I don't have hardware to actually check if it's actually the correct output but I'll just reference clang/gcc output in be mode.


================
Comment at: test/ExecutionEngine/RuntimeDyld/AArch64/ELF_ARM64_relocations.s:9-15
+        movz    x0, #:abs_g3:f
+        movk    x0, #:abs_g2_nc:f
+        movk    x0, #:abs_g1_nc:f
+        movk    x0, #:abs_g0_nc:f
+        ret
+        .Lfunc_end0:
+        .size   g, .Lfunc_end0-g
----------------
davide wrote:
> Can you add comments pointing out the relocations emitted?
Sorry, what exactly do you mean? The relocations are explicitly specified in the code (`#:abs_g3:` etc.). Do you mean to write the name like `R_AARCH64_MOVW_UABS_G3`? Or something else?


https://reviews.llvm.org/D27609