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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers 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);
+
----------------
dblaikie wrote:
> 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.
> do you care about local types for optimized-out functions in -fno-eliminate-unused-debug-types?

I assume the answer is "yes" but let me check with the consumers of this data (in Android in the UK).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155818