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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Arch/PPC64.cpp:883
+    } else {
+      error("R_PPC64_TLS must be either 4 byte aligned or one byte offset "
+            "from 4 byte aligned");
----------------
errorOrWarn


================
Comment at: lld/test/ELF/Inputs/ppc64-tls-vardef.s:3
+.section .tbss,"awT", at nobits
+.globl	x
+x:
----------------
You can delete `.size` and move this snippet to the main file with `RUN: split-file`


================
Comment at: lld/test/ELF/ppc64-tls-pcrel-ie.s:23
+
+## This test checks the Initial Exec PC Relative TLS implementation for lld.
+## The SHARE version checks that the relocations are generated correctly.
----------------
You can omit `for lld` as the whole directory is testing lld.


================
Comment at: lld/test/ELF/ppc64-tls-pcrel-ie.s:24
+## This test checks the Initial Exec PC Relative TLS implementation for lld.
+## The SHARE version checks that the relocations are generated correctly.
+## The STATIC version checks that the Initial Exec to Local Exec relaxation is
----------------
Suggest: IE and LE instead of SHARED and STATIC.

It is normally incorrect to use IE for -shared. It is for very narrow cases where dlopen is not considered and the TLS size is small.


================
Comment at: lld/test/ELF/ppc64-tls-pcrel-ie.s:92
+	lwzx 4, 4, y at tls@pcrel
+	add 3, 4, 3
+	clrldi	3, 3, 32
----------------
Please remove unneeded instructions like add and clrldi across the file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86893