[DebugInfo] Generate DWARF debug information for labels.
Needs ReviewPublic

Authored by HsiangKai on Thu, Apr 12, 1:15 AM.

Details

Summary

There are two forms for label debug information in DWARF format.

  1. Labels in a non-inlined function:

DW_TAG_label

DW_AT_name
DW_AT_decl_file
DW_AT_decl_line
DW_AT_low_pc
  1. Labels in an inlined function:

DW_TAG_label

DW_AT_abstract_origin
DW_AT_low_pc

We will collect label information from DBG_LABEL. Before every DBG_LABEL,
we will generate a temporary symbol to denote the location of the label.
The symbol could be used to get DW_AT_low_pc afterwards. So, we create a
mapping between 'inlined label' and DBG_LABEL MachineInstr in DebugHandlerBase.
The DBG_LABEL in the mapping is used to query the symbol before it.

The AbstractLabels in DwarfCompileUnit is used to process labels in inlined
functions.

We also keep a mapping between scope and labels in DwarfFile to help to
generate correct tree structure of DIEs.

Diff Detail

Repository
rL LLVM
HsiangKai created this revision.Thu, Apr 12, 1:15 AM
aprantl added inline comments.Thu, Apr 12, 8:47 AM
lib/CodeGen/AsmPrinter/DebugHandlerBase.h
53

Often it's better for readability to just define a struct with named members so we can access .label and .loc instead of .first and .second.

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
490

Please put the doxygen comment on the declaration and don't repeat the function name.

lib/CodeGen/AsmPrinter/DwarfDebug.cpp
1233

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)?

lib/CodeGen/AsmPrinter/DwarfDebug.h
191

Could this share a common base with DbgVariable?

198

///

202
/// @{
/// Accessors.
208

/// @}

210

///

HsiangKai updated this revision to Diff 142532.Sat, Apr 14, 7:00 PM

Update according to reviewers' comments.
Update test cases.

HsiangKai added inline comments.Mon, Apr 16, 7:40 AM
lib/CodeGen/AsmPrinter/DebugHandlerBase.h
53

If I group DILabel and DILocation in a struct, I need to add isEqual(), getHashValue(), getEmptyKey(), etc to conform the interface of MapVector. I think it is too complex for the purpose. std::pair is ready to use as the 'Key Type' of MapVector.

Use MapVector is easier to establish the mapping between InlinedLabel and MachineInstr.

HsiangKai marked 5 inline comments as done.Mon, Apr 16, 8:03 AM
aprantl added inline comments.Mon, Apr 16, 9:14 AM
lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
170

This comment should go to the declaration.

lib/CodeGen/AsmPrinter/DebugHandlerBase.h
53

Fair point. One trick I've used in the past for situations like this is to define a struct that inherits from std::pair<> and defines a getLabel() and a getLocation() accessor function. Feel free to adopt this pattern if you think that it improves readability.

lib/CodeGen/AsmPrinter/DwarfDebug.h
191

Is this feasible?

About combining DwarfLabel and DbgVariable.

lib/CodeGen/AsmPrinter/DwarfDebug.cpp
1233

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.

lib/CodeGen/AsmPrinter/DwarfDebug.h
191

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.

aprantl added inline comments.
lib/CodeGen/AsmPrinter/DwarfDebug.cpp
1233

+ @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?

dblaikie added inline comments.Thu, Apr 19, 3:10 PM
lib/CodeGen/AsmPrinter/DwarfDebug.cpp
1233

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?)

HsiangKai added inline comments.Fri, Apr 20, 1:13 AM
lib/CodeGen/AsmPrinter/DwarfDebug.cpp
1233

There will be 'abstract labels' in inlined functions. 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.