[DebugInfo] Delete debug-intrinsic verifier checks #149066
Merged
Conversation
This file contains hidden or 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
We no longer produce debug-intrinsics, and whenever they're spotted in bitcode or textual IR they get autoupgraded. We could quite reasonably reject them out of hand as a construct that shouldn't be present. However, the DXIL folks are likely to be converting records back to intrinsics for years to come, and there's no need to make that an error. There's no value in verifying them IMO.
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-debuginfo Author: Jeremy Morse (jmorse) ChangesWe no longer produce debug-intrinsics, and whenever they're spotted in bitcode or textual IR they get autoupgraded. We could quite reasonably reject them out of hand as a construct that shouldn't be present. However, the DXIL folks are likely to be converting records back to intrinsics for years to come, and there's no need to make that an error. There's no value in verifying them IMO. Full diff: https://github.com/llvm/llvm-project/pull/149066.diff 1 Files Affected:
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index dc5373e172f28..670c33e05d925 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -597,7 +597,6 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
void visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call);
void visitConstrainedFPIntrinsic(ConstrainedFPIntrinsic &FPI);
void visitVPIntrinsic(VPIntrinsic &VPI);
- void visitDbgIntrinsic(StringRef Kind, DbgVariableIntrinsic &DII);
void visitDbgLabelIntrinsic(StringRef Kind, DbgLabelInst &DLI);
void visitAtomicCmpXchgInst(AtomicCmpXchgInst &CXI);
void visitAtomicRMWInst(AtomicRMWInst &RMWI);
@@ -636,15 +635,12 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
void verifyFrameRecoverIndices();
void verifySiblingFuncletUnwinds();
- void verifyFragmentExpression(const DbgVariableIntrinsic &I);
void verifyFragmentExpression(const DbgVariableRecord &I);
template <typename ValueOrMetadata>
void verifyFragmentExpression(const DIVariable &V,
DIExpression::FragmentInfo Fragment,
ValueOrMetadata *Desc);
- void verifyFnArgs(const DbgVariableIntrinsic &I);
void verifyFnArgs(const DbgVariableRecord &DVR);
- void verifyNotEntryValue(const DbgVariableIntrinsic &I);
void verifyNotEntryValue(const DbgVariableRecord &I);
/// Module-level debug info verification...
@@ -5494,11 +5490,6 @@ void Verifier::visitInstruction(Instruction &I) {
visitMDNode(*N, AreDebugLocsAllowed::Yes);
}
- if (auto *DII = dyn_cast<DbgVariableIntrinsic>(&I)) {
- verifyFragmentExpression(*DII);
- verifyNotEntryValue(*DII);
- }
-
SmallVector<std::pair<unsigned, MDNode *>, 4> MDs;
I.getAllMetadata(MDs);
for (auto Attachment : MDs) {
@@ -5703,18 +5694,14 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
visitConstrainedFPIntrinsic(cast<ConstrainedFPIntrinsic>(Call));
break;
case Intrinsic::dbg_declare: // llvm.dbg.declare
- Check(isa<MetadataAsValue>(Call.getArgOperand(0)),
- "invalid llvm.dbg.declare intrinsic call 1", Call);
- visitDbgIntrinsic("declare", cast<DbgVariableIntrinsic>(Call));
- break;
case Intrinsic::dbg_value: // llvm.dbg.value
- visitDbgIntrinsic("value", cast<DbgVariableIntrinsic>(Call));
- break;
case Intrinsic::dbg_assign: // llvm.dbg.assign
- visitDbgIntrinsic("assign", cast<DbgVariableIntrinsic>(Call));
- break;
case Intrinsic::dbg_label: // llvm.dbg.label
- visitDbgLabelIntrinsic("label", cast<DbgLabelInst>(Call));
+ // We no longer interpret debug intrinsics (the old variable-location
+ // design). They're meaningless as far as LLVM is concerned we could make
+ // it an error for them to appear, but it's possible we'll have users
+ // converting back to intrinsics for the forseeable future (such as DXIL),
+ // so tolerate their existance.
break;
case Intrinsic::memcpy:
case Intrinsic::memcpy_inline:
@@ -7123,123 +7110,6 @@ void Verifier::visitConstrainedFPIntrinsic(ConstrainedFPIntrinsic &FPI) {
}
}
-void Verifier::visitDbgIntrinsic(StringRef Kind, DbgVariableIntrinsic &DII) {
- auto *MD = DII.getRawLocation();
- CheckDI(isa<ValueAsMetadata>(MD) || isa<DIArgList>(MD) ||
- (isa<MDNode>(MD) && !cast<MDNode>(MD)->getNumOperands()),
- "invalid llvm.dbg." + Kind + " intrinsic address/value", &DII, MD);
- CheckDI(isa<DILocalVariable>(DII.getRawVariable()),
- "invalid llvm.dbg." + Kind + " intrinsic variable", &DII,
- DII.getRawVariable());
- CheckDI(isa<DIExpression>(DII.getRawExpression()),
- "invalid llvm.dbg." + Kind + " intrinsic expression", &DII,
- DII.getRawExpression());
-
- if (auto *DAI = dyn_cast<DbgAssignIntrinsic>(&DII)) {
- CheckDI(isa<DIAssignID>(DAI->getRawAssignID()),
- "invalid llvm.dbg.assign intrinsic DIAssignID", &DII,
- DAI->getRawAssignID());
- const auto *RawAddr = DAI->getRawAddress();
- CheckDI(
- isa<ValueAsMetadata>(RawAddr) ||
- (isa<MDNode>(RawAddr) && !cast<MDNode>(RawAddr)->getNumOperands()),
- "invalid llvm.dbg.assign intrinsic address", &DII,
- DAI->getRawAddress());
- CheckDI(isa<DIExpression>(DAI->getRawAddressExpression()),
- "invalid llvm.dbg.assign intrinsic address expression", &DII,
- DAI->getRawAddressExpression());
- // All of the linked instructions should be in the same function as DII.
- for (Instruction *I : at::getAssignmentInsts(DAI))
- CheckDI(DAI->getFunction() == I->getFunction(),
- "inst not in same function as dbg.assign", I, DAI);
- }
-
- // Ignore broken !dbg attachments; they're checked elsewhere.
- if (MDNode *N = DII.getDebugLoc().getAsMDNode())
- if (!isa<DILocation>(N))
- return;
-
- BasicBlock *BB = DII.getParent();
- Function *F = BB ? BB->getParent() : nullptr;
-
- // The scopes for variables and !dbg attachments must agree.
- DILocalVariable *Var = DII.getVariable();
- DILocation *Loc = DII.getDebugLoc();
- CheckDI(Loc, "llvm.dbg." + Kind + " intrinsic requires a !dbg attachment",
- &DII, BB, F);
-
- DISubprogram *VarSP = getSubprogram(Var->getRawScope());
- DISubprogram *LocSP = getSubprogram(Loc->getRawScope());
- if (!VarSP || !LocSP)
- return; // Broken scope chains are checked elsewhere.
-
- CheckDI(VarSP == LocSP,
- "mismatched subprogram between llvm.dbg." + Kind +
- " variable and !dbg attachment",
- &DII, BB, F, Var, Var->getScope()->getSubprogram(), Loc,
- Loc->getScope()->getSubprogram());
-
- // This check is redundant with one in visitLocalVariable().
- CheckDI(isType(Var->getRawType()), "invalid type ref", Var,
- Var->getRawType());
- verifyFnArgs(DII);
-}
-
-void Verifier::visitDbgLabelIntrinsic(StringRef Kind, DbgLabelInst &DLI) {
- CheckDI(isa<DILabel>(DLI.getRawLabel()),
- "invalid llvm.dbg." + Kind + " intrinsic variable", &DLI,
- DLI.getRawLabel());
-
- // Ignore broken !dbg attachments; they're checked elsewhere.
- if (MDNode *N = DLI.getDebugLoc().getAsMDNode())
- if (!isa<DILocation>(N))
- return;
-
- BasicBlock *BB = DLI.getParent();
- Function *F = BB ? BB->getParent() : nullptr;
-
- // The scopes for variables and !dbg attachments must agree.
- DILabel *Label = DLI.getLabel();
- DILocation *Loc = DLI.getDebugLoc();
- Check(Loc, "llvm.dbg." + Kind + " intrinsic requires a !dbg attachment", &DLI,
- BB, F);
-
- DISubprogram *LabelSP = getSubprogram(Label->getRawScope());
- DISubprogram *LocSP = getSubprogram(Loc->getRawScope());
- if (!LabelSP || !LocSP)
- return;
-
- CheckDI(LabelSP == LocSP,
- "mismatched subprogram between llvm.dbg." + Kind +
- " label and !dbg attachment",
- &DLI, BB, F, Label, Label->getScope()->getSubprogram(), Loc,
- Loc->getScope()->getSubprogram());
-}
-
-void Verifier::verifyFragmentExpression(const DbgVariableIntrinsic &I) {
- DILocalVariable *V = dyn_cast_or_null<DILocalVariable>(I.getRawVariable());
- DIExpression *E = dyn_cast_or_null<DIExpression>(I.getRawExpression());
-
- // We don't know whether this intrinsic verified correctly.
- if (!V || !E || !E->isValid())
- return;
-
- // Nothing to do if this isn't a DW_OP_LLVM_fragment expression.
- auto Fragment = E->getFragmentInfo();
- if (!Fragment)
- return;
-
- // The frontend helps out GDB by emitting the members of local anonymous
- // unions as artificial local variables with shared storage. When SROA splits
- // the storage for artificial local variables that are smaller than the entire
- // union, the overhang piece will be outside of the allotted space for the
- // variable and this check fails.
- // FIXME: Remove this check as soon as clang stops doing this; it hides bugs.
- if (V->isArtificial())
- return;
-
- verifyFragmentExpression(*V, *Fragment, &I);
-}
void Verifier::verifyFragmentExpression(const DbgVariableRecord &DVR) {
DILocalVariable *V = dyn_cast_or_null<DILocalVariable>(DVR.getRawVariable());
DIExpression *E = dyn_cast_or_null<DIExpression>(DVR.getRawExpression());
@@ -7282,34 +7152,6 @@ void Verifier::verifyFragmentExpression(const DIVariable &V,
CheckDI(FragSize != *VarSize, "fragment covers entire variable", Desc, &V);
}
-void Verifier::verifyFnArgs(const DbgVariableIntrinsic &I) {
- // This function does not take the scope of noninlined function arguments into
- // account. Don't run it if current function is nodebug, because it may
- // contain inlined debug intrinsics.
- if (!HasDebugInfo)
- return;
-
- // For performance reasons only check non-inlined ones.
- if (I.getDebugLoc()->getInlinedAt())
- return;
-
- DILocalVariable *Var = I.getVariable();
- CheckDI(Var, "dbg intrinsic without variable");
-
- unsigned ArgNo = Var->getArg();
- if (!ArgNo)
- return;
-
- // Verify there are no duplicate function argument debug info entries.
- // These will cause hard-to-debug assertions in the DWARF backend.
- if (DebugFnArgs.size() < ArgNo)
- DebugFnArgs.resize(ArgNo, nullptr);
-
- auto *Prev = DebugFnArgs[ArgNo - 1];
- DebugFnArgs[ArgNo - 1] = Var;
- CheckDI(!Prev || (Prev == Var), "conflicting debug info for argument", &I,
- Prev, Var);
-}
void Verifier::verifyFnArgs(const DbgVariableRecord &DVR) {
// This function does not take the scope of noninlined function arguments into
// account. Don't run it if current function is nodebug, because it may
@@ -7339,29 +7181,6 @@ void Verifier::verifyFnArgs(const DbgVariableRecord &DVR) {
Prev, Var);
}
-void Verifier::verifyNotEntryValue(const DbgVariableIntrinsic &I) {
- DIExpression *E = dyn_cast_or_null<DIExpression>(I.getRawExpression());
-
- // We don't know whether this intrinsic verified correctly.
- if (!E || !E->isValid())
- return;
-
- if (isa<ValueAsMetadata>(I.getRawLocation())) {
- Value *VarValue = I.getVariableLocationOp(0);
- if (isa<UndefValue>(VarValue) || isa<PoisonValue>(VarValue))
- return;
- // We allow EntryValues for swift async arguments, as they have an
- // ABI-guarantee to be turned into a specific register.
- if (auto *ArgLoc = dyn_cast_or_null<Argument>(VarValue);
- ArgLoc && ArgLoc->hasAttribute(Attribute::SwiftAsync))
- return;
- }
-
- CheckDI(!E->isEntryValue(),
- "Entry values are only allowed in MIR unless they target a "
- "swiftasync Argument",
- &I);
-}
void Verifier::verifyNotEntryValue(const DbgVariableRecord &DVR) {
DIExpression *E = dyn_cast_or_null<DIExpression>(DVR.getRawExpression());
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
OCHyams
approved these changes
Jul 16, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/23513 Here is the relevant piece of the build log for the reference |
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Jul 17, 2025
…)" This reverts commit b470ac4.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Jul 18, 2025
)" This reverts commit ba650b8.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Jul 18, 2025
Includes currently unmerged changes in https://github.com/AMD-Lightning-Internal/llvm-project/pull/3157 to reland: * ba650b8 Revert "[DebugInfo] Delete debug-intrinsic verifier checks (llvm#149066)" * 8c7c451 Revert "[DebugInfo] Delete a now-unused function after 5328c73" * 8d23754 Revert "[DebugInfo] Strip more debug-intrinsic code from local utils (llvm#149037)"
This was referenced Jul 23, 2025
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.
We no longer produce debug-intrinsics, and whenever they're spotted in bitcode or textual IR they get autoupgraded. We could quite reasonably reject them out of hand as a construct that shouldn't be present. However, the DXIL folks are likely to be converting records back to intrinsics for years to come, and there's no need to make that an error. There's no value in verifying them IMO.