This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by grimar on May 25 2016, 8:01 AM.

Details

Summary

D15779 introduced basic approach to support new relaxations.
This patch implements relaxations for jmp and call instructions,
described in System V Application Binary Interface AMD64 Architecture Processor
Supplement Draft Version 0.99.8 (https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-r249.pdf,
B.2 "B.2 Optimize GOTPCRELX Relocations")

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 58423.May 25 2016, 8:01 AM
grimar retitled this revision from to [ELF] - Added support for jmp/call relaxations when R_X86_64_GOTPCRELX/R_X86_64_REX_GOTPCRELX are used..
grimar updated this object.
grimar added reviewers: rafael, ruiu.
grimar added subscribers: llvm-commits, grimar.
rafael edited edge metadata.May 25 2016, 8:37 AM
rafael added a subscriber: rafael.
  • 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@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.

+ if (Op == 0x8b) {
+ // Convert mov foo@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@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@gmail.com this
suggestion for addition in the psabi?

+ *(Loc - 2) = 0x67; addr32 prefix
+ *(Loc - 1) = 0xe8;
call

early return.

+ } else {
+ ModRm == 0x25.
+
Convert jmp *foo@GOTPCREL(%rip) to jmp foo nop.

Can't you use a prefix in here?

+ *(Loc - 2) = 0xe9; jmp
+ *(Loc + 3) = 0x90;
nop
+ Loc -= 1;
+ Val += 1;
+ }
+ }
+

relocateOne(Loc, R_X86_64_PC32, Val);

}

Cheers,
Rafael

  • 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@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@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@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@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@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

Can't you use a prefix in here?

I`ll try to investigate this tomorrow.

George.

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

Can't you use a prefix in here?

Ah, I think I know ! ABI says:
jmp *foo@GOTPCREL(%rip) can be relaxed to jmp foo nop

For call it gives 2 ways:
call foo nop
nop call foo

But for jump only one way allowed . Therefore we can't switch nop to something else,
like adding instruction prefix without violation of ABI.

Cheers,
Rafael

grimar updated this revision to Diff 58442.May 25 2016, 9:42 AM
grimar edited edge metadata.
  • Addressed review comments.
rafael accepted this revision.May 25 2016, 9:50 AM
rafael edited edge metadata.

LGTM with nit.

ELF/Target.cpp
764 ↗(On Diff #58442)

Make this an assert, since we only got here if canRelaxGot returned true.

774 ↗(On Diff #58442)

Replace comment with assert.

This revision is now accepted and ready to land.May 25 2016, 9:50 AM
This revision was automatically updated to reflect the committed changes.
grimar marked 2 inline comments as done.

LGTM with nit.

Thanks for so fast reviews of this !