Skip to content

[RemoveDIs][NFC] Add DPLabel class [2/3] #82376

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
Feb 22, 2024
Merged

[RemoveDIs][NFC] Add DPLabel class [2/3] #82376

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1de5104
[RemoveDIs][NFC] Add DPLabel class [2/3]
OCHyams Jan 15, 2024
ff072ac
address review coments
OCHyams Feb 22, 2024
84ecb43
nit
OCHyams Feb 22, 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
32 changes: 29 additions & 3 deletions llvm/include/llvm/IR/DebugProgramInstruction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,13 @@ class raw_ostream;
/// deleteRecord
/// clone
/// isIdenticalToWhenDefined
/// isEquivalentTo
/// both print methods
class DbgRecord : public ilist_node<DbgRecord> {
public:
/// Marker that this DbgRecord is linked into.
DPMarker *Marker = nullptr;
/// Subclass discriminator.
enum Kind : uint8_t { ValueKind };
enum Kind : uint8_t { ValueKind, LabelKind };

protected:
DebugLoc DbgLoc;
Expand All @@ -104,9 +103,11 @@ class DbgRecord : public ilist_node<DbgRecord> {
void print(raw_ostream &O, bool IsForDebug = false) const;
void print(raw_ostream &O, ModuleSlotTracker &MST, bool IsForDebug) const;
bool isIdenticalToWhenDefined(const DbgRecord &R) const;
bool isEquivalentTo(const DbgRecord &R) const;
///@}

/// Same as isIdenticalToWhenDefined but checks DebugLoc too.
bool isEquivalentTo(const DbgRecord &R) const;

Kind getRecordKind() const { return RecordKind; }

void setMarker(DPMarker *M) { Marker = M; }
Expand Down Expand Up @@ -156,6 +157,31 @@ class DbgRecord : public ilist_node<DbgRecord> {
~DbgRecord() = default;
};

/// Records a position in IR for a source label (DILabel). Corresponds to the
/// llvm.dbg.label intrinsic.
/// FIXME: Rename DbgLabelRecord when DPValue is renamed to DbgVariableRecord.
class DPLabel : public DbgRecord {
DILabel *Label;

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

DPLabel *clone() const;
void print(raw_ostream &O, bool IsForDebug = false) const;
void print(raw_ostream &ROS, ModuleSlotTracker &MST, bool IsForDebug) const;

void setLabel(DILabel *NewLabel) { Label = NewLabel; }
DILabel *getLabel() const { return Label; }

/// Support type inquiry through isa, cast, and dyn_cast.
static bool classof(const DbgRecord *E) {
return E->getRecordKind() == LabelKind;
}
};

/// Record of a variable value-assignment, aka a non instruction representation
/// of the dbg.value intrinsic.
///
Expand Down
41 changes: 38 additions & 3 deletions llvm/lib/IR/AsmWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ static const Module *getModuleFromDPI(const DPMarker *Marker) {
return M ? M->getParent() : nullptr;
}

static const Module *getModuleFromDPI(const DPValue *DPV) {
return DPV->getMarker() ? getModuleFromDPI(DPV->getMarker()) : nullptr;
static const Module *getModuleFromDPI(const DbgRecord *DR) {
return DR->getMarker() ? getModuleFromDPI(DR->getMarker()) : nullptr;
}

static void PrintCallingConv(unsigned cc, raw_ostream &Out) {
Expand Down Expand Up @@ -2676,6 +2676,7 @@ class AssemblyWriter {
void printInstruction(const Instruction &I);
void printDPMarker(const DPMarker &DPI);
void printDPValue(const DPValue &DPI);
void printDPLabel(const DPLabel &DPL);
void printDbgRecord(const DbgRecord &DPI);
Copy link
Contributor

Choose a reason for hiding this comment

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

printDbgRecord should be updated to call through to printDPLabel, right?


void printUseListOrder(const Value *V, const std::vector<unsigned> &Shuffle);
Expand Down Expand Up @@ -4580,7 +4581,7 @@ void AssemblyWriter::printDbgRecord(const DbgRecord &DR) {
if (auto *DPV = dyn_cast<DPValue>(&DR))
printDPValue(*DPV);
else
llvm_unreachable("unsupported dbg record");
printDPLabel(cast<DPLabel>(DR));
Copy link
Contributor

Choose a reason for hiding this comment

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

At the risk of being a pain, should we make this an else if (<Label Cast>) followed by an llvm_unreachable? As a guard against future types being added and missed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cast would catch that, but I can do so if you'd prefer. The unreachable has a nicer error message than cast failure (and is what I've done elsewhere). Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer the unreachable, since it's clearer and more consistent with the approach elsewhere - imo at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I said I'd done the unreachable approach elsewhere, but I've actually done a mix (in future patches) of whatever looks right at the site. I'll go with unreachable here as there's not much in it, but I might push back against this pattern in the next patches - in some cases it's much cleaner to just cast (imo), but we can discuss that in those patches!

}

void AssemblyWriter::printDPValue(const DPValue &Value) {
Expand Down Expand Up @@ -4622,6 +4623,16 @@ void AssemblyWriter::printDPValue(const DPValue &Value) {
Out << " }";
}

void AssemblyWriter::printDPLabel(const DPLabel &Label) {
// There's no formal representation of a DPLabel -- print purely as
// a debugging aid.
Out << " DPLabel { ";
auto WriterCtx = getContext();
WriteAsOperandInternal(Out, Label.getLabel(), WriterCtx, true);
Out << " marker @" << Label.getMarker();
Out << " }";
}

void AssemblyWriter::printMetadataAttachments(
const SmallVectorImpl<std::pair<unsigned, MDNode *>> &MDs,
StringRef Separator) {
Expand Down Expand Up @@ -4885,6 +4896,12 @@ void DPMarker::print(raw_ostream &ROS, ModuleSlotTracker &MST,
W.printDPMarker(*this);
}

void DPLabel::print(raw_ostream &ROS, bool IsForDebug) const {

ModuleSlotTracker MST(getModuleFromDPI(this), true);
print(ROS, MST, IsForDebug);
}

void DPValue::print(raw_ostream &ROS, ModuleSlotTracker &MST,
bool IsForDebug) const {
// There's no formal representation of a DPValue -- print purely as a
Expand All @@ -4904,6 +4921,24 @@ void DPValue::print(raw_ostream &ROS, ModuleSlotTracker &MST,
W.printDPValue(*this);
}

void DPLabel::print(raw_ostream &ROS, ModuleSlotTracker &MST,
bool IsForDebug) const {
// There's no formal representation of a DbgLabelRecord -- print purely as
// a debugging aid.
formatted_raw_ostream OS(ROS);
SlotTracker EmptySlotTable(static_cast<const Module *>(nullptr));
SlotTracker &SlotTable =
MST.getMachine() ? *MST.getMachine() : EmptySlotTable;
auto incorporateFunction = [&](const Function *F) {
if (F)
MST.incorporateFunction(*F);
};
incorporateFunction(Marker->getParent() ? Marker->getParent()->getParent()
: nullptr);
AssemblyWriter W(OS, SlotTable, getModuleFromDPI(this), nullptr, IsForDebug);
W.printDPLabel(*this);
}

void Value::print(raw_ostream &ROS, bool IsForDebug) const {
bool ShouldInitializeAllMetadata = false;
if (auto *I = dyn_cast<Instruction>(this))
Expand Down
23 changes: 16 additions & 7 deletions llvm/lib/IR/DebugProgramInstruction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ void DbgRecord::deleteRecord() {
case ValueKind:
delete cast<DPValue>(this);
return;
case LabelKind:
delete cast<DPLabel>(this);
return;
}
llvm_unreachable("unsupported DbgRecord kind");
}
Expand All @@ -73,6 +76,9 @@ void DbgRecord::print(raw_ostream &O, bool IsForDebug) const {
case ValueKind:
cast<DPValue>(this)->print(O, IsForDebug);
return;
case LabelKind:
cast<DPLabel>(this)->print(O, IsForDebug);
return;
};
llvm_unreachable("unsupported DbgRecord kind");
}
Expand All @@ -83,6 +89,9 @@ void DbgRecord::print(raw_ostream &O, ModuleSlotTracker &MST,
case ValueKind:
cast<DPValue>(this)->print(O, MST, IsForDebug);
return;
case LabelKind:
cast<DPLabel>(this)->print(O, MST, IsForDebug);
return;
};
llvm_unreachable("unsupported DbgRecord kind");
}
Expand All @@ -93,18 +102,14 @@ bool DbgRecord::isIdenticalToWhenDefined(const DbgRecord &R) const {
switch (RecordKind) {
case ValueKind:
return cast<DPValue>(this)->isIdenticalToWhenDefined(*cast<DPValue>(&R));
case LabelKind:
return cast<DPLabel>(this)->getLabel() == cast<DPLabel>(R).getLabel();
};
llvm_unreachable("unsupported DbgRecord kind");
}

bool DbgRecord::isEquivalentTo(const DbgRecord &R) const {
if (RecordKind != R.RecordKind)
return false;
switch (RecordKind) {
case ValueKind:
return cast<DPValue>(this)->isEquivalentTo(*cast<DPValue>(&R));
};
llvm_unreachable("unsupported DbgRecord kind");
return getDebugLoc() == R.getDebugLoc() && isIdenticalToWhenDefined(R);
}

DPValue *DPValue::createDPValue(Value *Location, DILocalVariable *DV,
Expand Down Expand Up @@ -307,12 +312,16 @@ DbgRecord *DbgRecord::clone() const {
switch (RecordKind) {
case ValueKind:
return cast<DPValue>(this)->clone();
case LabelKind:
return cast<DPLabel>(this)->clone();
};
llvm_unreachable("unsupported DbgRecord kind");
}

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

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

DbgVariableIntrinsic *
DPValue::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
[[maybe_unused]] DICompileUnit *Unit =
Expand Down