[PATCH] D155818: [CloneFunction][DebugInfo] Clone DISubprogram's local types

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a subscriber: nickdesaulniers.
dblaikie added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CloneFunction.cpp:281-301
+      // If we cloned the type, referenced by DICompileUnit, we must add a
+      // reference on it.
+      SmallVector<Metadata *> TypesToAdd;
+      for (DIScope *S : CU->getRetainedTypes()) {
+        if (DIType *Type = dyn_cast<DIType>(S)) {
+          auto It = VMap.MD().find(Type);
+
----------------
dexonsmith wrote:
> dzhidzhoev wrote:
> > dblaikie wrote:
> > > I guess this comes up, though I'd love it if it doesn't...
> > > 
> > > When does this situation arise? I would've hoped that function-local types wouldn't appear in the CU's retained types list. It'd simplify the model if they were only retained by either the CU or the subprogram, and not both.
> > ```
> > enum my_enum { AA, AB, AC } e;
> > int main() {
> >   enum my_local_enum { A, B, C };
> >   return 0;
> > }
> > ```
> > This example can be compiled with the flag `-fno-eliminate-unused-debug-types` :
> > ```
> > bin/clang local-retained.c -c -fno-eliminate-unused-debug-types -O0 -g -emit-llvm
> > ```
> > That's what metadata looks like:
> > ```
> > !2 = distinct !DICompileUnit(language: DW_LANG_C11, file: !3, retainedTypes: !11 ...
> > !5 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "my_enum", ...
> > !11 = !{!5, !12}
> > !12 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "my_local_enum", scope: !13, ...
> > !13 = distinct !DISubprogram(name: "main", retainedNodes: !17, ...
> > !17 = !{!12}
> > ```
> > 
> > Here `my_local_enum`, which is local to the subprogram, is referenced by both CU and Subprogram, via retainedTypes and retainedNodes correspondingly. I haven't encountered such a situation in real-world metadata since I haven't got much experience dealing with DebugInfo.
> I agree with @dblaikie that local types seem like a property of subprograms.
> 
> To replicate the current IRGen retention policy, we could make `-fno-eliminate-unused-debug-types` retain any subprogram that has a local type (dropping the explicit reference to the local types from compile-unit) and get the same effect.
> 
> But maybe `-fno-eliminate-unused-debug-types` is overreaching here? It's not obvious to me that it implies retaining function-local types. Feels like `-fno-eliminate-unused-debug-subprograms` (or similar) would be a better fit for that behaviour.
> 
> Either way, then we'd end up with a clean model here and this could wouldn't be necessary.
Yeah - @nickdesaulniers do you care about local types for optimized-out functions in `-fno-eliminate-unused-debug-types`? Looks like GCC is maybe slightly inconsistent. For a totally uncalled inline function with a local type, it doesn't generate any DWARF, but if it's called and then optimized away, it still manages to emit a `DW_TAG_inlined_subroutine` (though it seems to do this even without the local type - so the local type isn't what's causing the inlined subroutine description to be emitted)

I think it's fine to, yeah, change `-fno-eliminate-unused-debug-types` to attach local types to the subprogram - and if at some point we improve inlining (I'm not sure it's an improvement to include a `DW_TAG_inlined_subroutine` that describes a zero length range of instructions, though... ) then we'll keep these types too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155818