[PATCH] D86893: [PowerPC] Add support for R_PPC64_GOT_TPREL_PCREL34 used in TLS Initial Exec

[Fangrui Song via Phabricator via llvm-commits]

MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.

Seems that this require a dependent patch to land first. I can only get around to it after llvm-mc can assemble the special form R_PPC64_TLS. Requested changes so that I will not forget it.



================
Comment at: lld/ELF/Arch/PPC64.cpp:843
+    const uint64_t pld = readPrefixedInstruction(loc);
+    const uint64_t pldRT = pld & 0x0000000003e00000;
+    // paddi RT(from pld), r13, symbol at tprel, 0
----------------
pld is used only once and can be inlined


================
Comment at: lld/ELF/Arch/PPC64.cpp:851
+    const uintptr_t locAsInt = reinterpret_cast<uintptr_t>(loc);
+    if ((locAsInt % 4) == 0) {
+      uint32_t primaryOp = getPrimaryOpCode(read32(loc));
----------------
`locAstInt % 4 == 0`


================
Comment at: lld/ELF/Arch/PPC64.cpp:861
+      relocateNoSym(loc + offset, R_PPC64_TPREL16_LO, val);
+    } else if ((locAsInt % 4) == 1) {
+      // If the offset is not 4 byte aligned then we have a PCRel type reloc.
----------------
locAsInt % 4 == 1


================
Comment at: lld/ELF/Arch/PPC64.cpp:870
+      // The add is a special case and can be turned into a nop.
+      if (secondaryOp == 266)
+        write32(loc - 1, 0x60000000);
----------------
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

Add braces to both `then` and `else`


================
Comment at: lld/ELF/Arch/PPC64.cpp:875
+        if (dFormOp == 0)
+          error("unrecognized instruction for IE to LE R_PPC64_TLS");
+        write32(loc - 1, ((dFormOp << 26) | (read32(loc - 1) & 0x03FF0000)));
----------------
Consider errorOrWarn whenever possible.



================
Comment at: lld/ELF/Arch/PPC64.cpp:880
+    else
+      error("R_PPC64_TLS must be either 4 byte aligned or one byte offset "
+            "from 4 byte aligned");
----------------
How is this conveyed in the ABI document (I don't have a copy)?

Is it: r_offset has to be 0 or 1 modulo 4?

Note that bufLoc is buffer address plus r_offset. bufLoc can be aligned while r_offset isn't.


================
Comment at: lld/test/ELF/ppc64-tls-pcrel-ie.s:11
+# RUN: llvm-mc -filetype=obj -triple=powerpc64le %p/Inputs/ppc64-tls-vardef.s -o %t-defs.o
+# RUN: ld.lld --shared %t-defs.o -o %t-defs.so
+# RUN: ld.lld -T %t.script %t.o %t-defs.so -o %t-shared
----------------
If a linked shared object is used to link another file, the shared object needs to specify `--soname=t` or another fixed string. Otherwise the DT_SONAME field of the other file is dependent on the full path of `%t2.so`, which can vary on different test machines.


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

https://reviews.llvm.org/D86893