[PATCH] D45556: [DebugInfo] Generate DWARF debug information for labels.

[Hsiangkai Wang via llvm-commits]

On Tue, Apr 24, 2018 at 4:04 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Fri, Apr 20, 2018 at 1:13 AM Hsiangkai Wang via Phabricator
> <reviews at reviews.llvm.org> wrote:
>>
>> HsiangKai added inline comments.
>>
>>
>> ================
>> Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1232
>>
>> +void DwarfDebug::createConcreteLabel(DwarfCompileUnit &TheCU,
>> +                                     LexicalScope &Scope,
>> ----------------
>> dblaikie wrote:
>> > aprantl wrote:
>> > > HsiangKai wrote:
>> > > > aprantl wrote:
>> > > > > This and the next function looks like it is almost the same code
>> > > > > as createConcreteVariable and collectVariableInfo. Could the implementation
>> > > > > be shared (see my comment on DwarfLabel)?
>> > > > I took DILocalVariable as the reference model. So, you will find
>> > > > that some implementation is similar. However, processing of labels is much
>> > > > simpler than processing of variables. I do not need to handle location list
>> > > > for variables, instruction ranges that variables are accessible, local
>> > > > variables v.s. argument variables, and data types, etc.
>> > > >
>> > > > The only similarity is they are both local entities in a function.
>> > > > They have function scope. They have DILocation to tell users where they are.
>> > > > I have no idea how to combine these two classes and make label processing
>> > > > simple and make sense.
>> > > + @dblaikie: Do you also agree that it is better to keep labels and
>> > > variables separate or should we make an effort to share bits of the
>> > > implementation?
>> > Yeah, it certainly looks like many of these functions are similar/the
>> > same - though the label things talk about "inlined" instead of "abstract" -
>> > is there a reason for that? I would've thought it'd be exactly the same.
>> >
>> > As for commoning the code - I would guess, in general, all the abstract
>> > handling should be basically the same (with a switch/if-chain on "how to
>> > populate abstract attributes for these entities" - variables would have
>> > name, type, location, and labels would just have name and location?)
>>
>>
>>
>> There will be 'abstract labels' in inlined functions.
>
>
> Not sure I understand - inside inlined functions you would find inlined
> labels, not abstract labels. In the abstract origin you would find abstract
> labels.

Your description is more precise. Now I know why 'abstract' is more appropriate
than 'inlined' when naming these variables. I will change these variable names.

>
>>
>> So, I use the specific name, 'inlined label' for it. It is fine to follow
>> the naming of variables.
>>
>> If we want to combine the processing of abstract variables and labels, we
>> need to review all processing for abstract variables. Not only create a
>> common base class for DbgVariable and DwarfLabel, but all processing of
>> abstract variables need to be reviewed. I need some time to refactor all of
>> the related data structures and functions.
>>
>>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D45556
>>
>>
>>
>