[PATCH] D54802: [LLD][COFF] Generate import modules in PDB

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 26 18:43:46 PST 2018


rnk added a comment.

Cool. :)



================
Comment at: lld/trunk/COFF/Driver.cpp:1226
   // precedence, but we will emit a warning if there is a conflict.
-  parseMerge(".idata=.rdata");
   parseMerge(".didat=.rdata");
----------------
Do you want to submit this separately? It seems to cause wide ranging mechanical changes to the tests. Feel free to submit it without review.


================
Comment at: lld/trunk/COFF/Writer.cpp:758-760
+    InputSection *ISec =
+        make<InputSection>(Pair.first.first, OutChars, Chunks, Sec);
+    InputSections.push_back(ISec);
----------------
This seems like it's really just replicating `Map` into some longer lived data structure, and then creating a backreference from the output section to the concatenated input section. There are three other helper functions that take a reference to `Map`, and that type is ridiculously long. I think we could clean this code up significantly if we planned some NFC changes first to make `Map` a member of `Writer` and then give it a real key and value type. WDYT? I'm imagining that the type would become: `std::map<NameAndChars, InputSection>`

That would have the side effect of cleaning up a lot of unreadable `Pair.second` and `Pair.first.first` accesses in this code.

Honestly I started looking at this at first just because we copy the `Chunks` vector again, and I was wondering if that could be avoided, especially for a no-PDB build...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54802/new/

https://reviews.llvm.org/D54802





More information about the llvm-commits mailing list