[PATCH] D55248: [ELF] Simplify getSectionPiece

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D55248#1318096 <https://reviews.llvm.org/D55248#1318096>, @grimar wrote:

> I think this kills the additional optimization we have here. Unfortunately, I do not remember
>  how much improvement this piece had in comparison with the simple code as your patch has,
>  but this was the code initially implemented by Rafael in r284594, which comment says:
>
>   Add a faster binary search.
>  
>   Even with the hash table cache, binary search was still pretty
>   hot. This can be made even faster with prefetching.
>  
>   Idea from http://cglab.ca/~morin/misc/arraylayout-v2/
>  
>   I will suggest moving this to llvm.
>
>
> I do not know if that was moved to llvm (or if that what we might want atm, if not).


The optimization is that by using a simple conditional assignment, the compilers will know to generate conditional move (branch-less binary search as the paper says)

  // either one below generates conditional mov
  if (Pieces[Idx + H].InputOff <= Offset)
   Idx += H;
  
  Idx = Pieces[Idx + H].InputOff <= Offset ? Idx + H : Idx;
  
  // but std::upper_bound doesn't 

The (negligible) disadvantage of this approach is that in each iteration, it reduces the range size from `n` to `ceil(n/2)`, instead of `floor(n/2)` as is the case of `std::upper_bound` (which produces the `not; add` instruction sequence to do `size -= H+1`).

But note `OffsetMap.find(Offset)` also takes some time. The improvement here is negligible to me..

A static `-DCMAKE_BUILD_TYPE=Debug` build of `bin/clang-8`:

`perf stat -r 10 ~/llvm/Release/bin/ld.lld @response.txt -o /dev/null` => The times vary from 4.5435 to 4.7403 with or without this optimization. The differences are purely noise.. I cannot find the (very minor) optimization (branch instruction -> conditional move) improve the linking time.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D55248