[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:
> Well, I think that is rather too specific. You set `Demanded` field in Writer.cpp, but that boolean value is consumed immediately in the following line in an obscure way. (You explicitly wrote to the boolean variable but read from it through leaveInOutput() function.)
> 
> Can we write like this in Writer.cpp?
> 
>   bool needsGot = (isOutputDynamic() && Config->EMachine == EM_MIPS);
> 
>   // If we have a relocation that is relative to GOT (such as GOTOFFREL),
>   // we need to emit a GOT even if it's empty.
>   if (HasGotoffRel)
>     needsGot = true;
> 
>   if (needsGotMips || !Out<ELFT>::Got->empty()) {
>     ....
>   }
Done.
(I actually don`t like idea of global variable at all. I would prefer static member of GotSection then. But if you insist..)

================
Comment at: ELF/Target.h:62
@@ -61,2 +61,3 @@
                            uint8_t *PairedLoc = nullptr) const = 0;
+  virtual bool relocUsesGotAddr(uint32_t Type) const;
   virtual bool isTlsOptimized(unsigned Type, const SymbolBody *S) const;
----------------
ruiu wrote:
> I'd name relocNeedsGot.
I`d too, but unfortunately we already have one bool relocNeedsGot(...) above :)
Which is used to determine if relocation needs got entry to be allocated. So I dont think you want to move functionality from relocUsesGotAddr there.
May be relocAddressesGot() ?


http://reviews.llvm.org/D15383