[PATCH] D27609: Fix R_AARCH64_MOVW_UABS_G3 relocation

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

I don't have additional comments but please wait for Tim/Lang to sign this off.



================
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;
----------------
yuyichao wrote:
> 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.
Fine.


================
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
----------------
yuyichao wrote:
> yuyichao wrote:
> > 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?
> Or do you mean the disassemble of the relocated instructions?
The former (spelling the as `R_AARCH64_FOO`)


https://reviews.llvm.org/D27609