[PATCH] D62464: [PPC32] Improve 32-bit PowerPC: many applications linked against musl may work with this patch

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Generally looking great.



================
Comment at: ELF/Options.td:30
 
+def bss_plt: F<"bss-plt">, HelpText<"Use old-style BSS PLT for powerpc32">;
+
----------------
nit: powerpc32 → PowerPC32


================
Comment at: ELF/Options.td:311
 
+def secure_plt: F<"secure-plt">, HelpText<"Use new-style PLT for powerpc32 (default)">;
+
----------------
Ditto


================
Comment at: ELF/SyntheticSections.cpp:1039
+    Name = ".plt";
+  }
+  if (Config->EMachine == EM_PPC64) {
----------------
Can you add return here? Or maybe you want to make this a switch whose each "case" ends with a return.


================
Comment at: ELF/SyntheticSections.cpp:1053
+  if (Config->EMachine == EM_PPC && !Config->SecurePlt)
+    return 72 + 12 * Entries.size();
   return (Target->GotPltHeaderEntriesNum + Entries.size()) * Config->Wordsize;
----------------
This magic number needs a comment.


================
Comment at: ELF/SyntheticSections.cpp:2267
+          SHF_ALLOC | SHF_EXECINSTR, SHT_PROGBITS, 16,
+          Config->EMachine == EM_PPC || Config->EMachine == EM_PPC64 ? ".glink"
+                                                                     : ".plt"),
----------------
nit: add parentheses before ?.


================
Comment at: ELF/SyntheticSections.cpp:3227
+
+void PPC32Got2Section::finalizeContents() {
+  uint32_t Offset = 0;
----------------
Can you add a comment?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D62464