<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Apr 20, 2018 at 1:13 AM Hsiangkai Wang via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">HsiangKai added inline comments.<br>
<br>
<br>
================<br>
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1232<br>
<br>
+void DwarfDebug::createConcreteLabel(DwarfCompileUnit &TheCU,<br>
+                                     LexicalScope &Scope,<br>
----------------<br>
dblaikie wrote:<br>
> aprantl wrote:<br>
> > HsiangKai wrote:<br>
> > > aprantl wrote:<br>
> > > > 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)?<br>
> > > 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.<br>
> > > <br>
> > > 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.<br>
> > + @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?<br>
> 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.<br>
> <br>
> 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?) </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
There will be 'abstract labels' in inlined functions. </blockquote><div><br>Not sure I understand - inside inlined functions you would find inlined labels, not abstract labels. In the abstract origin you would find abstract labels.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">So, I use the specific name, 'inlined label' for it. It is fine to follow the naming of variables.<br>
<br>
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.<br>
<br>
<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D45556" rel="noreferrer" target="_blank">https://reviews.llvm.org/D45556</a><br>
<br>
<br>
<br>
</blockquote></div></div>