[PATCH] D20622: [ELF] - Added support for jmp/call relaxations when R_X86_64_GOTPCRELX/R_X86_64_REX_GOTPCRELX are used.

[George Rimar via llvm-commits]

grimar added a comment.

In http://reviews.llvm.org/D20622#439251, @rafael wrote:

> >   - ELF/Target.cpp +++ ELF/Target.cpp @@ -740,14 +740,39 @@ uint64_t Offset) const { if (Type != R_X86_64_GOTPCRELX && Type != R_X86_64_REX_GOTPCRELX) return false; -
>
> > - // Converting mov foo at GOTPCREL(%rip), %reg to lea foo(%rip), %reg
>
> > - // is the only supported relaxation for now.
>
> > - return (Offset >= 2 && Data[Offset - 2] == 0x8b);
>
>
> This deletes the "Offset >= 2" check. Do you know if it will be needed
>  once all optimizations are implemented? If it is there just to guard
>  against corrupted inputs, for now just remove the Offset argument and
>  pass Data+Offset to this function. That can be another patch.


I think there should be no way to have such relocations and Offset < 2.
All instructions that can be relaxed seems to be save to check without this.
I guess that gold/bfd did that just to protect against incorrect inputs. I do not think this 
is really needed.

> 

> 

> > +  if (Op == 0x8b) {

> 

> >  +    // Convert mov foo at GOTPCREL(%rip), %reg to lea foo(%rip), %reg.

> 

> >  +    *(Loc - 2) = 0x8d;

> 

> 

> Use an early return here.

> 

> > +  } else if (Op == 0xff) {

> 

> >  +    if (ModRm == 0x15) {

> 

> >  +      // ABI says we can convert call *foo at GOTPCREL(%rip) to nop call foo.

> 

> >  +      // Instead we convert to addr32 call foo, where addr32 is instruction

> 

> >  +      // prefix. That makes result expression to be a single instruction.

> 

> 

> Interesting idea. For tls data16 and rex64 are used. Any idea which

>  one is better when? Would you mind sending hjl.tools at gmail.com this

>  suggestion for addition in the psabi?


Unfortunately it is not mine idea. It is what I did not understood at first from gnu ld output,
but after some research about what it is doing, I think I got the idea right.

> 

> 

> > +      *(Loc - 2) = 0x67; // addr32 prefix

> 

> >  +      *(Loc - 1) = 0xe8; // call

> 

> 

> early return.

> 

> > +    } else {

> 

> >  +      // ModRm == 0x25.

> 

> >  +      // Convert jmp *foo at GOTPCREL(%rip) to jmp foo nop.

> 

> 

> Can't you use a prefix in here?


I did not investigate that yet. I guess there might be some trouble with incompatibility
of prefixes with some instructions, but that is just a quess. bfd do the same here and
I didn't have chance to dig here. So they use prefix for call and does not do that for jmp.
gold does not relax jmp/call at all it seems.

> > +      *(Loc - 2) = 0xe9; // jmp

> 

> >  +      *(Loc + 3) = 0x90; // nop

> 

> >  +      Loc -= 1;

> 

> >  +      Val += 1;

> 

> >  +    }

> 

> >  +  }

> 

> >  +

> 

> > 

> 

> >     relocateOne(Loc, R_X86_64_PC32, Val);

> 

> >   }

> 

> 

> Cheers,

> Rafael



http://reviews.llvm.org/D20622