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

Wed Jun 1 05:52:44 PDT 2022

sunho added inline comments.

================
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");
+
----------------
sgraenitz wrote:
> Alignment can not be smaller then one byte right? Can we skip the check in this case?
I realized the I wrote a very confusing error message here. The more clear message would be "R_AARCH64_LDST8_ABS_LO12_NC target is not a LDRB/STRB (imm12) instruction." I'll change to that. 

We still need to check here because technically R_AARCH64_LDST8_ABS_LO12_NC relocation needs the address to be not shifted at all, but if instruction is like LDRH, we're going to shift it left by 1 since getPageOffset12Shift(Instr) returns 1.

I belive such a relocation should not be generated if codegen is doing sane thing, but we still need to check just in case.

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

https://reviews.llvm.org/D126630