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

[Vic Yang via Phabricator via llvm-commits]

victoryang added a comment.

In D65242#1618191 <https://reviews.llvm.org/D65242#1618191>, @MaskRay wrote:

> > Would it be simpler to only support an addend of 0 here? I reckon that non-zero addends are rare enough that we can just let them be emitted in ungroupedNonRelatives.
>
> I'd like @pcc's comment to be addressed. The three relocation types `R_*_JUMP_SLOT, R_*_GLOB_DAT, R_*_COPY` always have 0 r_addend. A symbolic relocation (`R_AARCH64_ABS64, R_X86_64_64`, etc) may have non-zero r_addend but they are rare. Non-relative relocation with non-zero r_addend can just be placed in `ungroupedNonRelatives`.


I don't disagree that non-zero addends are rare, but I don't think it'd be any simpler if we only support addends of zero. As it is now, the grouping logic only needs to compare adjacent entries. If we want to only group entries with zero addends, it'd need to check the value of addends in addition to what's currently implemented. What you are asking essentially comes down to adding the following code block to before line 1716:

  if (config->isRela && i->addend != 0) {
    ungroupedNonRelatives.push_back(*i++);
    continue; 
  }


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

https://reviews.llvm.org/D65242