[PATCH] D15779: [ELF] - Implemented optimization for R_X86_64_GOTPCREL relocation.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Tue May 24 07:36:23 PDT 2016


grimar added a comment.

In http://reviews.llvm.org/D15779#437390, @rafael wrote:

> I decided to apply the patch to give it a better review.
>
> Some of the issues I pointed out in the review (R_RELAX_GOT referring
>  to got entry) were the causes of other changes (unnecessary continue).
>
> Also, the *GOTPCRELX relocations can always be relaxed if the symbol
>  is not ifunc or preemptable, no? It is just that it is not always a
>  mov->lea.
>
> In any case, I have attached the result of simplifying your patch a
>  bit. Let me know what you think.
>
> Cheers,
> Rafael
>
> - F1976708: t.diff <http://reviews.llvm.org/F1976708>


Rafael, returning to this, I have few thoughts now.

Latest x64 ABI describes 3 possible groups of relaxations atm (https://github.com/hjl-tools/x86-psABI/wiki/X86-psABI):

1. Convert call and jump
2. Convert mov->lea
3. Convert Test and Binop Convert memory operand of test and binop into

immediate operand, where binop is one of adc, add, and, cmp, or,
sbb, sub, xor instructions, when position-independent code is disabled.

If there was no 3, we probably would be able to leave R_RELAX_GOT_PC expression assigning
logic as is and just teach relaxGot() to relax all of them at once. If do that at once, that would help to avoid
the op code checks anywhere except relaxGot() itself.

But 3 says that depending on PIC for a specific set of instruction relaxation is possible or not.

That probably does not leave us a chance for above and solution I see is introduce Target method 
canGotBeRelaxed() or something, which will check the instructions opcodes, like:

  bool canGotBeRelaxed() {
  if (call or jump or move opcodes) 
    return true;
  if (Test or given binop opcodes)
    return !PIC:
  return false; //I guess it is possible to have instructions that can not be relaxed even if RELX relaxation is present ?
  }

and call it from adjustExpr(). What do you think ?


http://reviews.llvm.org/D15779





More information about the llvm-commits mailing list