[LLVMdev] [Debug Info PATCH] for support of ref_addr and removal of DIE duplication

[David Blaikie]

On Wed, Oct 16, 2013 at 12:38 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
>
> On Wed, Oct 16, 2013 at 11:54 AM, Manman Ren <manman.ren at gmail.com> wrote:
>
>>
>>
>>
>> On Wed, Oct 16, 2013 at 11:10 AM, David Blaikie <dblaikie at gmail.com>wrote:
>>
>>>
>>>
>>>
>>> On Wed, Oct 16, 2013 at 10:54 AM, Manman Ren <manman.ren at gmail.com>wrote:
>>>
>>>>
>>>>
>>>>
>>>> On Tue, Oct 15, 2013 at 5:59 PM, David Blaikie <dblaikie at gmail.com>wrote:
>>>>
>>>>>
>>>>>   In beginModule, we construct the CUs, but not all the DIEs that
>>>>>>>> belong to the CU.
>>>>>>>> In endFunction, we started to construct the scope DIEs. So in some
>>>>>>>> sense, we are adding things to prior CUs.
>>>>>>>>
>>>>>>>> Looking at void CompileUnit::addDIEEntry(DIE *Die, uint16_t
>>>>>>>> Attribute, DIE *Entry), we can possibly have 3 CUs, this CU,
>>>>>>>> the CU Die belongs to (i.e. the CU the parent chain points to), the
>>>>>>>> CU Entry belongs to.
>>>>>>>>
>>>>>>>
>>>>>>> When does "this CU" differ from both the Die and the Entry?
>>>>>>>
>>>>>>
>>>>>> The signature of addDIEEntry didn't say that "this CU" should be the
>>>>>> same as either Die's CU or Entry's CU.
>>>>>>
>>>>>
>>>>> No, but the code prior to your patch that's the only really sane thing
>>>>> for it to do, right? Why would one CompileUnit be dealing with some other
>>>>> CU's DIEs?
>>>>>
>>>>> This is why we should carefully think about the new semantics we're
>>>>> introducing by doing cross-CU DIE references.
>>>>>
>>>>>
>>>>>>  If you think it should not happen, we should put a comment in so
>>>>>> other developers will not violate it.
>>>>>> But that is irrelevant if we are going to make a single assumption:
>>>>>>   "in addDIEEntry, if a DIE is without a parent, it belongs to the CU
>>>>>> we call addDIEEntry on".
>>>>>>
>>>>>
>>>>> I'm not entirely sure that will be true. In the case of implicit
>>>>> special members, such as the copy ctor, what could happen is that a later
>>>>> CU might want to add a member function to an earlier CU. That member
>>>>> function DIE will be being constructed from within the latter CompileUnit,
>>>>> but its parent will be the former CU. Walking the parent chain will find
>>>>> the correct (older CU) because we build the context for a member function
>>>>> and attach the member function to that context before we build any of its
>>>>> parameters, template arguments, etc.
>>>>>
>>>>> Exactly, so it is possible that a later CU can construct a DIE that
>>>> belongs to another CU.
>>>>
>>>> I was rephrasing the statement in your earlier email and trying to be
>>>> sure what assumption we are going to make.
>>>>   The claim Eric and I are making/discussed is that we should assume
>>>> any DIE without a parent is in the current CU and/or that it possibly
>>>> should never come up (I haven't thought through the codepaths carefully
>>>> here - if we add CUs to their parent before creating their members, I think
>>>> it should never come up).
>>>>
>>>> I know that the assumption may not be true with the current code base,
>>>> but we need to either
>>>> 1> keep the worklist or
>>>> 2> remove the worklist and modify the code base to make the assumption
>>>> true, right?
>>>>
>>>
>>> I'm OK assuming it to be true until we show it to not be true and
>>> treating those cases as bugs and fixing them on an as-found basis.
>>>
>>
>> Okay, I attached a patch removing the worklist. Please review,
>>
>
> Will do.
>

patch looks fairly obvious/trivial

Have you tried any of the test cases I've described (special members,
nested types, and member templates - all used across CUs so an earlier CU
has the type definition and a latter one has one of these extra members)?
I'd like to see test results for these before we progress.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131016/fda059ac/attachment.html>