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

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/InputSection.cpp:150-154
@@ -149,3 +149,7 @@
       continue;
+    } else if (Body.isTLS() &&
+               Target->getTlsOptimization(Type, Body) == TargetInfo::ToLE) {
+      Target->relocateTlsToLe(BufLoc, BufEnd, Type, AddrLoc, SymVA);
+      continue;
     }
     Target->relocateOne(BufLoc, BufEnd, Type, AddrLoc,
----------------
ruiu wrote:
> Can you move this above line 137 just like "if (Target->isTlsGlobalDynamicReloc(Type)) { ... }"?
> 
> Also, do you think you can merge getTlsOptimization and relocateTlsToLe into one function? I'd probably write like this.
> 
>   // Some TLS relocations are always compiled to use GOT, but the linker
>   // is sometimes able to rewrite it so that it doesn't use GOT. This may not
>   // only apply relocations but also modify preceding instructions.
>   if (applyOptimizeTls(BufLoc, Type, AddrLoc, Body))
>     continue;
About merging into one.
getTlsOptimization() is also now used also in another place, in bool X86_64TargetInfo::relocNeedsGot().
So such merging probably not possible.

================
Comment at: ELF/Target.cpp:362-368
@@ +361,9 @@
+  uint8_t Reg = Loc[-1] >> 3;
+  // Originally it can be one of two:
+  // 1) movq foo at gottpoff(%rip), %reg
+  // We change it into one of:
+  // movq $foo, %reg
+  // addq $foo, %rsp (addressing with %rsp is special).
+  // 2) addq foo at gottpoff(%rip), %reg
+  // We change it into leaq foo(%reg), %reg.
+  bool RspAdd = (Ins != 0x8b && Reg == 4);
----------------
ruiu wrote:
> grimar wrote:
> > I dont sure what to do here. How to check out of bound for negative idx. Should I check (will need BufBegin arg I guess) or we can assume its never be out ?
> Is that actually a possible scenario that one wants to add a TLS value to SP register? Do you have to take care of that?
I did not analyse how much people do that :) 
gold and bfd do the same. And it is still possible scenario even if chance is low. I jsut want to be on a safe side with this single line.

================
Comment at: ELF/Target.h:61
@@ -59,2 +60,3 @@
                            uint64_t P, uint64_t SA) const = 0;
-
+  virtual TlsOpt getTlsOptimization(unsigned Type, const SymbolBody &S) const;
+  virtual void relocateTlsToLe(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
----------------
ruiu wrote:
> Currently it only returns None or ToLE, so can you change the signature of the function so that it returns a boolean value?
Will do.


http://reviews.llvm.org/D14713