[PATCH] D45642: [PPC64] V2 abi: Add glink section for lazy symbol resolution.

[Sean Fertile via Phabricator via llvm-commits]

sfertile marked 6 inline comments as done.
sfertile added inline comments.


================
Comment at: ELF/SyntheticSections.cpp:1208
+  // machine.
+  if (Config->EMachine == EM_PPC64 && ELF64LEKind &&
+      !InX::Glink->empty()) {
----------------
sfertile wrote:
> grimar wrote:
> > Did you mean `Config->EMachine == EM_PPC64 && Config->IsLE` ?
> > 
> > Given the comment, should `Config->IsLE` check be omitted?
> > 
> > Actually, it seems it could be the following then.
> > 
> > `if (InX::Glink && !InX::Glink->empty()) { ...`
> Yeah I meant `Config->EMachine == EM_PPC64 && Config->IsLE`,  but what I should be checking is `Config->EMachine == EM_PPC64 &&  Config->EFlags == 2`.
> 
> I've since added the eflag check I want in https://reviews.llvm.org/D46076 
Zaara committed a patch that removes support for the V1 abi, so I no longer need to check the abi version here.


================
Comment at: ELF/SyntheticSections.cpp:882-883
 GotPltSection::GotPltSection()
-    : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_PROGBITS,
-                       Target->GotPltEntrySize, ".got.plt") {}
+    : SyntheticSection(SHF_ALLOC | SHF_WRITE,
+                       Config->EMachine == EM_PPC64 ? SHT_NOBITS : SHT_PROGBITS,
+                       Target->GotPltEntrySize,
----------------
ruiu wrote:
> sfertile wrote:
> > ruiu wrote:
> > > BSS-ish .got.plt looks pretty odd. Is this really what the spec says? It at least needs some explanation in comment for a reader.
> > yeah this is odd, I don't know why its made to look like a .bss section but it is specified by the abi (both V1 and V2 64 bit abis as well as the 32 bit abi I believe)
> Can you add this comment as a code comment? I believe honest comment like that will be useful in the future.
Yep, added.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D45642