[PATCH] D55293: [LLD][COFF] Unmerged sections

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 12 15:09:09 PST 2018


rnk added a subscriber: mstorsjo.
rnk added a comment.

> I used a std::map below instead of a DenseHash because we need to retain a sorted order when enumerating.

Yep, I looked at removing this before. It might actually be worth it for mingw, because GCC outputs sections named like ".text$_Z3foov", so you can imagine that this map is going to have one entry per symbol, so one extra memory allocation per symbol.

@mstorsjo, I assume that ld.bfd doesn't actually sort the .text (and .pdata and .xdata) section contributions alphabetically. Should we drop the "$_Z3foov" portion of the object file section name before making the unmerged section? It might matter for performance.



================
Comment at: COFF/Writer.cpp:327
 
+namespace {
+
----------------
LLVM generally tries to keep anon namespaces small and use `static` instead:
https://llvm.org/docs/CodingStandards.html#anonymous-namespaces


================
Comment at: COFF/Writer.cpp:613
   // with alphabetical ordering of the object fils within a library.
-  for (auto &Pair : Map) {
-    StringRef SectionName = Pair.first.first;
-    if (!SectionName.startswith(".idata"))
+  for (auto &NC : UnmergedSections) {
+    UnmergedSection *USec = NC.second;
----------------
What is `NC` supposed to reference here and below?


================
Comment at: COFF/Writer.cpp:695
 
-  SmallDenseMap<std::pair<StringRef, uint32_t>, OutputSection *> Sections;
-  auto CreateSection = [&](StringRef Name, uint32_t OutChars) {
-    OutputSection *&Sec = Sections[{Name, OutChars}];
+  std::map<NameAndChars, OutputSection *> OutSectionsMap;
+  auto CreateOutSection = [&](StringRef Name, uint32_t OutChars) {
----------------
Eh, this is small, since it's only the real output sections of the PE. I guess it doesn't matter if we use std::map for simplicity.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D55293





More information about the llvm-commits mailing list