[llvm] 2de23c8 - [DebugInfo at O2][Utils] Undef instead of delete dbg.values in helper func
via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 25 03:05:28 PST 2019
Author: OCHyams
Date: 2019-11-25T10:55:14Z
New Revision: 2de23c8364babb49fe39d81048cd304a5ac2934e
URL: https://github.com/llvm/llvm-project/commit/2de23c8364babb49fe39d81048cd304a5ac2934e
DIFF: https://github.com/llvm/llvm-project/commit/2de23c8364babb49fe39d81048cd304a5ac2934e.diff
LOG: [DebugInfo at O2][Utils] Undef instead of delete dbg.values in helper func
Summary:
Related bug: https://bugs.llvm.org/show_bug.cgi?id=40648
Static helper function rewriteDebugUsers in Local.cpp deletes dbg.value
intrinsics when it cannot move or rewrite them, or salvage the deleted
instruction's value. It should instead undef them in this case.
This patch fixes that and I've added a test which covers the failing test
case in bz40648. I've updated the unit test Local.ReplaceAllDbgUsesWith
to check for this behaviour (and fixed a typo in the test which would
cause the old test to always pass).
Reviewers: aprantl, vsk, djtodoro, probinson
Reviewed By: vsk
Subscribers: hiraditya, llvm-commits
Tags: #debug-info, #llvm
Differential Revision: https://reviews.llvm.org/D70604
Added:
llvm/test/DebugInfo/X86/dbg-value-dropped-instcombine.ll
Modified:
llvm/lib/Transforms/Utils/Local.cpp
llvm/unittests/Transforms/Utils/LocalTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index aa1341eea056..eaccaa1c4ac5 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1732,7 +1732,7 @@ DIExpression *llvm::salvageDebugInfoImpl(Instruction &I,
using DbgValReplacement = Optional<DIExpression *>;
/// Point debug users of \p From to \p To using exprs given by \p RewriteExpr,
-/// possibly moving/deleting users to prevent use-before-def. Returns true if
+/// possibly moving/undefing users to prevent use-before-def. Returns true if
/// changes are made.
static bool rewriteDebugUsers(
Instruction &From, Value &To, Instruction &DomPoint, DominatorTree &DT,
@@ -1745,7 +1745,7 @@ static bool rewriteDebugUsers(
// Prevent use-before-def of To.
bool Changed = false;
- SmallPtrSet<DbgVariableIntrinsic *, 1> DeleteOrSalvage;
+ SmallPtrSet<DbgVariableIntrinsic *, 1> UndefOrSalvage;
if (isa<Instruction>(&To)) {
bool DomPointAfterFrom = From.getNextNonDebugInstruction() == &DomPoint;
@@ -1760,14 +1760,14 @@ static bool rewriteDebugUsers(
// Users which otherwise aren't dominated by the replacement value must
// be salvaged or deleted.
} else if (!DT.dominates(&DomPoint, DII)) {
- DeleteOrSalvage.insert(DII);
+ UndefOrSalvage.insert(DII);
}
}
}
// Update debug users without use-before-def risk.
for (auto *DII : Users) {
- if (DeleteOrSalvage.count(DII))
+ if (UndefOrSalvage.count(DII))
continue;
LLVMContext &Ctx = DII->getContext();
@@ -1781,18 +1781,10 @@ static bool rewriteDebugUsers(
Changed = true;
}
- if (!DeleteOrSalvage.empty()) {
+ if (!UndefOrSalvage.empty()) {
// Try to salvage the remaining debug users.
- Changed |= salvageDebugInfo(From);
-
- // Delete the debug users which weren't salvaged.
- for (auto *DII : DeleteOrSalvage) {
- if (DII->getVariableLocation() == &From) {
- LLVM_DEBUG(dbgs() << "Erased UseBeforeDef: " << *DII << '\n');
- DII->eraseFromParent();
- Changed = true;
- }
- }
+ salvageDebugInfoOrMarkUndef(From);
+ Changed = true;
}
return Changed;
diff --git a/llvm/test/DebugInfo/X86/dbg-value-dropped-instcombine.ll b/llvm/test/DebugInfo/X86/dbg-value-dropped-instcombine.ll
new file mode 100644
index 000000000000..18e51b6fabd4
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/dbg-value-dropped-instcombine.ll
@@ -0,0 +1,76 @@
+; RUN: opt -instcombine -S %s -o - | FileCheck %s
+
+; In pr40648 one of two dbg.values used to describe the variable bumble was
+; being dropped by instcombine. Test that both dbg.values survive instcombine.
+
+; $ clang -O0 -Xclang -disable-O0-optnone -g bees.c -emit-llvm -S -o - \
+; | opt -opt-bisect-limit=10 -O2 -o -
+; $ cat bees.c
+; struct bees {
+; int a;
+; int b;
+; }
+
+; int global = 0;
+
+; struct bees foo(struct bees bumble)
+; {
+; global = 1;
+; int temp = bumble.a + bumble.b;
+; return bumble;
+; }
+
+; CHECK: define dso_local i64 @foo
+; CHECK: @llvm.dbg.value({{.*}}, metadata ![[BEE:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)),
+; CHECK: @llvm.dbg.value(metadata i32 undef, metadata ![[BEE]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32)),
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-unknown"
+
+ at global = dso_local local_unnamed_addr global i32 0, align 4, !dbg !0
+
+define dso_local i64 @foo(i64 %bumble.coerce) local_unnamed_addr !dbg !11 {
+entry:
+ %bumble.sroa.0.0.extract.trunc = trunc i64 %bumble.coerce to i32
+ call void @llvm.dbg.value(metadata i32 %bumble.sroa.0.0.extract.trunc, metadata !18, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)), !dbg !19
+ %bumble.sroa.3.0.extract.shift = lshr i64 %bumble.coerce, 32
+ %bumble.sroa.3.0.extract.trunc = trunc i64 %bumble.sroa.3.0.extract.shift to i32
+ call void @llvm.dbg.value(metadata i32 %bumble.sroa.3.0.extract.trunc, metadata !18, metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32)), !dbg !19
+ store i32 1, i32* @global, align 4, !dbg !20
+ call void @llvm.dbg.value(metadata i32 undef, metadata !21, metadata !DIExpression()), !dbg !19
+ %retval.sroa.2.0.insert.ext = zext i32 %bumble.sroa.3.0.extract.trunc to i64, !dbg !22
+ %retval.sroa.2.0.insert.shift = shl i64 %retval.sroa.2.0.insert.ext, 32, !dbg !22
+ %retval.sroa.0.0.insert.ext = zext i32 %bumble.sroa.0.0.extract.trunc to i64, !dbg !22
+ %retval.sroa.0.0.insert.insert = or i64 %retval.sroa.2.0.insert.shift, %retval.sroa.0.0.insert.ext, !dbg !22
+ ret i64 %retval.sroa.0.0.insert.insert, !dbg !22
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!7, !8, !9}
+!llvm.ident = !{!10}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "global", scope: !2, file: !3, line: 6, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 10.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: None)
+!3 = !DIFile(filename: "bees.c", directory: "/")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!7 = !{i32 2, !"Dwarf Version", i32 4}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+!9 = !{i32 1, !"wchar_size", i32 4}
+!10 = !{!"clang version 10.0.0"}
+!11 = distinct !DISubprogram(name: "foo", scope: !3, file: !3, line: 8, type: !12, scopeLine: 9, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !4)
+!12 = !DISubroutineType(types: !13)
+!13 = !{!14, !14}
+!14 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "bees", file: !3, line: 1, size: 64, elements: !15)
+!15 = !{!16, !17}
+!16 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !14, file: !3, line: 2, baseType: !6, size: 32)
+!17 = !DIDerivedType(tag: DW_TAG_member, name: "b", scope: !14, file: !3, line: 3, baseType: !6, size: 32, offset: 32)
+!18 = !DILocalVariable(name: "bumble", arg: 1, scope: !11, file: !3, line: 8, type: !14)
+!19 = !DILocation(line: 0, scope: !11)
+!20 = !DILocation(line: 10, column: 10, scope: !11)
+!21 = !DILocalVariable(name: "temp", scope: !11, file: !3, line: 11, type: !6)
+!22 = !DILocation(line: 12, column: 3, scope: !11)
diff --git a/llvm/unittests/Transforms/Utils/LocalTest.cpp b/llvm/unittests/Transforms/Utils/LocalTest.cpp
index 1f67a1ec84c7..99713d5817fe 100644
--- a/llvm/unittests/Transforms/Utils/LocalTest.cpp
+++ b/llvm/unittests/Transforms/Utils/LocalTest.cpp
@@ -759,11 +759,14 @@ TEST(Local, ReplaceAllDbgUsesWith) {
auto *ADbgVal = cast<DbgValueInst>(A.getNextNode());
EXPECT_EQ(ConstantInt::get(A.getType(), 0), ADbgVal->getVariableLocation());
- // Introduce a use-before-def. Check that the dbg.values for %f are deleted.
+ // Introduce a use-before-def. Check that the dbg.values for %f become undef.
EXPECT_TRUE(replaceAllDbgUsesWith(F_, G, G, DT));
+ auto *FDbgVal = cast<DbgValueInst>(F_.getNextNode());
+ EXPECT_TRUE(isa<UndefValue>(FDbgVal->getVariableLocation()));
+
SmallVector<DbgValueInst *, 1> FDbgVals;
- findDbgValues(FDbgVals, &F);
+ findDbgValues(FDbgVals, &F_);
EXPECT_EQ(0U, FDbgVals.size());
// Simulate i32 -> i64 conversion to test sign-extension. Here are some
More information about the llvm-commits
mailing list