[PATCH] D126630: [JITLink][ELF/AARCH64] Implement R_AARCH64_LDST*_ABS_LO12_NC relocation types

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

sgraenitz added a comment.



> The difference between macho backend is that in ELF, the shift value is explicitly given by relocation type. lld generates the relocation type that matches with instruction bitwidth, so getting the shift value implicitly from instruction bytes should be fine in typical use cases.

Good explanation, thanks. Yes, I'd agree that's the best solution.

> Merge testcase

Thanks!



================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp:143
+            "R_AARCH64_LDST8_ABS_LO12_NC target is not a "
+            "byte alligend LD/ST (imm12) instruction");
+
----------------
Alignment can not be smaller then one byte right? Can we skip the check in this case?


================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp:154
+            "R_AARCH64_LDST16_ABS_LO12_NC target is not a "
+            "half alligend LD/ST (imm12) instruction");
+
----------------
Can we factor this out into a helper function like `checkAlignment(Instr, N)`? I guess it's sufficient for the error message to say something like "not aligned to N byte boundary".


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

https://reviews.llvm.org/D126630