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

[David Blaikie via llvm-commits]

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.


> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180423/acea87f0/attachment.html>