[PATCH] D122163: [StripDeadDebugInfo] Drop dead CUs

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/StripSymbols.cpp:299
+static void
+collectCUsWithScope(const DIScope *Scope, std::set<DICompileUnit *> &LiveCUs,
+                    SmallPtrSet<const DIScope *, 32> &VisitedScopes) {
----------------
Can you add a doxygen comment explaining what this function does?


================
Comment at: llvm/lib/Transforms/IPO/StripSymbols.cpp:300
+collectCUsWithScope(const DIScope *Scope, std::set<DICompileUnit *> &LiveCUs,
+                    SmallPtrSet<const DIScope *, 32> &VisitedScopes) {
+  if (!Scope)
----------------
This is highly subjective, but to me, 32 seems a bit large for a smallptrset. Did you base that number on any performance measurements?


================
Comment at: llvm/lib/Transforms/IPO/StripSymbols.cpp:338
-        LiveGVs.insert(DIG);
-
       // Make sure we only visit each global variable only once.
----------------
This change seems to be unrelated to the above change, and it might be better to discuss it in a separate review. Is this going to drop //all// constant globals?


================
Comment at: llvm/lib/Transforms/IPO/StripSymbols.cpp:370
+        const DILocation *DILoc = I.getDebugLoc().get();
+        if (DILoc && DILoc->getInlinedAt())
+          collectCUsForInlinedFuncs(DILoc, LiveCUs, VisitedScopes);
----------------
how about sinking the getInlinedAt() into collectCUsForInlinedFuncs() so we don't check for if(Loc) twice?


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

https://reviews.llvm.org/D122163