From 970574c28ec4b3e1e56ff83c03e29d129db5626f Mon Sep 17 00:00:00 2001 From: Vedant Kumar Date: Thu, 7 Nov 2019 11:19:41 -0800 Subject: [PATCH] Wrong debug info generated at -O2 (-O0 is correct) Instcombiner pass was erasing trivially dead instruction without updating dependent llvm.dbg.value. which was not showing programmer current state of variables while debugging. As a part of this fix I did following, Iterate throught all the users (llvm.dbg) of a instruction which is trivially dead and set each if them undef, Before deleting the instruction. Now user will see optimized out, when try to print those variables. This fixes https://bugs.llvm.org/show_bug.cgi?id=43893 This is my first fix to llvm. Patch by kamlesh kumar! Differential Revision: https://reviews.llvm.org/D69809 --- include/llvm/Transforms/Utils/Local.h | 4 ++ .../InstCombine/InstCombineInternal.h | 2 +- .../InstCombine/InstructionCombining.cpp | 3 +- lib/Transforms/Utils/Local.cpp | 5 ++ .../Transforms/InstCombine/cast-mul-select.ll | 2 + test/Transforms/InstCombine/debuginfo-dce.ll | 2 +- test/Transforms/InstCombine/pr43893.ll | 54 +++++++++++++++++++ .../InstCombine/stacksave-debuginfo.ll | 9 ++-- 8 files changed, 73 insertions(+), 8 deletions(-) create mode 100644 test/Transforms/InstCombine/pr43893.ll diff --git a/include/llvm/Transforms/Utils/Local.h b/include/llvm/Transforms/Utils/Local.h index 9fcb2f64d79..d1dc0b3e46b 100644 --- a/include/llvm/Transforms/Utils/Local.h +++ b/include/llvm/Transforms/Utils/Local.h @@ -354,6 +354,10 @@ AllocaInst *findAllocaForValue(Value *V, /// Returns true if any debug users were updated. bool salvageDebugInfo(Instruction &I); +/// Salvage all debug users of the instruction \p I or mark it as undef if it +/// cannot be salvaged. +void salvageDebugInfoOrMarkUndef(Instruction &I); + /// Implementation of salvageDebugInfo, applying only to instructions in /// \p Insns, rather than all debug users of \p I. bool salvageDebugInfoForDbgValues(Instruction &I, diff --git a/lib/Transforms/InstCombine/InstCombineInternal.h b/lib/Transforms/InstCombine/InstCombineInternal.h index 1dbc06d92e7..aabedbda7e6 100644 --- a/lib/Transforms/InstCombine/InstCombineInternal.h +++ b/lib/Transforms/InstCombine/InstCombineInternal.h @@ -705,7 +705,7 @@ public: Instruction *eraseInstFromFunction(Instruction &I) { LLVM_DEBUG(dbgs() << "IC: ERASE " << I << '\n'); assert(I.use_empty() && "Cannot erase instruction that is used!"); - salvageDebugInfo(I); + salvageDebugInfoOrMarkUndef(I); // Make sure that we reprocess all operands now that we reduced their // use counts. diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp index a3dfbfaf8dc..dd2907a05fe 100644 --- a/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -3414,8 +3414,7 @@ static bool AddReachableCodeToWorklist(BasicBlock *BB, const DataLayout &DL, if (isInstructionTriviallyDead(Inst, TLI)) { ++NumDeadInst; LLVM_DEBUG(dbgs() << "IC: DCE: " << *Inst << '\n'); - if (!salvageDebugInfo(*Inst)) - replaceDbgUsesWithUndef(Inst); + salvageDebugInfoOrMarkUndef(*Inst); Inst->eraseFromParent(); MadeIRChange = true; continue; diff --git a/lib/Transforms/Utils/Local.cpp b/lib/Transforms/Utils/Local.cpp index 5bcd05757ec..d5ae13ad285 100644 --- a/lib/Transforms/Utils/Local.cpp +++ b/lib/Transforms/Utils/Local.cpp @@ -1611,6 +1611,11 @@ bool llvm::salvageDebugInfo(Instruction &I) { return salvageDebugInfoForDbgValues(I, DbgUsers); } +void llvm::salvageDebugInfoOrMarkUndef(Instruction &I) { + if (!salvageDebugInfo(I)) + replaceDbgUsesWithUndef(&I); +} + bool llvm::salvageDebugInfoForDbgValues( Instruction &I, ArrayRef DbgUsers) { auto &Ctx = I.getContext(); diff --git a/test/Transforms/InstCombine/cast-mul-select.ll b/test/Transforms/InstCombine/cast-mul-select.ll index c501fd8d04c..5de3314b472 100644 --- a/test/Transforms/InstCombine/cast-mul-select.ll +++ b/test/Transforms/InstCombine/cast-mul-select.ll @@ -13,6 +13,8 @@ define i32 @mul(i32 %x, i32 %y) { ; we preserve the debug information in the resulting ; instruction. ; DBGINFO-LABEL: @mul( +; DBGINFO-NEXT: call void @llvm.dbg.value(metadata i8 undef +; DBGINFO-NEXT: call void @llvm.dbg.value(metadata i8 undef ; DBGINFO-NEXT: [[C:%.*]] = mul i32 {{.*}} ; DBGINFO-NEXT: [[D:%.*]] = and i32 {{.*}} ; DBGINFO-NEXT: call void @llvm.dbg.value(metadata i32 [[C]] diff --git a/test/Transforms/InstCombine/debuginfo-dce.ll b/test/Transforms/InstCombine/debuginfo-dce.ll index 200ea26cdaf..5b10f7ba435 100644 --- a/test/Transforms/InstCombine/debuginfo-dce.ll +++ b/test/Transforms/InstCombine/debuginfo-dce.ll @@ -34,7 +34,7 @@ entry: call void @llvm.dbg.value(metadata %struct.entry* %1, metadata !18, metadata !20), !dbg !19 ; CHECK: define void @salvage_load ; CHECK-NEXT: entry: -; CHECK-NOT: dbg.value +; CHECK-NEXT: call void @llvm.dbg.value(metadata %struct.entry* undef store %struct.entry* %1, %struct.entry** %im_not_dead, align 8 ret void, !dbg !21 } diff --git a/test/Transforms/InstCombine/pr43893.ll b/test/Transforms/InstCombine/pr43893.ll new file mode 100644 index 00000000000..0eb926b19a6 --- /dev/null +++ b/test/Transforms/InstCombine/pr43893.ll @@ -0,0 +1,54 @@ +; Check for setting dbg.value as undef which depends on trivially dead instructions. +; RUN: opt -instcombine -S -o - %s | FileCheck %s + +@a = common dso_local global i8 0, align 1, !dbg !0 +@b = common dso_local global i8 0, align 1, !dbg !6 + +define dso_local i32 @main() !dbg !13 { +entry: + %0 = load i8, i8* @a, align 1, !dbg !17 + %dec = add i8 %0, -1, !dbg !17 + store i8 %dec, i8* @a, align 1, !dbg !17 +;CHECK: call void @llvm.dbg.value(metadata i32 undef +;CHECK: call void @llvm.dbg.value(metadata i32 -8 +;CHECK: call void @llvm.dbg.value(metadata i32 undef + %conv = sext i8 %dec to i32, !dbg !17 + call void @llvm.dbg.value(metadata i32 %conv, metadata !18, metadata !DIExpression()), !dbg !19 + call void @llvm.dbg.value(metadata i32 -8, metadata !20, metadata !DIExpression()), !dbg !19 + call void @llvm.dbg.value(metadata i32 %conv, metadata !20, metadata !DIExpression()), !dbg !19 + store i8 0, i8* @b, align 1, !dbg !21 + %cmp = icmp sgt i32 %conv, 0, !dbg !22 + %conv1 = zext i1 %cmp to i32, !dbg !22 + ret i32 0, !dbg !23 +} + +declare void @llvm.dbg.value(metadata, metadata, metadata) + +!llvm.dbg.cu = !{!2} +!llvm.module.flags = !{!9, !10, !11} +!llvm.ident = !{!12} + +!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression()) +!1 = distinct !DIGlobalVariable(name: "a", scope: !2, file: !3, line: 1, type: !8, 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: "test", directory: "n") +!4 = !{} +!5 = !{!0, !6} +!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression()) +!7 = distinct !DIGlobalVariable(name: "b", scope: !2, file: !3, line: 1, type: !8, isLocal: false, isDefinition: true) +!8 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char) +!9 = !{i32 2, !"Dwarf Version", i32 4} +!10 = !{i32 2, !"Debug Info Version", i32 3} +!11 = !{i32 1, !"wchar_size", i32 4} +!12 = !{!"clang version 10.0.0"} +!13 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 2, type: !14, scopeLine: 2, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !4) +!14 = !DISubroutineType(types: !15) +!15 = !{!16} +!16 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) +!17 = !DILocation(line: 4, column: 11, scope: !13) +!18 = !DILocalVariable(name: "c", scope: !13, file: !3, line: 4, type: !16) +!19 = !DILocation(line: 0, scope: !13) +!20 = !DILocalVariable(name: "l_1240", scope: !13, file: !3, line: 6, type: !16) +!21 = !DILocation(line: 10, column: 10, scope: !13) +!22 = !DILocation(line: 10, column: 5, scope: !13) +!23 = !DILocation(line: 12, column: 1, scope: !13) diff --git a/test/Transforms/InstCombine/stacksave-debuginfo.ll b/test/Transforms/InstCombine/stacksave-debuginfo.ll index 3c31c4c78a9..693ed870a86 100644 --- a/test/Transforms/InstCombine/stacksave-debuginfo.ll +++ b/test/Transforms/InstCombine/stacksave-debuginfo.ll @@ -8,10 +8,11 @@ declare void @llvm.stackrestore(i8*) #0 define i32* @test1(i32 %P) !dbg !6 { ; CHECK-LABEL: @test1( -; CHECK-NEXT: [[TMP1:%.*]] = zext i32 [[P:%.*]] to i64, !dbg !12 -; CHECK-NEXT: [[A:%.*]] = alloca i32, i64 [[TMP1]], align 4, !dbg !12 -; CHECK-NEXT: call void @llvm.dbg.value(metadata i32* [[A]], metadata !11, metadata !DIExpression()), !dbg !12 -; CHECK-NEXT: ret i32* [[A]], !dbg !13 +; CHECK-NEXT: call void @llvm.dbg.value(metadata i8* undef +; CHECK-NEXT: [[TMP1:%.*]] = zext i32 [[P:%.*]] to i64, !dbg !13 +; CHECK-NEXT: [[A:%.*]] = alloca i32, i64 [[TMP1]], align 4, !dbg !13 +; CHECK-NEXT: call void @llvm.dbg.value(metadata i32* [[A]] +; CHECK-NEXT: ret i32* [[A]], !dbg !14 ; %tmp = call i8* @llvm.stacksave(), !dbg !12 call void @llvm.dbg.value(metadata i8* %tmp, metadata !9, metadata !DIExpression()), !dbg !12