[PATCH] D53408: [PPC64] Long branch thunks.

[Sean Fertile via Phabricator via llvm-commits]

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


================
Comment at: ELF/Arch/PPC64.cpp:724
+
+  // If a symbol is a weak undefined and we are compiling an executable
+  // it doesn't need a range-extending thunk since it can't be called.
----------------
MaskRay wrote:
> executable -> position-dependent executable?
> 
> `Config->Pic` is true when linking .so or PIE executable.
Good catch. I intended to only exclude shared objects here.


================
Comment at: ELF/Symbols.h:214-216
+  // True if this symbol is in the Plt.
+  unsigned IsInPlt : 1;
+
----------------
ruiu wrote:
> nit: I'd add this before `IsInIplt` as it seems a more natural place to add this bit. (New code shouldn't look new but should be written as if it were there from the beginning.)
"New code shouldn't look new but should be written as if it were there from the beginning" <-- I like that, I'll keep it in mind from now on :)


================
Comment at: test/ELF/ppc64-long-branch.s:66
+
+        .section        .toc,"aw", at progbits
+.LC0:
----------------
smeenai wrote:
> MaskRay wrote:
> > The indentation (this section and the instructions below `.localentry`) does not look good in Phabricator. Is it intended?
> Phabricator always displays tabs as two spaces, so there might be some weird mixing of tabs and spaces going on.
Wasn't intended. There was a mix of tabs and spaces in the parts of the code that was generated from C, as well as I simply miss-formatted some of the parts I wrote. Fixed up now.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D53408