[PATCH] D61477: [ELF] -z combreloc: sort dynamic relocations by (!is_relative,symbol_index,r_offset)

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

ld.bfd's sorting logic is unnecessarily complex.
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=41f61c65a2e1cfdb4aca3bccf6e11025495ba02e;f=bfd/elflink.c#l9305
It uses elf_link_sort_cmp1 to sort relative relocations, then elf_link_sort_cmp2 to sort the non-relative relocations. The two functions can be combined to one: sorting by the triple `(!IsRelative, SymIndex, r_offset)`

gold's rule is at https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=41f61c65a2e1cfdb4aca3bccf6e11025495ba02e;f=gold/output.cc#l1180
As @mcgrathr mentioned in PR41692, it has the fourth key SymType, which probably just serves the purpose of `std::stable_sort`.

By using `std::make_tuple()`, this added a small cost as we unconditionally call `A.getOffset()` and `B.getOffset()` - this cost doesn't matter.

- Non-PIE executables/shared objects don't have many dynamic relocations.
- Most dynamic relocations of an PIE are relative relocations. The comparison function has to call `getOffset()` anyway.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D61477