[PATCH] D14713: [ELF2] - Optimization for R_X86_64_GOTTPOFF relocation.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 18 13:40:49 PST 2015


grimar added inline comments.

================
Comment at: ELF/InputSection.cpp:150
@@ -149,1 +149,3 @@
       continue;
+    } else if (Body.isTLS() && Target->getTlsOptimization(Type, Body)) {
+      Target->relocateTlsOptimize(BufLoc, Buf, BufEnd, Type, AddrLoc,
----------------
ruiu wrote:
> getTlsOptimization -> isTlsOptimized (since it returns a boolean value.)
> 
> Can you move this before
> 
>   uintX_t SymVA = getSymVA<ELFT>(Body);
> 
> ?
Done.

================
Comment at: ELF/InputSection.cpp:151
@@ +150,3 @@
+    } else if (Body.isTLS() && Target->getTlsOptimization(Type, Body)) {
+      Target->relocateTlsOptimize(BufLoc, Buf, BufEnd, Type, AddrLoc,
+                                  SymVA);
----------------
ruiu wrote:
> Currently you are not using BufEnd, so please remove that parameter.
It is used because relocateTlsOptimize() calls relocateOne() which accepts it.
I am not using Type now, so removed it.

================
Comment at: ELF/Target.cpp:354
@@ +353,3 @@
+    return false;
+  return (Type == R_X86_64_GOTTPOFF && !canBePreempted(&S, true));
+}
----------------
ruiu wrote:
> Remove outermost ().
Done.

================
Comment at: ELF/Target.cpp:371-372
@@ +370,4 @@
+                                           uint64_t P, uint64_t SA) const {
+  if (Loc - 3 < BufStart)
+    error("Tls relocation optimization fail, buffer overrun !");
+  uint8_t *Prefix = &Loc[-3];
----------------
ruiu wrote:
> I'm wondering if this is correct. One should never use a R_X86_64_GOTTPOFF relocation with instructions other than MOV or ADD?
As stated in comment above the method, @gottpoff(%rip) must be used in movq or addq instructions only (its written in Ulrich manual), so yes, this relocation optimization only can have mov and add cases and thats definitely correct.
Also just in case gold and bfd has the same logic (but Ulrich is more preferable information source I think).


http://reviews.llvm.org/D14713





More information about the llvm-commits mailing list