[NFC][AsmPrinter] Refactor constructVariableDIE #66435
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fold constructVariableDIEImpl into constructVariableDIE, simplify it and group related functions. Pull out the previously inline lambdas for visiting the active variant of the DbgVariable to add location and related attributes as an overload set for a private method applyConcreteDbgVariableAttributes. Rename applyVariableAttribute to reflect what kinds of attributes it applies, and to contrast it with the new applyConcreteDbgVariableAttributes. Move constructLabelDIE down in the implementation file, so all of the constructVariableDIE-related function impls are adjacent.
|
@llvm/pr-subscribers-debuginfo ChangesFold constructVariableDIEImpl into constructVariableDIE, simplify it and group related functions.Pull out the previously inline lambdas for visiting the active variant of the DbgVariable to add location and related attributes as an overload set for a private method Rename applyVariableAttribute to reflect what kinds of attributes it applies, and to contrast it with the new Move constructLabelDIE down in the implementation file, so all of the constructVariableDIE-related function impls are adjacent.Patch is 26.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66435.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index e37dd14c96f76b3..57531e3660c039c 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -740,232 +740,213 @@ DIE *DwarfCompileUnit::constructLexicalScopeDIE(LexicalScope *Scope) {
return ScopeDIE;
}
-/// constructVariableDIE - Construct a DIE for the given DbgVariable.
DIE *DwarfCompileUnit::constructVariableDIE(DbgVariable &DV, bool Abstract) {
- auto D = constructVariableDIEImpl(DV, Abstract);
- DV.setDIE(*D);
- return D;
+ auto *VariableDie = DIE::get(DIEValueAllocator, DV.getTag());
+ insertDIE(DV.getVariable(), VariableDie);
+ DV.setDIE(*VariableDie);
+ // Abstract variables don't get common attributes later, so apply them now.
+ if (Abstract) {
+ applyCommonDbgVariableAttributes(DV, *VariableDie);
+ } else {
+ std::visit(
+ [&](const auto &V) {
+ applyConcreteDbgVariableAttributes(V, DV, VariableDie);
+ },
+ DV.asVariant());
+ }
+ return VariableDie;
}
-DIE *DwarfCompileUnit::constructLabelDIE(DbgLabel &DL,
- const LexicalScope &Scope) {
- auto LabelDie = DIE::get(DIEValueAllocator, DL.getTag());
- insertDIE(DL.getLabel(), LabelDie);
- DL.setDIE(*LabelDie);
+void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
+ const Loc::Single &Single, const DbgVariable &DV, DIE *VariableDie) {
+ const DbgValueLoc *DVal = &Single.getValueLoc();
+ if (!DVal->isVariadic()) {
+ const DbgValueLocEntry *Entry = DVal->getLocEntries().begin();
+ if (Entry->isLocation()) {
+ addVariableAddress(DV, *VariableDie, Entry->getLoc());
+ } else if (Entry->isInt()) {
+ auto *Expr = Single.getExpr();
+ if (Expr && Expr->getNumElements()) {
+ DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+ DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+ // If there is an expression, emit raw unsigned bytes.
+ DwarfExpr.addFragmentOffset(Expr);
+ DwarfExpr.addUnsignedConstant(Entry->getInt());
+ DwarfExpr.addExpression(Expr);
+ addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+ if (DwarfExpr.TagOffset)
+ addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
+ dwarf::DW_FORM_data1, *DwarfExpr.TagOffset);
+ } else
+ addConstantValue(*VariableDie, Entry->getInt(), DV.getType());
+ } else if (Entry->isConstantFP()) {
+ addConstantFPValue(*VariableDie, Entry->getConstantFP());
+ } else if (Entry->isConstantInt()) {
+ addConstantValue(*VariableDie, Entry->getConstantInt(), DV.getType());
+ } else if (Entry->isTargetIndexLocation()) {
+ DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+ DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+ const DIBasicType *BT = dyn_cast<DIBasicType>(
+ static_cast<const Metadata *>(DV.getVariable()->getType()));
+ DwarfDebug::emitDebugLocValue(*Asm, BT, *DVal, DwarfExpr);
+ addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+ }
+ return;
+ }
+ // If any of the location entries are registers with the value 0,
+ // then the location is undefined.
+ if (any_of(DVal->getLocEntries(), [](const DbgValueLocEntry &Entry) {
+ return Entry.isLocation() && !Entry.getLoc().getReg();
+ }))
+ return;
+ const DIExpression *Expr = Single.getExpr();
+ assert(Expr && "Variadic Debug Value must have an Expression.");
+ DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+ DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+ DwarfExpr.addFragmentOffset(Expr);
+ DIExpressionCursor Cursor(Expr);
+ const TargetRegisterInfo &TRI = *Asm->MF->getSubtarget().getRegisterInfo();
- if (Scope.isAbstractScope())
- applyLabelAttributes(DL, *LabelDie);
+ auto AddEntry = [&](const DbgValueLocEntry &Entry,
+ DIExpressionCursor &Cursor) {
+ if (Entry.isLocation()) {
+ if (!DwarfExpr.addMachineRegExpression(TRI, Cursor,
+ Entry.getLoc().getReg()))
+ return false;
+ } else if (Entry.isInt()) {
+ // If there is an expression, emit raw unsigned bytes.
+ DwarfExpr.addUnsignedConstant(Entry.getInt());
+ } else if (Entry.isConstantFP()) {
+ // DwarfExpression does not support arguments wider than 64 bits
+ // (see PR52584).
+ // TODO: Consider chunking expressions containing overly wide
+ // arguments into separate pointer-sized fragment expressions.
+ APInt RawBytes = Entry.getConstantFP()->getValueAPF().bitcastToAPInt();
+ if (RawBytes.getBitWidth() > 64)
+ return false;
+ DwarfExpr.addUnsignedConstant(RawBytes.getZExtValue());
+ } else if (Entry.isConstantInt()) {
+ APInt RawBytes = Entry.getConstantInt()->getValue();
+ if (RawBytes.getBitWidth() > 64)
+ return false;
+ DwarfExpr.addUnsignedConstant(RawBytes.getZExtValue());
+ } else if (Entry.isTargetIndexLocation()) {
+ TargetIndexLocation Loc = Entry.getTargetIndexLocation();
+ // TODO TargetIndexLocation is a target-independent. Currently
+ // only the WebAssembly-specific encoding is supported.
+ assert(Asm->TM.getTargetTriple().isWasm());
+ DwarfExpr.addWasmLocation(Loc.Index, static_cast<uint64_t>(Loc.Offset));
+ } else {
+ llvm_unreachable("Unsupported Entry type.");
+ }
+ return true;
+ };
- return LabelDie;
+ if (!DwarfExpr.addExpression(
+ std::move(Cursor),
+ [&](unsigned Idx, DIExpressionCursor &Cursor) -> bool {
+ return AddEntry(DVal->getLocEntries()[Idx], Cursor);
+ }))
+ return;
+
+ // Now attach the location information to the DIE.
+ addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+ if (DwarfExpr.TagOffset)
+ addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
+ *DwarfExpr.TagOffset);
}
-DIE *DwarfCompileUnit::constructVariableDIEImpl(const DbgVariable &DV,
- bool Abstract) {
- // Define variable debug information entry.
- auto VariableDie = DIE::get(DIEValueAllocator, DV.getTag());
- insertDIE(DV.getVariable(), VariableDie);
+void DwarfCompileUnit::applyConcreteDbgVariableAttributes(
+ const Loc::Multi &Multi, const DbgVariable &DV, DIE *VariableDie) {
+ addLocationList(*VariableDie, dwarf::DW_AT_location,
+ Multi.getDebugLocListIndex());
+ auto TagOffset = Multi.getDebugLocListTagOffset();
+ if (TagOffset)
+ addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
+ *TagOffset);
+}
- if (Abstract) {
- applyVariableAttributes(DV, *VariableDie);
- return VariableDie;
- }
+void DwarfCompileUnit::applyConcreteDbgVariableAttributes(const Loc::MMI &MMI,
+ const DbgVariable &DV,
+ DIE *VariableDie) {
+ std::optional<unsigned> NVPTXAddressSpace;
+ DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+ DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+ for (const auto &Fragment : MMI.getFrameIndexExprs()) {
+ Register FrameReg;
+ const DIExpression *Expr = Fragment.Expr;
+ const TargetFrameLowering *TFI = Asm->MF->getSubtarget().getFrameLowering();
+ StackOffset Offset =
+ TFI->getFrameIndexReference(*Asm->MF, Fragment.FI, FrameReg);
+ DwarfExpr.addFragmentOffset(Expr);
- // Add variable address.
- std::visit(
- makeVisitor(
- [=, &DV](const Loc::Single &Single) {
- const DbgValueLoc *DVal = &Single.getValueLoc();
- if (!DVal->isVariadic()) {
- const DbgValueLocEntry *Entry = DVal->getLocEntries().begin();
- if (Entry->isLocation()) {
- addVariableAddress(DV, *VariableDie, Entry->getLoc());
- } else if (Entry->isInt()) {
- auto *Expr = Single.getExpr();
- if (Expr && Expr->getNumElements()) {
- DIELoc *Loc = new (DIEValueAllocator) DIELoc;
- DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
- // If there is an expression, emit raw unsigned bytes.
- DwarfExpr.addFragmentOffset(Expr);
- DwarfExpr.addUnsignedConstant(Entry->getInt());
- DwarfExpr.addExpression(Expr);
- addBlock(*VariableDie, dwarf::DW_AT_location,
- DwarfExpr.finalize());
- if (DwarfExpr.TagOffset)
- addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
- dwarf::DW_FORM_data1, *DwarfExpr.TagOffset);
- } else
- addConstantValue(*VariableDie, Entry->getInt(), DV.getType());
- } else if (Entry->isConstantFP()) {
- addConstantFPValue(*VariableDie, Entry->getConstantFP());
- } else if (Entry->isConstantInt()) {
- addConstantValue(*VariableDie, Entry->getConstantInt(),
- DV.getType());
- } else if (Entry->isTargetIndexLocation()) {
- DIELoc *Loc = new (DIEValueAllocator) DIELoc;
- DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
- const DIBasicType *BT = dyn_cast<DIBasicType>(
- static_cast<const Metadata *>(DV.getVariable()->getType()));
- DwarfDebug::emitDebugLocValue(*Asm, BT, *DVal, DwarfExpr);
- addBlock(*VariableDie, dwarf::DW_AT_location,
- DwarfExpr.finalize());
- }
- return;
- }
- // If any of the location entries are registers with the value 0,
- // then the location is undefined.
- if (any_of(DVal->getLocEntries(),
- [](const DbgValueLocEntry &Entry) {
- return Entry.isLocation() && !Entry.getLoc().getReg();
- }))
- return;
- const DIExpression *Expr = Single.getExpr();
- assert(Expr && "Variadic Debug Value must have an Expression.");
- DIELoc *Loc = new (DIEValueAllocator) DIELoc;
- DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
- DwarfExpr.addFragmentOffset(Expr);
- DIExpressionCursor Cursor(Expr);
- const TargetRegisterInfo &TRI =
- *Asm->MF->getSubtarget().getRegisterInfo();
-
- auto AddEntry = [&](const DbgValueLocEntry &Entry,
- DIExpressionCursor &Cursor) {
- if (Entry.isLocation()) {
- if (!DwarfExpr.addMachineRegExpression(TRI, Cursor,
- Entry.getLoc().getReg()))
- return false;
- } else if (Entry.isInt()) {
- // If there is an expression, emit raw unsigned bytes.
- DwarfExpr.addUnsignedConstant(Entry.getInt());
- } else if (Entry.isConstantFP()) {
- // DwarfExpression does not support arguments wider than 64 bits
- // (see PR52584).
- // TODO: Consider chunking expressions containing overly wide
- // arguments into separate pointer-sized fragment expressions.
- APInt RawBytes =
- Entry.getConstantFP()->getValueAPF().bitcastToAPInt();
- if (RawBytes.getBitWidth() > 64)
- return false;
- DwarfExpr.addUnsignedConstant(RawBytes.getZExtValue());
- } else if (Entry.isConstantInt()) {
- APInt RawBytes = Entry.getConstantInt()->getValue();
- if (RawBytes.getBitWidth() > 64)
- return false;
- DwarfExpr.addUnsignedConstant(RawBytes.getZExtValue());
- } else if (Entry.isTargetIndexLocation()) {
- TargetIndexLocation Loc = Entry.getTargetIndexLocation();
- // TODO TargetIndexLocation is a target-independent. Currently
- // only the WebAssembly-specific encoding is supported.
- assert(Asm->TM.getTargetTriple().isWasm());
- DwarfExpr.addWasmLocation(Loc.Index,
- static_cast<uint64_t>(Loc.Offset));
- } else {
- llvm_unreachable("Unsupported Entry type.");
- }
- return true;
- };
-
- if (!DwarfExpr.addExpression(
- std::move(Cursor),
- [&](unsigned Idx, DIExpressionCursor &Cursor) -> bool {
- return AddEntry(DVal->getLocEntries()[Idx], Cursor);
- }))
- return;
-
- // Now attach the location information to the DIE.
- addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
- if (DwarfExpr.TagOffset)
- addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
- dwarf::DW_FORM_data1, *DwarfExpr.TagOffset);
- },
- [=](const Loc::Multi &Multi) {
- addLocationList(*VariableDie, dwarf::DW_AT_location,
- Multi.getDebugLocListIndex());
- auto TagOffset = Multi.getDebugLocListTagOffset();
- if (TagOffset)
- addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
- dwarf::DW_FORM_data1, *TagOffset);
- },
- [=](const Loc::MMI &MMI) {
- std::optional<unsigned> NVPTXAddressSpace;
- DIELoc *Loc = new (DIEValueAllocator) DIELoc;
- DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
- for (const auto &Fragment : MMI.getFrameIndexExprs()) {
- Register FrameReg;
- const DIExpression *Expr = Fragment.Expr;
- const TargetFrameLowering *TFI =
- Asm->MF->getSubtarget().getFrameLowering();
- StackOffset Offset =
- TFI->getFrameIndexReference(*Asm->MF, Fragment.FI, FrameReg);
- DwarfExpr.addFragmentOffset(Expr);
-
- auto *TRI = Asm->MF->getSubtarget().getRegisterInfo();
- SmallVector<uint64_t, 8> Ops;
- TRI->getOffsetOpcodes(Offset, Ops);
-
- // According to
- // https://docs.nvidia.com/cuda/archive/10.0/ptx-writers-guide-to-interoperability/index.html#cuda-specific-dwarf
- // cuda-gdb requires DW_AT_address_class for all variables to be
- // able to correctly interpret address space of the variable
- // address. Decode DW_OP_constu <DWARF Address Space> DW_OP_swap
- // DW_OP_xderef sequence for the NVPTX + gdb target.
- unsigned LocalNVPTXAddressSpace;
- if (Asm->TM.getTargetTriple().isNVPTX() && DD->tuneForGDB()) {
- const DIExpression *NewExpr = DIExpression::extractAddressClass(
- Expr, LocalNVPTXAddressSpace);
- if (NewExpr != Expr) {
- Expr = NewExpr;
- NVPTXAddressSpace = LocalNVPTXAddressSpace;
- }
- }
- if (Expr)
- Ops.append(Expr->elements_begin(), Expr->elements_end());
- DIExpressionCursor Cursor(Ops);
- DwarfExpr.setMemoryLocationKind();
- if (const MCSymbol *FrameSymbol = Asm->getFunctionFrameSymbol())
- addOpAddress(*Loc, FrameSymbol);
- else
- DwarfExpr.addMachineRegExpression(
- *Asm->MF->getSubtarget().getRegisterInfo(), Cursor,
- FrameReg);
- DwarfExpr.addExpression(std::move(Cursor));
- }
- if (Asm->TM.getTargetTriple().isNVPTX() && DD->tuneForGDB()) {
- // According to
- // https://docs.nvidia.com/cuda/archive/10.0/ptx-writers-guide-to-interoperability/index.html#cuda-specific-dwarf
- // cuda-gdb requires DW_AT_address_class for all variables to be
- // able to correctly interpret address space of the variable
- // address.
- const unsigned NVPTX_ADDR_local_space = 6;
- addUInt(*VariableDie, dwarf::DW_AT_address_class,
- dwarf::DW_FORM_data1,
- NVPTXAddressSpace.value_or(NVPTX_ADDR_local_space));
- }
- addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
- if (DwarfExpr.TagOffset)
- addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset,
- dwarf::DW_FORM_data1, *DwarfExpr.TagOffset);
- },
- [=](const Loc::EntryValue &EntryValue) {
- DIELoc *Loc = new (DIEValueAllocator) DIELoc;
- DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
- // Emit each expression as: EntryValue(Register) <other ops>
- // <Fragment>.
- for (auto [Register, Expr] : EntryValue.EntryValues) {
- DwarfExpr.addFragmentOffset(&Expr);
- DIExpressionCursor Cursor(Expr.getElements());
- DwarfExpr.beginEntryValueExpression(Cursor);
- DwarfExpr.addMachineRegExpression(
- *Asm->MF->getSubtarget().getRegisterInfo(), Cursor, Register);
- DwarfExpr.addExpression(std::move(Cursor));
- }
- addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
- },
- [](const std::monostate &) {}),
- DV.asVariant());
+ auto *TRI = Asm->MF->getSubtarget().getRegisterInfo();
+ SmallVector<uint64_t, 8> Ops;
+ TRI->getOffsetOpcodes(Offset, Ops);
- return VariableDie;
+ // According to
+ // https://docs.nvidia.com/cuda/archive/10.0/ptx-writers-guide-to-interoperability/index.html#cuda-specific-dwarf
+ // cuda-gdb requires DW_AT_address_class for all variables to be
+ // able to correctly interpret address space of the variable
+ // address. Decode DW_OP_constu <DWARF Address Space> DW_OP_swap
+ // DW_OP_xderef sequence for the NVPTX + gdb target.
+ unsigned LocalNVPTXAddressSpace;
+ if (Asm->TM.getTargetTriple().isNVPTX() && DD->tuneForGDB()) {
+ const DIExpression *NewExpr =
+ DIExpression::extractAddressClass(Expr, LocalNVPTXAddressSpace);
+ if (NewExpr != Expr) {
+ Expr = NewExpr;
+ NVPTXAddressSpace = LocalNVPTXAddressSpace;
+ }
+ }
+ if (Expr)
+ Ops.append(Expr->elements_begin(), Expr->elements_end());
+ DIExpressionCursor Cursor(Ops);
+ DwarfExpr.setMemoryLocationKind();
+ if (const MCSymbol *FrameSymbol = Asm->getFunctionFrameSymbol())
+ addOpAddress(*Loc, FrameSymbol);
+ else
+ DwarfExpr.addMachineRegExpression(
+ ...
|
felipepiovezan
approved these changes
Sep 20, 2023
slinder1
deleted the
dbgvariable-variant-refactor_visit-private-methods
branch
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Sep 21, 2023
Update to the commit landed in http://gerrit-git.amd.com/c/lightning/ec/llvm-project/+/926948 to match upstream review. I had intended to wait until the upstream review llvm#66435 was finished before submitting the gerrit merge of it, but accidentally submitted to soon. Change-Id: Ib244a34b662584b3314f70371e4527870ddea8e5
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fold constructVariableDIEImpl into constructVariableDIE, simplify it and group related functions.
Pull out the previously inline lambdas for visiting the active variant of the DbgVariable to add location and related attributes as an overload set for a private method
applyConcreteDbgVariableAttributes.
Rename applyVariableAttribute to reflect what kinds of attributes it applies, and to contrast it with the new
applyConcreteDbgVariableAttributes.
Move constructLabelDIE down in the implementation file, so all of the constructVariableDIE-related function impls are adjacent.