[PATCH] D50917: [LLD] [COFF] Support MinGW automatic dllimport of data

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: COFF/Chunks.cpp:428
 
+// Check whether a static relocation of type Type can be deferred and
+// handled at runtime as a pseudo relocation (for references to a module
----------------
ruiu wrote:
> I'd add `MinGW specific.` to all functions that are added in this patch.
Ok, will do, except for methods where MinGW is part of the name where it otherwise is apparent.


================
Comment at: COFF/Chunks.cpp:525
+    Res.emplace_back(
+        RuntimePseudoReloc(Target, this, Rel.VirtualAddress, SizeInBits));
+  }
----------------
ruiu wrote:
> `SizeInBits` is called `Flags` in `RuntimePseudoReloc` class. I was little confused because `Flags` sounds like bit flags. Can you consistently use one name?
This dualism comes from the MinGW sources itself; the field is called `Flags` in the struct, but so far the only flags used is the size of the relocation, but I guess they've left it open to use other higher bits. I'll try to clarify this.


================
Comment at: COFF/Writer.cpp:191
   bool SetNoSEHCharacteristic = false;
+  std::vector<RuntimePseudoReloc> RuntimePseudoRelocs;
 
----------------
ruiu wrote:
> Looks like this variable is used only by one function. Can this be a local variable?
Not really, the `PseudoRelocTableChunk` contains an `ArrayRef` to this, so it needs to be kept alive until the chunks actually are written.


https://reviews.llvm.org/D50917