Skip to content

[RemoveDIs][DebugInfo] Verifier and printing fixes for DPLabel #83242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 4, 2024
Merged

[RemoveDIs][DebugInfo] Verifier and printing fixes for DPLabel #83242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85b695a
Verifier and printing fixes for DPLabel
SLTozer Feb 28, 2024
165c4e2
Further printer/verifier fixes
SLTozer Feb 28, 2024
bd0e920
Fix nits
SLTozer Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 64 additions & 17 deletions llvm/include/llvm/IR/DebugProgramInstruction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,53 @@ class DPMarker;
class DPValue;
class raw_ostream;

/// A typed tracking MDNode reference that does not require a definition for its
/// parameter type. Necessary to avoid including DebugInfoMetadata.h, which has
/// a significant impact on compile times if included in this file.
template <typename T> class DbgRecordParamRef {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this tactic used anywhere else? If so, would it be worth putting in its own header / the TypedTrackingMDNodeRef header?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tactic is used for DebugLoc; that case specializes further for DILocations though, such that we can't trivially share a definition - at the very least we wouldn't deduplicate very much code. In principle I think this could be available in a broader header, //but// imo it should be done at the point we need it rather than preemptively.

TrackingMDNodeRef Ref;

public:
public:
DbgRecordParamRef() = default;

/// Construct from the templated type.
DbgRecordParamRef(const T *Param);

/// Construct from an \a MDNode.
///
/// Note: if \c Param does not have the template type, a verifier check will
/// fail, and accessors will crash. However, construction from other nodes
/// is supported in order to handle forward references when reading textual
/// IR.
explicit DbgRecordParamRef(const MDNode *Param);

/// Get the underlying type.
///
/// \pre !*this or \c isa<T>(getAsMDNode()).
/// @{
T *get() const;
operator T *() const { return get(); }
T *operator->() const { return get(); }
T &operator*() const { return *get(); }
/// @}

/// Check for null.
///
/// Check for null in a way that is safe with broken debug info.
explicit operator bool() const { return Ref; }

/// Return \c this as a \a MDNode.
MDNode *getAsMDNode() const { return Ref; }

bool operator==(const DbgRecordParamRef &Other) const {
return Ref == Other.Ref;
}
bool operator!=(const DbgRecordParamRef &Other) const {
return Ref != Other.Ref;
}
};

