-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[DebugInfo] Slice out a few more users of debug intrinsics #150631
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.
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
base: main
Are you sure you want to change the base?
Conversation
Here's some more dead code that will never run, due to debug intrinsics being decomissioned. I've also replaced one or two generic lambdas with DbgVariableRecord taking ones.
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-flang-openmp Author: Jeremy Morse (jmorse) ChangesHere's some more dead code that will never run, due to debug intrinsics being decomissioned. I've also replaced one or two generic lambdas with DbgVariableRecord taking ones. Full diff: https://github.com/llvm/llvm-project/pull/150631.diff 8 Files Affected:
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index dc5dfab4418e5..ea434fc29404f 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2238,17 +2238,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
return true;
}
case Intrinsic::dbg_assign:
- // A dbg.assign is a dbg.value with more information about stack locations,
- // typically produced during optimisation of variables with leaked
- // addresses. We can treat it like a normal dbg_value intrinsic here; to
- // benefit from the full analysis of stack/SSA locations, GlobalISel would
- // need to register for and use the AssignmentTrackingAnalysis pass.
- [[fallthrough]];
case Intrinsic::dbg_value: {
- // This form of DBG_VALUE is target-independent.
- const DbgValueInst &DI = cast<DbgValueInst>(CI);
- translateDbgValueRecord(DI.getValue(), DI.hasArgList(), DI.getVariable(),
- DI.getExpression(), DI.getDebugLoc(), MIRBuilder);
+ llvm_unreachable("Saw debug intrinsic, should no longer exist");
return true;
}
case Intrinsic::uadd_with_overflow:
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 840ca8364e218..834f7944b6f8b 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -6940,7 +6940,7 @@ static void FixupDebugInfoForOutlinedFunction(
return NewVar;
};
- auto UpdateDebugRecord = [&](auto *DR) {
+ auto UpdateDebugRecord = [&](DbgVariableRecord *DR) {
DILocalVariable *OldVar = DR->getVariable();
unsigned ArgNo = 0;
for (auto Loc : DR->location_ops()) {
@@ -6957,9 +6957,6 @@ static void FixupDebugInfoForOutlinedFunction(
// The location and scope of variable intrinsics and records still point to
// the parent function of the target region. Update them.
for (Instruction &I : instructions(Func)) {
- if (auto *DDI = dyn_cast<llvm::DbgVariableIntrinsic>(&I))
- UpdateDebugRecord(DDI);
-
for (DbgVariableRecord &DVR : filterDbgVars(I.getDbgRecordRange()))
UpdateDebugRecord(&DVR);
}
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index ab8ecee2a81e0..916818005ad32 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -199,9 +199,6 @@ void DebugInfoFinder::processCompileUnit(DICompileUnit *CU) {
void DebugInfoFinder::processInstruction(const Module &M,
const Instruction &I) {
- if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I))
- processVariable(DVI->getVariable());
-
if (auto DbgLoc = I.getDebugLoc())
processLocation(M, DbgLoc.get());
diff --git a/llvm/lib/IR/User.cpp b/llvm/lib/IR/User.cpp
index ab44cb4b8a3f7..2df49271a3722 100644
--- a/llvm/lib/IR/User.cpp
+++ b/llvm/lib/IR/User.cpp
@@ -33,12 +33,6 @@ bool User::replaceUsesOfWith(Value *From, Value *To) {
setOperand(i, To);
Changed = true;
}
- if (auto DVI = dyn_cast_or_null<DbgVariableIntrinsic>(this)) {
- if (is_contained(DVI->location_ops(), From)) {
- DVI->replaceVariableLocationOp(From, To);
- Changed = true;
- }
- }
return Changed;
}
diff --git a/llvm/lib/Transforms/Scalar/ADCE.cpp b/llvm/lib/Transforms/Scalar/ADCE.cpp
index 985b9c0e53125..96d98e0db7410 100644
--- a/llvm/lib/Transforms/Scalar/ADCE.cpp
+++ b/llvm/lib/Transforms/Scalar/ADCE.cpp
@@ -517,18 +517,18 @@ ADCEChanged AggressiveDeadCodeElimination::removeDeadInstructions() {
if (isLive(&I))
continue;
- if (auto *DII = dyn_cast<DbgVariableIntrinsic>(&I)) {
+ for (DbgVariableRecord &DV : llvm::filterDbgVars(I.getDbgRecordRange())) {
// Check if the scope of this variable location is alive.
- if (AliveScopes.count(DII->getDebugLoc()->getScope()))
+ if (AliveScopes.count(DV.getDebugLoc()->getScope()))
continue;
// If intrinsic is pointing at a live SSA value, there may be an
// earlier optimization bug: if we know the location of the variable,
// why isn't the scope of the location alive?
- for (Value *V : DII->location_ops()) {
+ for (Value *V : DV.location_ops()) {
if (Instruction *II = dyn_cast<Instruction>(V)) {
if (isLive(II)) {
- dbgs() << "Dropping debug info for " << *DII << "\n";
+ dbgs() << "Dropping debug info for " << DV << "\n";
break;
}
}
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index babd7f6b3a058..17c9ff866c438 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2056,9 +2056,8 @@ void llvm::salvageDebugInfoForDbgValues(Instruction &I,
bool StackValue =
DVR->getType() != DbgVariableRecord::LocationType::Declare;
auto DVRLocation = DVR->location_ops();
- assert(
- is_contained(DVRLocation, &I) &&
- "DbgVariableIntrinsic must use salvaged instruction as its location");
+ assert(is_contained(DVRLocation, &I) &&
+ "DbgRecord must use salvaged instruction as its location");
SmallVector<Value *, 4> AdditionalValues;
// 'I' may appear more than once in DVR's location ops, and each use of 'I'
// must be updated in the DIExpression and potentially have additional
diff --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp
index 0065615cbfe13..1ef2d08116547 100644
--- a/llvm/unittests/IR/DebugInfoTest.cpp
+++ b/llvm/unittests/IR/DebugInfoTest.cpp
@@ -236,17 +236,15 @@ TEST(MetadataTest, GlobalConstantMetadataUsedByDbgRecord) {
EXPECT_FALSE(isa<UndefValue>(DVRs[0]->getValue(0)));
}
-TEST(DbgVariableIntrinsic, EmptyMDIsKillLocation) {
+TEST(MetadataTest, EmptyMDIsKillLocation) {
LLVMContext Ctx;
std::unique_ptr<Module> M = parseIR(Ctx, R"(
define dso_local void @fun() local_unnamed_addr #0 !dbg !9 {
entry:
- call void @llvm.dbg.declare(metadata !{}, metadata !13, metadata !DIExpression()), !dbg !16
+ #dbg_declare(!{}, !13, !DIExpression(), !16)
ret void, !dbg !16
}
- declare void @llvm.dbg.declare(metadata, metadata, metadata)
-
!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!2, !3}
!llvm.ident = !{!8}
diff --git a/llvm/unittests/Transforms/Utils/LocalTest.cpp b/llvm/unittests/Transforms/Utils/LocalTest.cpp
index 0c70feb64e7e4..350975e0d0a81 100644
--- a/llvm/unittests/Transforms/Utils/LocalTest.cpp
+++ b/llvm/unittests/Transforms/Utils/LocalTest.cpp
@@ -679,7 +679,6 @@ TEST(Local, FindDbgRecords) {
findDbgUsers(Arg, Records);
EXPECT_EQ(Records.size(), 1u);
- SmallVector<DbgValueInst *> Vals;
Records.clear();
// Arg (%a) is used twice by a single dbg_assign. Check findDbgValues returns
// only 1 pointer to it rather than 2.
|
@llvm/pr-subscribers-debuginfo Author: Jeremy Morse (jmorse) ChangesHere's some more dead code that will never run, due to debug intrinsics being decomissioned. I've also replaced one or two generic lambdas with DbgVariableRecord taking ones. Full diff: https://github.com/llvm/llvm-project/pull/150631.diff 8 Files Affected:
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index dc5dfab4418e5..ea434fc29404f 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2238,17 +2238,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
return true;
}
case Intrinsic::dbg_assign:
- // A dbg.assign is a dbg.value with more information about stack locations,
- // typically produced during optimisation of variables with leaked
- // addresses. We can treat it like a normal dbg_value intrinsic here; to
- // benefit from the full analysis of stack/SSA locations, GlobalISel would
- // need to register for and use the AssignmentTrackingAnalysis pass.
- [[fallthrough]];
case Intrinsic::dbg_value: {
- // This form of DBG_VALUE is target-independent.
- const DbgValueInst &DI = cast<DbgValueInst>(CI);
- translateDbgValueRecord(DI.getValue(), DI.hasArgList(), DI.getVariable(),
- DI.getExpression(), DI.getDebugLoc(), MIRBuilder);
+ llvm_unreachable("Saw debug intrinsic, should no longer exist");
return true;
}
case Intrinsic::uadd_with_overflow:
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 840ca8364e218..834f7944b6f8b 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -6940,7 +6940,7 @@ static void FixupDebugInfoForOutlinedFunction(
return NewVar;
};
- auto UpdateDebugRecord = [&](auto *DR) {
+ auto UpdateDebugRecord = [&](DbgVariableRecord *DR) {
DILocalVariable *OldVar = DR->getVariable();
unsigned ArgNo = 0;
for (auto Loc : DR->location_ops()) {
@@ -6957,9 +6957,6 @@ static void FixupDebugInfoForOutlinedFunction(
// The location and scope of variable intrinsics and records still point to
// the parent function of the target region. Update them.
for (Instruction &I : instructions(Func)) {
- if (auto *DDI = dyn_cast<llvm::DbgVariableIntrinsic>(&I))
- UpdateDebugRecord(DDI);
-
for (DbgVariableRecord &DVR : filterDbgVars(I.getDbgRecordRange()))
UpdateDebugRecord(&DVR);
}
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index ab8ecee2a81e0..916818005ad32 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -199,9 +199,6 @@ void DebugInfoFinder::processCompileUnit(DICompileUnit *CU) {
void DebugInfoFinder::processInstruction(const Module &M,
const Instruction &I) {
- if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I))
- processVariable(DVI->getVariable());
-
if (auto DbgLoc = I.getDebugLoc())
processLocation(M, DbgLoc.get());
diff --git a/llvm/lib/IR/User.cpp b/llvm/lib/IR/User.cpp
index ab44cb4b8a3f7..2df49271a3722 100644
--- a/llvm/lib/IR/User.cpp
+++ b/llvm/lib/IR/User.cpp
@@ -33,12 +33,6 @@ bool User::replaceUsesOfWith(Value *From, Value *To) {
setOperand(i, To);
Changed = true;
}
- if (auto DVI = dyn_cast_or_null<DbgVariableIntrinsic>(this)) {
- if (is_contained(DVI->location_ops(), From)) {
- DVI->replaceVariableLocationOp(From, To);
- Changed = true;
- }
- }
return Changed;
}
diff --git a/llvm/lib/Transforms/Scalar/ADCE.cpp b/llvm/lib/Transforms/Scalar/ADCE.cpp
index 985b9c0e53125..96d98e0db7410 100644
--- a/llvm/lib/Transforms/Scalar/ADCE.cpp
+++ b/llvm/lib/Transforms/Scalar/ADCE.cpp
@@ -517,18 +517,18 @@ ADCEChanged AggressiveDeadCodeElimination::removeDeadInstructions() {
if (isLive(&I))
continue;
- if (auto *DII = dyn_cast<DbgVariableIntrinsic>(&I)) {
+ for (DbgVariableRecord &DV : llvm::filterDbgVars(I.getDbgRecordRange())) {
// Check if the scope of this variable location is alive.
- if (AliveScopes.count(DII->getDebugLoc()->getScope()))
+ if (AliveScopes.count(DV.getDebugLoc()->getScope()))
continue;
// If intrinsic is pointing at a live SSA value, there may be an
// earlier optimization bug: if we know the location of the variable,
// why isn't the scope of the location alive?
- for (Value *V : DII->location_ops()) {
+ for (Value *V : DV.location_ops()) {
if (Instruction *II = dyn_cast<Instruction>(V)) {
if (isLive(II)) {
- dbgs() << "Dropping debug info for " << *DII << "\n";
+ dbgs() << "Dropping debug info for " << DV << "\n";
break;
}
}
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index babd7f6b3a058..17c9ff866c438 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2056,9 +2056,8 @@ void llvm::salvageDebugInfoForDbgValues(Instruction &I,
bool StackValue =
DVR->getType() != DbgVariableRecord::LocationType::Declare;
auto DVRLocation = DVR->location_ops();
- assert(
- is_contained(DVRLocation, &I) &&
- "DbgVariableIntrinsic must use salvaged instruction as its location");
+ assert(is_contained(DVRLocation, &I) &&
+ "DbgRecord must use salvaged instruction as its location");
SmallVector<Value *, 4> AdditionalValues;
// 'I' may appear more than once in DVR's location ops, and each use of 'I'
// must be updated in the DIExpression and potentially have additional
diff --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp
index 0065615cbfe13..1ef2d08116547 100644
--- a/llvm/unittests/IR/DebugInfoTest.cpp
+++ b/llvm/unittests/IR/DebugInfoTest.cpp
@@ -236,17 +236,15 @@ TEST(MetadataTest, GlobalConstantMetadataUsedByDbgRecord) {
EXPECT_FALSE(isa<UndefValue>(DVRs[0]->getValue(0)));
}
-TEST(DbgVariableIntrinsic, EmptyMDIsKillLocation) {
+TEST(MetadataTest, EmptyMDIsKillLocation) {
LLVMContext Ctx;
std::unique_ptr<Module> M = parseIR(Ctx, R"(
define dso_local void @fun() local_unnamed_addr #0 !dbg !9 {
entry:
- call void @llvm.dbg.declare(metadata !{}, metadata !13, metadata !DIExpression()), !dbg !16
+ #dbg_declare(!{}, !13, !DIExpression(), !16)
ret void, !dbg !16
}
- declare void @llvm.dbg.declare(metadata, metadata, metadata)
-
!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!2, !3}
!llvm.ident = !{!8}
diff --git a/llvm/unittests/Transforms/Utils/LocalTest.cpp b/llvm/unittests/Transforms/Utils/LocalTest.cpp
index 0c70feb64e7e4..350975e0d0a81 100644
--- a/llvm/unittests/Transforms/Utils/LocalTest.cpp
+++ b/llvm/unittests/Transforms/Utils/LocalTest.cpp
@@ -679,7 +679,6 @@ TEST(Local, FindDbgRecords) {
findDbgUsers(Arg, Records);
EXPECT_EQ(Records.size(), 1u);
- SmallVector<DbgValueInst *> Vals;
Records.clear();
// Arg (%a) is used twice by a single dbg_assign. Check findDbgValues returns
// only 1 pointer to it rather than 2.
|
const DbgValueInst &DI = cast<DbgValueInst>(CI); | ||
translateDbgValueRecord(DI.getValue(), DI.hasArgList(), DI.getVariable(), | ||
DI.getExpression(), DI.getDebugLoc(), MIRBuilder); | ||
llvm_unreachable("Saw debug intrinsic, should no longer exist"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the dead return after
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM when @arsenm's comment is resolved and he's happy
Here's some more dead code that will never run, due to debug intrinsics being decomissioned. I've also replaced one or two generic lambdas with DbgVariableRecord taking ones.