Skip to content

[RemoveDIs][DebugInfo] Make DIAssignID always replaceable #78300

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 1 commit into from
Jan 17, 2024
Merged

[RemoveDIs][DebugInfo] Make DIAssignID always replaceable #78300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions llvm/include/llvm/IR/DebugInfoMetadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ class DIAssignID : public MDNode {
// This node has no operands to replace.
void replaceOperandWith(unsigned I, Metadata *New) = delete;

SmallVector<DPValue *> getAllDPValueUsers() {
return Context.getReplaceableUses()->getAllDPValueUsers();
}

static DIAssignID *getDistinct(LLVMContext &Context) {
return getImpl(Context, Distinct);
}
Expand Down
4 changes: 3 additions & 1 deletion llvm/include/llvm/IR/Metadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1057,6 +1057,7 @@ struct TempMDNodeDeleter {
class MDNode : public Metadata {
friend class ReplaceableMetadataImpl;
friend class LLVMContextImpl;
friend class DIAssignID;

/// The header that is coallocated with an MDNode along with its "small"
/// operands. It is located immediately before the main body of the node.
Expand Down Expand Up @@ -1239,7 +1240,8 @@ class MDNode : public Metadata {
bool isDistinct() const { return Storage == Distinct; }
bool isTemporary() const { return Storage == Temporary; }

bool isReplaceable() const { return isTemporary(); }
bool isReplaceable() const { return isTemporary() || isAlwaysReplaceable(); }
bool isAlwaysReplaceable() const { return getMetadataID() == DIAssignIDKind; }

/// RAUW a temporary.
///
Expand Down
9 changes: 2 additions & 7 deletions llvm/lib/IR/DebugInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1793,13 +1793,6 @@ void at::deleteAssignmentMarkers(const Instruction *Inst) {
}

void at::RAUW(DIAssignID *Old, DIAssignID *New) {
// Replace MetadataAsValue uses.
if (auto *OldIDAsValue =
MetadataAsValue::getIfExists(Old->getContext(), Old)) {
auto *NewIDAsValue = MetadataAsValue::get(Old->getContext(), New);
OldIDAsValue->replaceAllUsesWith(NewIDAsValue);
}

// Replace attachments.
AssignmentInstRange InstRange = getAssignmentInsts(Old);
// Use intermediate storage for the instruction ptrs because the
Expand All @@ -1808,6 +1801,8 @@ void at::RAUW(DIAssignID *Old, DIAssignID *New) {
SmallVector<Instruction *> InstVec(InstRange.begin(), InstRange.end());
for (auto *I : InstVec)
I->setMetadata(LLVMContext::MD_DIAssignID, New);

Old->replaceAllUsesWith(New);
}

void at::deleteAll(Function *F) {
Expand Down
16 changes: 11 additions & 5 deletions llvm/lib/IR/Metadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,24 +439,30 @@ void ReplaceableMetadataImpl::resolveAllUses(bool ResolveUsers) {
// commentry in DIArgList::handleChangedOperand for details. Hidden behind
// conditional compilation to avoid a compile time regression.
ReplaceableMetadataImpl *ReplaceableMetadataImpl::getOrCreate(Metadata &MD) {
if (auto *N = dyn_cast<MDNode>(&MD))
return N->isResolved() ? nullptr : N->Context.getOrCreateReplaceableUses();
if (auto *N = dyn_cast<MDNode>(&MD)) {
return !N->isResolved() || N->isAlwaysReplaceable()
? N->Context.getOrCreateReplaceableUses()
: nullptr;
}
if (auto ArgList = dyn_cast<DIArgList>(&MD))
return ArgList;
return dyn_cast<ValueAsMetadata>(&MD);
}

ReplaceableMetadataImpl *ReplaceableMetadataImpl::getIfExists(Metadata &MD) {
if (auto *N = dyn_cast<MDNode>(&MD))
return N->isResolved() ? nullptr : N->Context.getReplaceableUses();
if (auto *N = dyn_cast<MDNode>(&MD)) {
return !N->isResolved() || N->isAlwaysReplaceable()
? N->Context.getReplaceableUses()
: nullptr;
}
if (auto ArgList = dyn_cast<DIArgList>(&MD))
return ArgList;
return dyn_cast<ValueAsMetadata>(&MD);
}

bool ReplaceableMetadataImpl::isReplaceable(const Metadata &MD) {
if (auto *N = dyn_cast<MDNode>(&MD))
return !N->isResolved();
return !N->isResolved() || N->isAlwaysReplaceable();
return isa<ValueAsMetadata>(&MD) || isa<DIArgList>(&MD);
}

Expand Down
11 changes: 11 additions & 0 deletions llvm/lib/IR/Verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ struct VerifierSupport {
}
}

void Write(const DPValue *V) {
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 reason for this to be added here in a mostly unrelated-looking patch is because of the other verifier change below - now that we can lookup DPValue users of a DIAssignID, the verifier should be checking them the same as it does for debug intrinsic users, and as this is the first time DPValue has come up in the verifier, it needs a Write implementation for the purpose of printing errors.

if (V)
V->print(*OS, MST, false);
}

void Write(const Metadata *MD) {
if (!MD)
return;
Expand Down Expand Up @@ -4723,6 +4728,12 @@ void Verifier::visitDIAssignIDMetadata(Instruction &I, MDNode *MD) {
"dbg.assign not in same function as inst", DAI, &I);
}
}
for (DPValue *DPV : cast<DIAssignID>(MD)->getAllDPValueUsers()) {
CheckDI(DPV->isDbgAssign(),
"!DIAssignID should only be used by Assign DPVs.", MD, DPV);
CheckDI(DPV->getFunction() == I.getFunction(),
"DPVAssign not in same function as inst", DPV, &I);
}
}

void Verifier::visitCallStackMetadata(MDNode *MD) {
Expand Down