[PATCH] D113736: [DebugInfo][Try 2] Only create concrete DIEs of concrete functions

Mon Nov 29 16:36:53 PST 2021

dblaikie added a comment.

In D113736#3159993 <https://reviews.llvm.org/D113736#3159993>, @ellis wrote:

> In D113736#3159992 <https://reviews.llvm.org/D113736#3159992>, @dblaikie wrote:
>
>> In D113736#3130872 <https://reviews.llvm.org/D113736#3130872>, @krisb wrote:
>>
>>> @ellis, @dblaikie
>>>
>>>> can we have some design discussion of these alternatives in one place (here, or an llvm-dev thread) to try to avoid redundant/divergent design discussions across multiple mutually exclusive patches/directions to address these issues?
>>>
>>> That probably would have been done in the first place. But I was going to base on ellis's work for inlined SPs while working on lexical blocks scopes for static locals and other until realized that ellis's patch doesn't fix the issue for SPs with concrete out-of-line instances (which I'm looking to get fixed as well). 
>>> The problem is if we have a concrete out-of-line instance, static locals and local type declarations will be children of this concrete scope, which makes them not accessible from inlined SPs.
>>> To solve this problem we need to know if the given SP has (or will have) abstract origin or not before we will try to emit any local declarations. Thus we need either (or both) to collect this information before processing local entities or (and) to postpone creating local entities until we have all SPs get emitted. The latter one seems the most safe and reliable option, except for the fact that it completely changes the order of emitted entities (but it's still possible to maintain some specific order, if the ordering is an issue).
>>
>> Hmm, I'm still a bit lost with regards to which patches fix what, whether they overlap, are dependent, or are independent.
>>
>> For instance, does D113741 <https://reviews.llvm.org/D113741> subsume this patch (D113736 <https://reviews.llvm.org/D113736>)? (ie: if we take D113741 <https://reviews.llvm.org/D113741> do we still/not need D113736 <https://reviews.llvm.org/D113736>?)
>
> D113741 <https://reviews.llvm.org/D113741> includes the test cases for this patch so if we take D113741 <https://reviews.llvm.org/D113741> (and I think we should) then I will drop this patch.

OK, I'll set this aside for now - perhaps you can mark it as "abandoned" (or wait until D113741 <https://reviews.llvm.org/D113741> is committed before doing that)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113736