[PATCH] D38513: [LLD] [COFF] Add support for GNU binutils import libraries

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D38513#1239919, @ruiu wrote:

> Wow, this patch is almost one year old.


Yup, I put it on hold last year as it seemed complex, and I had more important things to focus on first. Now after reading a bit of binutils source, I think I understand how it's supposed to be handled (and my previous attempt didn't seem to be too far off). And after looking at the code a second time (with more experience with LLD), I managed to clean it up more so it's very lean and pretty nonintrusive now, and no longer with an RFC tag, but this should be good to go now.



================
Comment at: COFF/Writer.cpp:563
 
+// Sort concrete section chunks from GNU import libraries.
+static bool fixGnuImportChunks(
----------------
ruiu wrote:
> Perhaps this is a good place to explain what is GNU import libraries? We shouldn't assume that code readers know what that is when reading this code.
Ok, I'll expand the comment.


================
Comment at: COFF/Writer.cpp:820
 
-  if (!Idata.empty())
-    for (Chunk *C : Idata.getChunks())
-      IdataSec->addChunk(C);
+    if (File->ThunkSym) {
+      if (!isa<DefinedImportThunk>(File->ThunkSym))
----------------
ruiu wrote:
> Flip the condition and continue early.
Ok, will do.


https://reviews.llvm.org/D38513