[PATCH] D20793: [ELF] - Implemented support for test/binop relaxations from latest ABI.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/InputSection.cpp:260
@@ -259,1 +259,3 @@
     return Body.getVA<ELFT>(A) - P;
+  case R_RELAX_GOT_PC_NOPIC:
+    return Body.getVA<ELFT>();
----------------
rafael wrote:
> This is the same value as R_ABS, move it there.
Done.

================
Comment at: ELF/Relocations.cpp:247
@@ -246,3 +246,3 @@
   return Expr == R_PC || Expr == R_GOTREL || Expr == R_PAGE_PC ||
-         Expr == R_RELAX_GOT_PC;
+         Expr == R_RELAX_GOT_PC || Expr == R_RELAX_GOT_PC_NOPIC;
 }
----------------
rafael wrote:
> This one is not RelExpr if it has the same value as R_ABS.
Removed.

================
Comment at: ELF/Target.cpp:746
@@ -742,2 +745,3 @@
   // Relax mov.
   if (Op == 0x8b)
+    return R_RELAX_GOT_PC;
----------------
rafael wrote:
> A mov can be relaxed into a "mov $" if we are producing a non pic output. No need to do it now,  but add a fixme about it.
Done.

================
Comment at: ELF/Target.cpp:797
@@ +796,3 @@
+  // 2) http://wiki.osdev.org/X86-64_Instruction_Encoding
+  // 3) http://www.swansontec.com/sintel.html
+  assert(!Config->Pic);
----------------
rafael wrote:
> These are for the encoding? Maybe refer the official intel docs?
Done. 
(Btw we are using (1) in comments about TLS relaxations.)

================
Comment at: ELF/Target.cpp:801
@@ +800,3 @@
+  if (Op == 0x85) {
+    // See 0x85 description in (1). Column "o" has "r" what indicates that the
+    // instruction uses "full" ModR / M byte, (no opcode extension).
----------------
rafael wrote:
> Which instruction is this? testq?
Yes, it is testq.

================
Comment at: test/ELF/gotpc-relax-nopic.s:92
@@ +91,3 @@
+_start:
+  adcl    bar at GOTPCREL(%rip), %eax
+  addl    bar at GOTPCREL(%rip), %ebx
----------------
rafael wrote:
> Not sure there is any value is testing movl in here.  We know we don't produce a relaxable relocation with mc.
Did it to show the difference between lld and other linkers here. Do not have strong opinion about real value, so removed.


http://reviews.llvm.org/D20793