[PATCH] D87504: [PowerPC][LLD] Support for PC Relative TLS for Local Dynamic

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: lld/ELF/Arch/PPC64.cpp:793
+    // Relax from paddi r3, 0, x1 at got@tlsld at pcrel, 1 to
+    //            paddi 3, 13, 0x1000, 0
+    writePrefixedInstruction(loc, 0x06000000386d1000); // paddi 3, 13, 0x1000, 0
----------------
missing the 'r' on the registers.


================
Comment at: lld/ELF/Arch/PPC64.cpp:794
+    //            paddi 3, 13, 0x1000, 0
+    writePrefixedInstruction(loc, 0x06000000386d1000); // paddi 3, 13, 0x1000, 0
+    break;
----------------
Real minor nit: No need to duplicate the comment on this line.


================
Comment at: lld/ELF/Arch/PPC64.cpp:798
+    // PC Relative Relaxation:
+    // Relax from bl __tls_get_addr at notoc(x at tlsld) to
+    //            nop
----------------
Real minor nit: move the 'to' to the next line. It matches how you structured the comment on the toc case, and helps to delineate the original code from the optimized code.


================
Comment at: lld/ELF/Arch/PPC64.cpp:1323
+    // need to subtract that value.
+    if (type == R_PPC64_DTPREL34)
+      val -= 0x8000;
----------------
Would you consider having the `R_PPC64_DTPREL34` case come first and adjust the value, then fall through to the common case?


================
Comment at: lld/ELF/Arch/PPC64.cpp:1324
+    if (type == R_PPC64_DTPREL34)
+      val -= 0x8000;
     const uint64_t si0Mask = 0x00000003ffff0000;
----------------
We have a constant for this already:  `dynamicThreadPointerOffset`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87504