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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Generally on board with this direction - perhaps it doesn't make anything /worse/ (if the cases where types are not reachable through the CU's retainedTypes list get the same behavior they've always got - can you confirm that's the case? If that's the case, perhaps we can go forward with this, and separately/in addition, if you have time/interest/motivnation, you could work on adding local entities to the DISubprogram's retainedNodes list and search that to ensure those entities are put in the right scopes)



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1094-1097
+  for (auto *Ty : CUNode->getRetainedTypes())
+    if (DIType *RT = dyn_cast<DIType>(Ty))
+      if (auto *LScope = getLocalScope(RT->getScope()))
+        CU.recordLocalScopeWithDecls(LScope);
----------------
This is fairly problematic to the strategy - generally we don't put types in the "retained types" list because the point is to lazily reference types/other content as much as possible (so if the middle end optimizes away a function and that's the only place where the type is referenced, then the type gets optimized away too).

Perhaps this points to some other direction (perhaps in addition) being needed? Perhaps local entities could go on the `DISubprogram`'s `retainedNodes` attribute?

(this would address a test failure/FIXME you mentioned in one of the test cases - that a local type was missing from the intended scope)


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDie.cpp:675-676
       return;
+    if (D.getTag() == DW_TAG_lexical_block)
+      return;
     D = D.resolveTypeUnitReference();
----------------
This change could probably be committed separately with a llvm-dwarfdump+assembly test case demonstrating this fix. Then the LLVM code generation changes that depend on this would come in the second patch. Not a huge deal, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113741