[PATCH] D32072: [AArch64] ILP32 Backend Relocation Support

[Joel Jones via Phabricator via llvm-commits]

joelkevinjones added a comment.

As noted in the inline comments, there are several relocations with no implementations in LP64 or ILP32. My intent with this patch was towards the goal of ILP32 parity with LP64. I still need to file a defect for the missing functionality.



================
Comment at: include/llvm/Support/ELFRelocs/AArch64.def:205
+ELF_RELOC(R_AARCH64_P32_TLSDESC_LD_PREL19,           0x07a)
+ELF_RELOC(R_AARCH64_P32_TLSDESC_ADR_PREL19,          0x07b)
+ELF_RELOC(R_AARCH64_P32_TLSDESC_ADR_PAGE21,          0x07c)
----------------
peter.smith wrote:
> I think this should be R_AARCH64_P32_TLSDESC_ADR_PREL21
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056c/IHI0056C_beta_aaelf64.pdf has this as 
> R_<CLS>_TLSDESC_ADR_PREL21 (search for 123)
> 
Concur


================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp:22
 #include "llvm/MC/MCValue.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/ELF.h"
----------------
peter.smith wrote:
> It isn't obvious from the changes why this is needed, just checking if it has been left in by mistake?
Concur


================
Comment at: test/MC/AArch64/arm32-tls-relocs.s:127
+
+
+////////////////////////////////////////////////////////////////////////////////
----------------
peter.smith wrote:
> I think that there are also the R_AARCH64_P32_TLSLE_LD128_TPREL_LO12 and LO12_NC, I see that there is a test case in arm32-elf-relocs.s, but it would be nice to do the additional checks here. 
Concur


================
Comment at: test/MC/AArch64/arm32-tls-relocs.s:245
+        blr x3
+
+// CHECK-ILP32: adrp    x8, :tlsdesc:var        // encoding: [0x08'A',A,A,0x90'A']
----------------
peter.smith wrote:
> I think that there is a missing test needed to generate the R_AARCH64_TLSDESC_ADR_PREL21 relocation, this is the one I pointed out earlier had been given the code R_AARCH64_TLSDESC_ADR_PREL19
Currently, there is no implementation for either LP64 or ILP32 for TLSDESC_ADR_PREL21.


================
Comment at: test/MC/AArch64/arm32-tls-relocs.s:270
+// CHECK-ELF-ILP32-NEXT:     Binding: Global
+// CHECK-ELF-ILP32-NEXT:     Type: TLS
----------------
peter.smith wrote:
> The general dynamic forms are missing tests:
> R_AARCH64_P32_TLSGD_ADR_PREL21
> R_AARCH64_P32_TLSGD_ADR_PAGE21
> R_AARCH64_P32_TLSGD_ADD_LO12_NC
There are currently no LP64 or ILP32 implementations for these relocations.


Repository:
  rL LLVM

https://reviews.llvm.org/D32072