[PATCH] D126287: [JITLink][ELF/AARCH64] Implement R_AARCH64_ADR_PREL_PG_HI21 and R_AARCH64_ADD_ABS_LO12_NC

[Sunho Kim via Phabricator via llvm-commits]

sunho added inline comments.


================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp:57
+    case ELF::R_AARCH64_ADD_ABS_LO12_NC:
+      return ELF_aarch64_Edges::ELFAddAbs12;
     }
----------------
sgraenitz wrote:
> The `using namespace` on file level makes the explicit qualifications here redundant right? It's a detail, but let's agree on one option and stick to it consistently. (More cases below.)
In other backends, using namespace seems to be more common. I changed the code to consistently do without namespace.


================
Comment at: llvm/test/ExecutionEngine/JITLink/AArch64/ELF_aarch64_relocations.s:49
+        .globl  named_data
+        .p2align  4
+        .type   named_data, at object
----------------
sgraenitz wrote:
> The value of p2align varies between test cases. How does it affect the resolution? (Trying other values like 1 or 2 doesn't fail the test.)
It didn't affect the test result since we don't execute the code (-noexec), but in arm64, function entries need to be aligned to 4 bytes. Although techincally not required, I just added p2align 2 to function entry for the sake of cleaness.

The reason for p2align 4 in named_data is since it might be used to test 128bit/64bit data load/store.



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

https://reviews.llvm.org/D126287