/// Base class for non-instruction debug metadata records that have positions
/// within IR. Features various methods copied across from the Instruction
/// class to aid ease-of-use. DbgRecords should always be linked into a
Expand Down Expand Up @@ -174,13 +221,10 @@ inline raw_ostream &operator<<(raw_ostream &OS, const DbgRecord &R) {
/// llvm.dbg.label intrinsic.
/// FIXME: Rename DbgLabelRecord when DPValue is renamed to DbgVariableRecord.
class DPLabel : public DbgRecord {
DILabel *Label;
DbgRecordParamRef<DILabel> Label;

public:
DPLabel(DILabel *Label, DebugLoc DL)
: DbgRecord(LabelKind, DL), Label(Label) {
assert(Label && "Unexpected nullptr");
}
DPLabel(DILabel *Label, DebugLoc DL);

DPLabel *clone() const;
void print(raw_ostream &O, bool IsForDebug = false) const;
Expand All @@ -189,7 +233,8 @@ class DPLabel : public DbgRecord {
Instruction *InsertBefore) const;

void setLabel(DILabel *NewLabel) { Label = NewLabel; }
DILabel *getLabel() const { return Label; }
DILabel *getLabel() const { return Label.get(); }
MDNode *getRawLabel() const { return Label.getAsMDNode(); };

/// Support type inquiry through isa, cast, and dyn_cast.
static bool classof(const DbgRecord *E) {
Expand Down Expand Up @@ -224,9 +269,9 @@ class DPValue : public DbgRecord, protected DebugValueUser {
// DebugValueUser superclass instead. The referred to Value can either be a
// ValueAsMetadata or a DIArgList.

TrackingMDNodeRef Variable;
DIExpression *Expression;
DIExpression *AddressExpression;
DbgRecordParamRef<DILocalVariable> Variable;
DbgRecordParamRef<DIExpression> Expression;
DbgRecordParamRef<DIExpression> AddressExpression;

public:
/// Create a new DPValue representing the intrinsic \p DVI, for example the
Expand Down Expand Up @@ -331,10 +376,6 @@ class DPValue : public DbgRecord, protected DebugValueUser {
void addVariableLocationOps(ArrayRef<Value *> NewValues,
DIExpression *NewExpr);

void setVariable(DILocalVariable *NewVar);

void setExpression(DIExpression *NewExpr) { Expression = NewExpr; }

unsigned getNumVariableLocationOps() const;

bool hasArgList() const { return isa<DIArgList>(getRawLocation()); }
Expand All @@ -349,10 +390,13 @@ class DPValue : public DbgRecord, protected DebugValueUser {
void setKillLocation();
bool isKillLocation() const;

DILocalVariable *getVariable() const;
MDNode *getRawVariable() const { return Variable; }
void setVariable(DILocalVariable *NewVar) { Variable = NewVar; }
DILocalVariable *getVariable() const { return Variable.get(); };
MDNode *getRawVariable() const { return Variable.getAsMDNode(); }

DIExpression *getExpression() const { return Expression; }
void setExpression(DIExpression *NewExpr) { Expression = NewExpr; }
DIExpression *getExpression() const { return Expression.get(); }
MDNode *getRawExpression() const { return Expression.getAsMDNode(); }

/// Returns the metadata operand for the first location description. i.e.,
/// dbg intrinsic dbg.value,declare operand and dbg.assign 1st location
Expand Down Expand Up @@ -401,7 +445,10 @@ class DPValue : public DbgRecord, protected DebugValueUser {
}
Metadata *getRawAssignID() const { return DebugValues[2]; }
DIAssignID *getAssignID() const;
DIExpression *getAddressExpression() const { return AddressExpression; }
DIExpression *getAddressExpression() const { return AddressExpression.get(); }
MDNode *getRawAddressExpression() const {
return AddressExpression.getAsMDNode();
}
void setAddressExpression(DIExpression *NewExpr) {
AddressExpression = NewExpr;
}
Expand Down
14 changes: 8 additions & 6 deletions llvm/lib/IR/AsmWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1147,7 +1147,7 @@ void SlotTracker::processDbgRecordMetadata(const DbgRecord &DR) {
if (DPV->isDbgAssign())
CreateMetadataSlot(cast<MDNode>(DPV->getRawAssignID()));
} else if (const DPLabel *DPL = dyn_cast<const DPLabel>(&DR)) {
CreateMetadataSlot(DPL->getLabel());
CreateMetadataSlot(DPL->getRawLabel());
} else {
llvm_unreachable("unsupported DbgRecord kind");
}
Expand Down Expand Up @@ -4631,16 +4631,16 @@ void AssemblyWriter::printDPValue(const DPValue &DPV) {
Out << "(";
WriteAsOperandInternal(Out, DPV.getRawLocation(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, DPV.getVariable(), WriterCtx, true);
WriteAsOperandInternal(Out, DPV.getRawVariable(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, DPV.getExpression(), WriterCtx, true);
WriteAsOperandInternal(Out, DPV.getRawExpression(), WriterCtx, true);
Out << ", ";
if (DPV.isDbgAssign()) {
WriteAsOperandInternal(Out, DPV.getAssignID(), WriterCtx, true);
WriteAsOperandInternal(Out, DPV.getRawAssignID(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, DPV.getRawAddress(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, DPV.getAddressExpression(), WriterCtx, true);
WriteAsOperandInternal(Out, DPV.getRawAddressExpression(), WriterCtx, true);
Out << ", ";
}
WriteAsOperandInternal(Out, DPV.getDebugLoc().getAsMDNode(), WriterCtx, true);
Expand All @@ -4659,7 +4659,9 @@ void AssemblyWriter::printDbgRecordLine(const DbgRecord &DR) {
void AssemblyWriter::printDPLabel(const DPLabel &Label) {
auto WriterCtx = getContext();
Out << "#dbg_label(";
WriteAsOperandInternal(Out, Label.getLabel(), WriterCtx, true);
WriteAsOperandInternal(Out, Label.getRawLabel(), WriterCtx, true);
Out << ", ";
WriteAsOperandInternal(Out, Label.getDebugLoc(), WriterCtx, true);
Out << ")";
}

Expand Down
32 changes: 24 additions & 8 deletions llvm/lib/IR/DebugProgramInstruction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,26 @@

namespace llvm {

template <typename T>
DbgRecordParamRef<T>::DbgRecordParamRef(const T *Param)
: Ref(const_cast<T *>(Param)) {}
template <typename T>
DbgRecordParamRef<T>::DbgRecordParamRef(const MDNode *Param)
: Ref(const_cast<MDNode *>(Param)) {}

template <typename T> T *DbgRecordParamRef<T>::get() const {
return cast<T>(Ref);
}

template class DbgRecordParamRef<DIExpression>;
template class DbgRecordParamRef<DILabel>;
template class DbgRecordParamRef<DILocalVariable>;

DPValue::DPValue(const DbgVariableIntrinsic *DVI)
: DbgRecord(ValueKind, DVI->getDebugLoc()),
DebugValueUser({DVI->getRawLocation(), nullptr, nullptr}),
Variable(DVI->getVariable()), Expression(DVI->getExpression()),
AddressExpression(nullptr) {
AddressExpression() {
switch (DVI->getIntrinsicID()) {
case Intrinsic::dbg_value:
Type = LocationType::Value;
Expand Down Expand Up @@ -123,6 +138,11 @@ DbgRecord::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
llvm_unreachable("unsupported DbgRecord kind");
}

DPLabel::DPLabel(DILabel *Label, DebugLoc DL)
: DbgRecord(LabelKind, DL), Label(Label) {
assert(Label && "Unexpected nullptr");
}

DPValue *DPValue::createDPValue(Value *Location, DILocalVariable *DV,
DIExpression *Expr, const DILocation *DI) {
return new DPValue(ValueAsMetadata::get(Location), DV, Expr, DI,
Expand Down Expand Up @@ -175,8 +195,6 @@ DPValue *DPValue::createLinkedDPVAssign(Instruction *LinkedInstr, Value *Val,
return NewDPVAssign;
}

void DPValue::setVariable(DILocalVariable *NewVar) { Variable.reset(NewVar); }

iterator_range<DPValue::location_op_iterator> DPValue::location_ops() const {
auto *MD = getRawLocation();
// If a Value has been deleted, the "location" for this DPValue will be
Expand Down Expand Up @@ -315,10 +333,6 @@ bool DPValue::isKillLocation() const {
any_of(location_ops(), [](Value *V) { return isa<UndefValue>(V); });
}

DILocalVariable *DPValue::getVariable() const {
return cast<DILocalVariable>(Variable.get());
}

std::optional<uint64_t> DPValue::getFragmentSizeInBits() const {
if (auto Fragment = getExpression()->getFragmentInfo())
return Fragment->SizeInBits;
Expand All @@ -337,7 +351,9 @@ DbgRecord *DbgRecord::clone() const {

DPValue *DPValue::clone() const { return new DPValue(*this); }

DPLabel *DPLabel::clone() const { return new DPLabel(Label, getDebugLoc()); }
DPLabel *DPLabel::clone() const {
return new DPLabel(getLabel(), getDebugLoc());
}

DbgVariableIntrinsic *
DPValue::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
Expand Down
Loading