[PATCH] D15494: [ELF] - implemented @indntpoff (x86) relocation and its optimization.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Target.cpp:106
@@ -103,3 +105,3 @@
                          uint64_t SA) const;
-  void relocateTlsIeToLe(uint8_t *Loc, uint8_t *BufEnd, uint64_t P,
-                         uint64_t SA) const;
+  void relocateTlsIeToLe(uint32_t Type, uint8_t *Loc, uint8_t *BufEnd,
+                         uint64_t P, uint64_t SA) const;
----------------
In other places we use `unsigned` for the symbol type.

================
Comment at: ELF/Target.cpp:351-354
@@ -339,4 +350,6 @@
     return Target->isTlsOptimized(Type, &S) && canBePreempted(&S, true);
   if (Type == R_386_TLS_GOTIE)
     return !isTlsOptimized(Type, &S);
+  if (Type == R_386_TLS_IE)
+    return !isTlsOptimized(Type, &S);
   return Type == R_386_GOT32 || relocNeedsPlt(Type, S);
----------------
You can combine these two `if`s using ||.

================
Comment at: ELF/Target.cpp:519-520
@@ +518,4 @@
+    // For R_386_TLS_IE relocation we perform the next transformations:
+    // MOVL foo at INDNTPOFF,%EAX is transformed to MOVL $foo,%EAX
+    // MOVL foo at INDNTPOFF,%REG is transformed to MOVL $foo,%REG
+    // ADDL foo at INDNTPOFF,%REG is transformed to ADDL $foo,%REG
----------------
Isn't the former a special case of the latter? (EAX is a REG?)

================
Comment at: ELF/Target.h:48
@@ -46,1 +47,3 @@
   virtual unsigned getPltRefReloc(unsigned Type) const;
+  virtual unsigned getTlsGotReloc(unsigned Type = -1) const {
+    return TlsGotReloc;
----------------
Can you make the argument mandatory? Technically nothing wrong with the optional parameter, but that defines actually a shorthand for defining two different functions, getTlsGotReloc() and getTlsGotReloc(unsigned), thus it (slightly) increase program complexity.


http://reviews.llvm.org/D15494