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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1232
 
+void DwarfDebug::createConcreteLabel(DwarfCompileUnit &TheCU,
+                                     LexicalScope &Scope,
----------------
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?)


Repository:
  rL LLVM

https://reviews.llvm.org/D45556