[PATCH] D113741: [RFC][DwarfDebug][AsmPrinter] Support emitting function-local declaration for a lexical block

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1408
+      }
+      return llvm::make_range(CUs.begin(), CUs.end());
+    }
----------------
krisb wrote:
> dblaikie wrote:
> > Wouldn't this return a dangling reference, since CUs is local to this function?
> Right. Second time 'make_range' makes me thinking it's creating a copy here. My bad. Fixed (might be not the best way & best naming, but should be ok for a patch that, likely, will be abandoned in favor of D125693). 
> patch that, likely, will be abandoned in favor of D125693

Not sure, I think this patch might still be a good direction - without the IR changes necessary for D125693. Does mean local types that end up unreferenced (because their uses are optimized awya) could be eliminated, which is good and bad (it's something we favor for non-local types for size reasons, but arguably we try more to keep all the local names for scoping/shadowing reasons - though it's not a terribly consistent principle)


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h:318
+  /// Maps subprogram to its containing DwarfCompileUnit(s).
+  DenseMap<const DISubprogram *, SmallPtrSet<DwarfCompileUnit *, 2>> SPCUsMap;
+
----------------
Why does this map from one subprogram to multiple CUs? Shouldn't we be only placing local types/static variables in one CU (on the abstract origin SP, which should be singular)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113741