[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D80369#2050962 <https://reviews.llvm.org/D80369#2050962>, @djtodoro wrote:

> In D80369#2050022 <https://reviews.llvm.org/D80369#2050022>, @dblaikie wrote:
>
> > In D80369#2048932 <https://reviews.llvm.org/D80369#2048932>, @djtodoro wrote:
> >
> > > Still have test failing:
> > >
> > >   Clang :: Modules/DebugInfoTransitiveImport.m
> > >   Clang :: Modules/ModuleDebugInfo.cpp
> > >   Clang :: Modules/ModuleDebugInfo.m
> > >   
> > >
> > > I haven't looked into the failures, but I guess something Objective-C++ related should be handled with some additional work here.
> >
> >
> > These look like over-constrained/accidental tests. At least the ModuleDEbugInfo.m test - the type being tested for won't be emitted, because the type itself isn't in the retained types list nor is it reachable from any other metadata that is going to be emitted.
> >
> > I think it'd probably be good to clean these tests up - and leave it to @aprantl to look into if this has exposed other bugs - but the bugs are already there, and this IR/debug info metadata isn't working around the bugs, since the IR is going unused anyway.
>
>
> Looks like ObjC expects the declarations within retained types  (based on some tests failures), so I left it as is (I think @aprantl can help us with that).
>  Therefore, this patch will remove the declarations only in the case of call site debug info for now.


Yeah, that's what I was trying to say - I think the tests expect that, but I don't see any code path that uses that debug info metadata - I think the test is overconstrained/accidentally testing for this behavior.

I think the right thing to do is to change these tests to test the new behavior (the behavior of the first version of this patch) - and if some use arises/someone can demonstrate a codepath that's using this data, it can be added back in.

@aprantl can you check here? I've attached two IR files for the ModuleDebugInfo.m test, before/after ('x' is before), stripped of the metadata numbers to make diffing easier - but I think you can still follow what's connected to what with reasonable guesswork. The point is these two function declarations end up in the retainedTypes list, and since the function declarations are emitted, so are the types used in their parameters, etc - but those types aren't reachable from anywhere else in the debug info metadata, so they won't be emitted into the final object file so far as I can see (because nothing looks at the function declarations in retainedTypes - only types).

F11980002: x.ll <https://reviews.llvm.org/F11980002>

F11980001: y.ll <https://reviews.llvm.org/F11980001>


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

https://reviews.llvm.org/D80369