[PATCH] D65242: [ELF] More dynamic relocation packing

Wed Aug 7 14:07:05 PDT 2019

victoryang added inline comments.

================
Comment at: lld/ELF/SyntheticSections.cpp:1745
+    add(g.size());
+    add(RELOCATION_GROUPED_BY_INFO_FLAG | hasAddendIfRela |
+        groupedByAddendIfRela);
----------------
pcc wrote:
> I don't think you need to set `hasAddendIfRela | groupedByAddendIfRela`. If these flags are not set, the unpacker will use 0 as the addend value. See:
> https://android.googlesource.com/platform/bionic/+/master/linker/linker_reloc_iterators.h#142
> 
> I think this also means that you don't need to reorder non-relatives before relatives.
Ah nice! I was assuming without grouping by addend, we'd always need to emit addend for each relocation entry. Thanks for pointing this out! This indeed is the simplest way to do it.

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

https://reviews.llvm.org/D65242