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

[Ellis Hoag via Phabricator via llvm-commits]

ellis added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1607
+DIE *DwarfCompileUnit::findLocalScopeDIE(const DIScope *S) {
+  if (!S || !isa<DILocalScope>(S))
+    return nullptr;
----------------



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h:66
+  /// LocalScopeToDieMap - A map of all local scopes for this unit.
+  DenseMap<const DIScope *, DIE *> LocalScopeToDieMap;
 
----------------
Does this need to map from `DIScope` (rather than `DILocalScope`) because it could take a `DICommonBlock`? If so maybe we should rename this map to remove the "Local".


Or could we possibly make this a map from `DISubprogram`?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1077
+    if (isa<DICommonBlock>(S))
+      S = S->getScope();
+    if (const auto *LScope = dyn_cast_or_null<DILocalScope>(S))
----------------
Can a `DICommonBlock` have a scope that is also a `DICommonBlock`?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1213-1218
+  // Collect functions with inlined subprograms.
+  for (const auto &F : M->functions())
+    for (const BasicBlock &BB : F)
+      for (const Instruction &I : BB)
+        if (auto DbgLoc = I.getDebugLoc())
+          recordInlinedSubprogram(DbgLoc);
----------------
This is really great. Previously in https://reviews.llvm.org/D113736 I recorded concrete functions, but that is limited because we aren't sure if there will be an abstract origin. Here we record exactly which subprograms have abstract origins. Also, thanks for including my tests from that diff!


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h:855
+
+  void recordInlinedSubprogram(const DILocation *DbgLoc) {
+    if (!DbgLoc->getInlinedAt())
----------------
NIT: Since this is recursive we could record multiple SPs from a single Loc.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h:859
+    InlinedSPs.insert(cast<DILocalScope>(DbgLoc->getScope())->getSubprogram());
+    return recordInlinedSubprogram(DbgLoc->getInlinedAt());
+  }
----------------



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h:866
+  /// LexicalScope will be created.
+  bool HasInlinedInstances(const DISubprogram *SP) const {
+    return InlinedSPs.count(SP);
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113741