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

[Hsiangkai Wang via Phabricator via llvm-commits]

HsiangKai added a comment.

About combining DwarfLabel and DbgVariable.



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


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.h:191
+/// Labels are collected from \c DBG_LABEL instructions.
+class DwarfLabel {
+  const DILabel *Label;                 /// Label Descriptor.
----------------
aprantl wrote:
> aprantl wrote:
> > Could this share a common base with DbgVariable?
> Is this feasible?
The only similar data members are DILocation, and DIE *. The only similar member function is getTag(). It seems that there is few benefits to combine these two classes under a common base.

Processing labels is much simpler than variables. I have no idea how to combine these two classes and let the implementation simple and make sense.


Repository:
  rL LLVM

https://reviews.llvm.org/D45556