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

[Rui Ueyama via llvm-commits]

ruiu 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,
----------------
grimar wrote:
> 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.
Fair. Then please rename relocateTlsOptimize. (Readers don't really want to know how it is going to be optimized in this context, so "ToLe" is too detailed.)

================
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);
----------------
grimar wrote:
> 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.
As long as it can be written simple enough, I'm fine, but currently it looks a bit too cryptic.

There are six instructions this function may emit. Could you explain what each instruction mean?

0x49 0x81 0xc?
0x49 0xc7 0xc?
0x?? 0x81 0xc?
0x?? 0xc7 0xc?
0x4d 0x8d 0x8?
0x?? 0x8d 0x8?


http://reviews.llvm.org/D14713