[PATCH] D15383: [ELF] - R_386_GOTOFF relocation implemented.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/OutputSections.h:140
@@ -139,2 +139,3 @@
 
+  bool Demanded = false;
 private:
----------------
ruiu wrote:
> Maybe we should have a separate global variable, HasGotoffRel, instead of this field.
I think HasGotOffRel is too specific, at least it can be used for mips as shown in this patch:

```
  Out<ELFT>::Got->Demanded |=
      (isOutputDynamic() && Config->EMachine == EM_MIPS);
```
I can make it separate static but what the point for it ? It only should be used to keep got section now, but when separate static it looks like it can be used for something else, what is not true.

================
Comment at: ELF/Target.cpp:328-334
@@ -327,2 +327,9 @@
 bool X86TargetInfo::relocNeedsGot(uint32_t Type, const SymbolBody &S) const {
+  // This relocation does not require got entry,
+  // but it is relative to got and needs it to be created.
+  // Here we request for that.
+  if (Type == R_386_GOTOFF) {
+    Out<ELF32LE>::Got->Demanded = true;
+    return false;
+  }
   if (S.isTLS() && Type == R_386_TLS_GD)
----------------
ruiu wrote:
> I don't think this is the right place to write this code. This function is doing more than its name implies.
Yes, I didn`t like that place too, but then only suitable place for me would be adding Target->relocUsesGotAddr() and call it during iterations over relocations at upper level.
That adds one more method to Target thats only useful only for x86 now. Made that change.


http://reviews.llvm.org/D15383