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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I've spotted what I think is one mistake, R_AARCH64_P32_TLSDESC_ADR_PREL19 should be R_AARCH64_P32_TLSDESC_ADR_PREL21. I've put some suggestions for some missing test cases as well. Other than that I can't spot any obvious problems.



================
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)
----------------
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)



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


================
Comment at: test/MC/AArch64/arm32-tls-relocs.s:127
+
+
+////////////////////////////////////////////////////////////////////////////////
----------------
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. 


================
Comment at: test/MC/AArch64/arm32-tls-relocs.s:235
+// CHECK-ELF-ILP32-NEXT:     {{0x[0-9A-F]+}} R_AARCH64_P32_TLSLD_LDST64_DTPREL_LO12_NC [[VARSYM]]
+
+////////////////////////////////////////////////////////////////////////////////
----------------
Is it worth a test case for R_AARCH64_P32_TLSLD_LDST128_DTPREL_LO12 and R_AARCH64_P32_TLSLD_LDST128_LO12_NC forms?


================
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']
----------------
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


================
Comment at: test/MC/AArch64/arm32-tls-relocs.s:270
+// CHECK-ELF-ILP32-NEXT:     Binding: Global
+// CHECK-ELF-ILP32-NEXT:     Type: TLS
----------------
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


Repository:
  rL LLVM

https://reviews.llvm.org/D32072