[PATCH] D51078: Reduce the memory footprint of dsymutil. (NFC)

[Frederic Riss via Phabricator via llvm-commits]

friss added inline comments.


================
Comment at: tools/dsymutil/DwarfLinker.cpp:2468-2469
       }
-
-      if (!CUDie || LLVM_UNLIKELY(Options.Update) ||
-          !registerModuleReference(CUDie, *CU, ModuleMap, LinkContext.DMO,
-                                   LinkContext.Ranges, OffsetsStringPool,
-                                   UniquingStringPool, ODRContexts, UnitID)) {
-        LinkContext.CompileUnits.push_back(llvm::make_unique<CompileUnit>(
-            *CU, UnitID++, !Options.NoODR && !Options.Update, ""));
-      }
+      if (auto CUDie = CU->getUnitDIE(false))
+        registerModuleReference(CUDie, *CU, ModuleMap, LinkContext.DMO,
+                                LinkContext.Ranges, OffsetsStringPool,
----------------
You're changing the semantics here. registerModuleReference would not have been called in Update mode and now it will.

Also, why not simply `if (CUDie)`?


================
Comment at: tools/dsymutil/DwarfLinker.cpp:2502
+        // effectively skips over fully resolved skeleton units.
+        bool Quiet = false;
+        auto CUDie = CU->getUnitDIE(false);
----------------
Don't you want `Quiet = true` ? 


================
Comment at: tools/dsymutil/DwarfLinker.cpp:2505
+        if (!CUDie || LLVM_UNLIKELY(Options.Update) ||
+            !registerModuleReference(CUDie, *CU, ModuleMap, LinkContext.DMO,
+                                     LinkContext.Ranges, OffsetsStringPool,
----------------
I would adda comment explaining that registerModuleReference() won't do any work here because the modules have been registered previously. It's just here to filter out module CUs.


https://reviews.llvm.org/D51078