[PATCH] D54720: [PPC64] toc-indirect to toc-relative relaxation.

[Sean Fertile via Phabricator via llvm-commits]

sfertile marked an inline comment as done.
sfertile added inline comments.


================
Comment at: ELF/Arch/PPC64.cpp:126
+
+  // The relocations are sorted by offset, and while the most common case is
+  // that every .toc entry will have a relocation, it is not guaranteed. For
----------------
MaskRay wrote:
> > The relocations are sorted by offset
> 
> IIRC this is not guaranteed but I think it is fine to lose toc optimization if the input does not have this property.
> 
> The linear search below `for (auto R : llvm::reverse(Relas.slice(0, Index))) {` concerns me. 
> 
> Is it possible to add a `LastRelaIndex` variable in `InputSectionBase::relocateAlloc`:
> 
>       if (!tryRelaxTocPPC64(Type, Rel, Expr, BufLoc, LastRelaIndex))
> 
> and keep it monotonically increasing in `getSymAndAddend`? If the input doesn't guarantee increasing `r_offset`, it may lose TOC optimizations for some relocations, but as I said it should be fine.
>> The relocations are sorted by offset
> IIRC this is not guaranteed
I believe its guaranteed for binutils tools. We also sort the relocations for the .toc section in `scanRelocs` in case there are other tools which don't produce the relocations in sorted order.

> The linear search below for (auto R : llvm::reverse(Relas.slice(0, Index))) { concerns me
I believe missing toc relocations to be an uncommon thing. As far as I am aware llvm will not place constants into the .toc section. gcc will for a few very specific cases but it is still not very common. xlc is the outlier here as I heard it will do this some what commonly despite there being no need to with the V2 abi. 

For the objects I found that were missing relocations when compiled with gcc , we either found the relocation we were looking for, or broke out of the search in a few comparisons. If a linear search does become a problem for some pathological inputs we could switch to a binary search but I think doing that initially we would end up pessimaling the common case for something that might not ever naturally occur.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54720