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

Wed Dec 1 14:31:50 PST 2021

krisb added inline comments.

================
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);
----------------
dblaikie wrote:
> krisb wrote:
> > dblaikie wrote:
> > > 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)
> > > Perhaps this points to some other direction (perhaps in addition) being needed? Perhaps local entities could go on the DISubprogram's retainedNodes attribute?
> > Unless I'm missing something retainedNodes semantic is to keep entities even if they can be optimized out (as of retainedTypes is for types, but only if we have corresponding compile option passed). So, I'm not sure we should use those fields to keep some local entities that are okay to be completely removed, if they unused/ get optimized away.
> > I was thinking about a kind of a new field for DISubprogram (and maybe for DILexicalBlock), something like `localDecls` which would keep references for static locals, local imported entities, and records/typedefs that belongs to this local scope (and stop referencing local entities from `globals` and `imports` fields of a compile unit), but have not come to a comprehensive solution yet.
> > 
> Yep, complicated tradeoffs. By having the list we'd avoid miscoping these entities (because they're otherwise intractible to find - they could appear in any part of the DWARF) - but any attempt to keep them in a list risks them being preserved too much too.
> 
> keeping the list on the DILexicalBlock would be pretty good - that way if the lexical block became unreferenced (all instructions in it were optimized away) the lexical block could be removed and the list along with it - hmm, but there are still cases where the type could be alive despite the function being totally optimized away (eg: a trivial function that returns a stateless local type, like a lambda - so the local type escapes the scope, and the function gets inlined and optimized away - no scope, but the type is still out there in the rest of the DWARF) - in which case we'd still scope the type incorrectly... 
Another problem is even if for every scope there would be a list of types declared within it wasn't guaranteed that these types would be placed in the correct scope. Lexical blocks DIEs are emitted based on LexicalScopes which may (and likely) not be created for optimized out scopes. Thus it still would be possible to misplace a type DIE. But as far as I know, this should not be a problem for debuggers (at least for those that I'm aware of).
We need a more tricky solution (or more changes in DWARF emission design) to make everything come together. 

> because they're otherwise intractible to find - they could appear in any part of the DWARF
I hope they will be at least within a parent subprogram. We may do the things in a more conservative way and in case there is no suitable parent scope DIE emitted for this type (or any other local declaration in the subject) place these entities directly into a subprogram DIE, so we will have 2 possible places for local decls:
- direct scope,
- subprogram scope if the direct scope is missed.

Concerning the case with lambdas, I've checked a few examples and found that clang produces scopeless DIs for such types, and this seems to be one more problem here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113741