From 9a24e1a7cd0e7d24f0eaad8096617fdbfbc67fda Mon Sep 17 00:00:00 2001 From: Carlos Alberto Enciso Date: Thu, 25 Oct 2018 09:58:59 +0000 Subject: [PATCH] [DebugInfo][Dexter] Unreachable line stepped onto after SimplifyCFG. When SimplifyCFG changes the PHI node into a select instruction, the debug line records becomes ambiguous. It causes the debugger to display unreachable source lines. Differential Revision: https://reviews.llvm.org/D53287 llvm-svn: 345250 --- llvm/include/llvm/Transforms/Utils/Local.h | 9 ++ llvm/lib/Transforms/Utils/Local.cpp | 41 +++++++++ llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 22 +---- llvm/test/CodeGen/X86/pr38762.ll | 101 ++++++++++++++++++++++ llvm/test/CodeGen/X86/pr38763.ll | 20 ++--- llvm/test/CodeGen/X86/pr39243.ll | 132 +++++++++++++++++++++++++++++ 6 files changed, 296 insertions(+), 29 deletions(-) create mode 100644 llvm/test/CodeGen/X86/pr38762.ll create mode 100644 llvm/test/CodeGen/X86/pr39243.ll diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h index f7da696..86a32bb 100644 --- a/llvm/include/llvm/Transforms/Utils/Local.h +++ b/llvm/include/llvm/Transforms/Utils/Local.h @@ -446,6 +446,15 @@ void copyRangeMetadata(const DataLayout &DL, const LoadInst &OldLI, MDNode *N, /// Remove the debug intrinsic instructions for the given instruction. void dropDebugUsers(Instruction &I); +/// Hoist all of the instructions in the \p IfBlock to the dominant block +/// \p DomBlock, by moving its instructions to the insertion point \p InsertPt. +/// +/// The moved instructions receive the insertion point debug location values +/// (DILocations) and their debug intrinsic instructions (dbg.values) are +/// removed. +void hoistAllInstructionsInto(BasicBlock *DomBlock, Instruction *InsertPt, + BasicBlock *BB); + //===----------------------------------------------------------------------===// // Intrinsic pattern matching // diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index 1153f3c..82fb765 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -2529,6 +2529,47 @@ void llvm::dropDebugUsers(Instruction &I) { DII->eraseFromParent(); } +void llvm::hoistAllInstructionsInto(BasicBlock *DomBlock, Instruction *InsertPt, + BasicBlock *BB) { + // Since we are moving the instructions out of its basic block, we do not + // retain their original debug locations (DILocations) and debug intrinsic + // instructions (dbg.values). + // + // Doing so would degrade the debugging experience and adversely affect the + // accuracy of profiling information. + // + // Currently, when hoisting the instructions, we take the following actions: + // - Remove their dbg.values. + // - Set their debug locations to the values from the insertion point. + // + // As per PR39141 (comment #8), the more fundamental reason why the dbg.values + // need to be deleted, is because there will not be any instructions with a + // DILocation in either branch left after performing the transformation. We + // can only insert a dbg.value after the two branches are joined again. + // + // See PR38762, PR39243 for more details. + // + // TODO: Extend llvm.dbg.value to take more than one SSA Value (PR39141) to + // encode predicated DIExpressions that yield different results on different + // code paths. + for (BasicBlock::iterator II = BB->begin(), IE = BB->end(); II != IE;) { + Instruction *I = &*II; + I->dropUnknownNonDebugMetadata(); + if (I->isUsedByMetadata()) + dropDebugUsers(*I); + if (isa(I)) { + // Remove DbgInfo Intrinsics. + II = I->eraseFromParent(); + continue; + } + I->setDebugLoc(InsertPt->getDebugLoc()); + ++II; + } + DomBlock->getInstList().splice(InsertPt->getIterator(), BB->getInstList(), + BB->begin(), + BB->getTerminator()->getIterator()); +} + namespace { /// A potential constituent of a bitreverse or bswap expression. See diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 8dad617..dd0d441 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -2375,24 +2375,10 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI, // Move all 'aggressive' instructions, which are defined in the // conditional parts of the if's up to the dominating block. - if (IfBlock1) { - for (auto &I : *IfBlock1) { - I.dropUnknownNonDebugMetadata(); - dropDebugUsers(I); - } - DomBlock->getInstList().splice(InsertPt->getIterator(), - IfBlock1->getInstList(), IfBlock1->begin(), - IfBlock1->getTerminator()->getIterator()); - } - if (IfBlock2) { - for (auto &I : *IfBlock2) { - I.dropUnknownNonDebugMetadata(); - dropDebugUsers(I); - } - DomBlock->getInstList().splice(InsertPt->getIterator(), - IfBlock2->getInstList(), IfBlock2->begin(), - IfBlock2->getTerminator()->getIterator()); - } + if (IfBlock1) + hoistAllInstructionsInto(DomBlock, InsertPt, IfBlock1); + if (IfBlock2) + hoistAllInstructionsInto(DomBlock, InsertPt, IfBlock2); while (PHINode *PN = dyn_cast(BB->begin())) { // Change the PHI node into a select instruction. diff --git a/llvm/test/CodeGen/X86/pr38762.ll b/llvm/test/CodeGen/X86/pr38762.ll new file mode 100644 index 0000000..dc4d535 --- /dev/null +++ b/llvm/test/CodeGen/X86/pr38762.ll @@ -0,0 +1,101 @@ +; RUN: opt < %s -S -simplifycfg | FileCheck %s + +; Note: This patch is a complement to pr38763. +; +; When SimplifyCFG changes the PHI node into a select instruction, the debug +; information becomes ambiguous. It causes the debugger to display unreached +; lines and invalid variable values. +; +; When in the debugger, on the line "if (read1 > 3)", and we step from the +; 'if' condition, onto the addition, then back to the 'if' again, which is +; misleading because that addition doesn't really "happen" (it's speculated). + +; IR generated with: +; clang -S -g -gno-column-info -O2 -emit-llvm pr38762.cpp -o pr38762.ll -mllvm -opt-bisect-limit=10 + +; // pr38762.cpp +; int main() { +; volatile int foo = 0; +; int read1 = foo; +; int brains = foo; +; +; if (read1 > 3) { +; brains *= 2; +; brains += 1; +; } +; +; return brains; +; } + +; Change the debug locations associated with the PHI nodes being promoted, to +; the debug locations from the insertion point in the dominant block. + +; CHECK-LABEL: entry +; CHECK: %cmp = icmp sgt i32 %foo.0., 3, !dbg !14 +; CHECK: %mul = shl nsw i32 %foo.0.5, 1, !dbg !16 +; CHECK-NOT: call void @llvm.dbg.value(metadata i32 %mul, metadata !15, metadata !DIExpression()), !dbg !25 +; CHECK: %add = or i32 %mul, 1, !dbg !16 +; CHECK-NOT: call void @llvm.dbg.value(metadata i32 %add, metadata !15, metadata !DIExpression()), !dbg !25 +; CHECK: %brains.0 = select i1 %cmp, i32 %add, i32 %foo.0.5, !dbg !16 + +; ModuleID = 'pr38762.cpp' +source_filename = "pr38762.cpp" +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-pc-linux-gnu" + +; Function Attrs: norecurse nounwind uwtable +define dso_local i32 @main() local_unnamed_addr #0 !dbg !7 { +entry: + %foo = alloca i32, align 4 + %foo.0..sroa_cast = bitcast i32* %foo to i8* + store volatile i32 0, i32* %foo, align 4 + %foo.0. = load volatile i32, i32* %foo, align 4 + %foo.0.5 = load volatile i32, i32* %foo, align 4 + call void @llvm.dbg.value(metadata i32 %foo.0.5, metadata !15, metadata !DIExpression()), !dbg !25 + %cmp = icmp sgt i32 %foo.0., 3, !dbg !26 + br i1 %cmp, label %if.then, label %if.end, !dbg !28 + +if.then: ; preds = %entry + %mul = shl nsw i32 %foo.0.5, 1, !dbg !29 + call void @llvm.dbg.value(metadata i32 %mul, metadata !15, metadata !DIExpression()), !dbg !25 + %add = or i32 %mul, 1, !dbg !31 + call void @llvm.dbg.value(metadata i32 %add, metadata !15, metadata !DIExpression()), !dbg !25 + br label %if.end, !dbg !32 + +if.end: ; preds = %if.then, %entry + %brains.0 = phi i32 [ %add, %if.then ], [ %foo.0.5, %entry ], !dbg !33 + call void @llvm.dbg.value(metadata i32 %brains.0, metadata !15, metadata !DIExpression()), !dbg !25 + ret i32 %brains.0, !dbg !35 +} + +declare void @llvm.dbg.value(metadata, metadata, metadata) #2 + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!3, !4, !5} +!llvm.ident = !{!6} + +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 8.0.0 (trunk 343753)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None) +!1 = !DIFile(filename: "pr38762.cpp", directory: ".") +!2 = !{} +!3 = !{i32 2, !"Dwarf Version", i32 4} +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = !{i32 1, !"wchar_size", i32 4} +!6 = !{!"clang version 8.0.0 (trunk 343753)"} +!7 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: true, unit: !0, retainedNodes: !11) +!8 = !DISubroutineType(types: !9) +!9 = !{!10} +!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) +!11 = !{!15} +!13 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !10) +!15 = !DILocalVariable(name: "brains", scope: !7, file: !1, line: 4, type: !10) +!25 = !DILocation(line: 4, scope: !7) +!26 = !DILocation(line: 6, scope: !27) +!27 = distinct !DILexicalBlock(scope: !7, file: !1, line: 6) +!28 = !DILocation(line: 6, scope: !7) +!29 = !DILocation(line: 7, scope: !30) +!30 = distinct !DILexicalBlock(scope: !27, file: !1, line: 6) +!31 = !DILocation(line: 8, scope: !30) +!32 = !DILocation(line: 9, scope: !30) +!33 = !DILocation(line: 0, scope: !7) +!34 = !DILocation(line: 12, scope: !7) +!35 = !DILocation(line: 11, scope: !7) diff --git a/llvm/test/CodeGen/X86/pr38763.ll b/llvm/test/CodeGen/X86/pr38763.ll index b36e1ef..ee08727 100644 --- a/llvm/test/CodeGen/X86/pr38763.ll +++ b/llvm/test/CodeGen/X86/pr38763.ll @@ -30,13 +30,13 @@ ; branches, as they becomes ambiguous. ; CHECK-LABEL: entry -; CHECK: %cmp = icmp eq i32 %foo.0., 4 -; CHECK: %add = add nsw i32 %foo.0.4, 2, !dbg !18 +; CHECK: %cmp = icmp eq i32 %foo.0., 4, !dbg !14 +; CHECK: %add = add nsw i32 %foo.0.4, 2, !dbg !16 ; CHECK-NOT: @llvm.dbg.value(metadata i32 %add -; CHECK: %sub = add nsw i32 %foo.0.4, -2, !dbg !21 +; CHECK: %sub = add nsw i32 %foo.0.4, -2, !dbg !16 ; CHECK-NOT: @llvm.dbg.value(metadata i32 %sub ; CHECK: %result.0 = select i1 %cmp, i32 %add, i32 %sub -; CHECK: call void @llvm.dbg.value(metadata i32 %result.0, metadata !12, metadata !DIExpression()), !dbg !17 +; CHECK: call void @llvm.dbg.value(metadata i32 %result.0, metadata !12, metadata !DIExpression()), !dbg !13 ; ModuleID = 'pr38763.cpp' source_filename = "pr38763.cpp" @@ -48,12 +48,12 @@ define dso_local i32 @main() local_unnamed_addr #0 !dbg !7 { entry: %foo = alloca i32, align 4 %foo.0..sroa_cast = bitcast i32* %foo to i8* - store volatile i32 4, i32* %foo, align 4, !tbaa !19 + store volatile i32 4, i32* %foo, align 4 %foo.0. = load volatile i32, i32* %foo, align 4 %foo.0.4 = load volatile i32, i32* %foo, align 4 call void @llvm.dbg.value(metadata i32 0, metadata !16, metadata !DIExpression()), !dbg !27 - %cmp = icmp eq i32 %foo.0., 4 - br i1 %cmp, label %if.then, label %if.else + %cmp = icmp eq i32 %foo.0., 4, !dbg !28 + br i1 %cmp, label %if.then, label %if.else, !dbg !30 if.then: ; preds = %entry %add = add nsw i32 %foo.0.4, 2, !dbg !31 @@ -91,12 +91,10 @@ declare void @llvm.dbg.value(metadata, metadata, metadata) #2 !10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) !11 = !{!16} !16 = !DILocalVariable(name: "result", scope: !7, file: !1, line: 6, type: !10) -!19 = !{!20, !20, i64 0} -!20 = !{!"int", !21, i64 0} -!21 = !{!"omnipotent char", !22, i64 0} -!22 = !{!"Simple C++ TBAA"} !27 = !DILocation(line: 6, column: 7, scope: !7) +!28 = !DILocation(line: 7, column: 12, scope: !29) !29 = distinct !DILexicalBlock(scope: !7, file: !1, line: 7, column: 7) +!30 = !DILocation(line: 7, column: 7, scope: !7) !31 = !DILocation(line: 8, column: 20, scope: !32) !32 = distinct !DILexicalBlock(scope: !29, file: !1, line: 7, column: 18) !34 = !DILocation(line: 10, column: 20, scope: !35) diff --git a/llvm/test/CodeGen/X86/pr39243.ll b/llvm/test/CodeGen/X86/pr39243.ll new file mode 100644 index 0000000..a901e29 --- /dev/null +++ b/llvm/test/CodeGen/X86/pr39243.ll @@ -0,0 +1,132 @@ +; RUN: opt < %s -S -simplifycfg | FileCheck %s + +; Note: This patch fixes the regression introduced by pr38762. +; +; When SimplifyCFG changes the PHI node into a select instruction, the debug +; information becomes ambiguous. It causes the debugger to display unreached +; lines and invalid variable values. +; +; When the function 'hoistAllInstructionsInto' hoist a basic block: +; - Remove their dbg.values. +; - Set their debug locations to the values from the insertion point. +; +; But, if one of the instructions being hoisted is a debug intrinsic from an +; inlined function, assigning it the debug location from the insertion point +; will create a mismatch between the intrinsic's subprogram and the location's +; subprogram, causing the assertion "Expected inlined-at fields to agree" in +; SelectionDAG". + +; IR generated with: +; clang -S -g -gno-column-info -O2 -emit-llvm pr39243.cpp -o pr39243.ll -mllvm -opt-bisect-limit=103 + +; // pr39243.cpp +; union onion { +; double dd; +; int ii[2]; +; }; +; +; int alpha; +; int bravo(); +; +; int charlie() { +; int delta = 0; +; return bravo(); +; } +; +; int echo(onion foxtrot) { +; alpha = foxtrot.ii[0]; +; if (alpha) { +; int golf = bravo(); +; return -golf; +; } +; alpha = foxtrot.ii[1]; +; return -charlie(); +; } + +; Change the debug locations associated with the PHI nodes being promoted, to +; the debug locations from the insertion point in the dominant block. + +; CHECK-LABEL: entry +; CHECK: %foxtrot.sroa.0.0.extract.trunc = trunc i64 %foxtrot.coerce to i32 +; CHECK: %tobool = icmp eq i32 %foxtrot.sroa.0.0.extract.trunc, 0 +; CHECK: %foxtrot.sroa.2.0.extract.shift = lshr i64 %foxtrot.coerce, 32 +; CHECK-NOT: call void @llvm.dbg.value(metadata i32 %foxtrot.sroa.2.0.extract.trunc, metadata !30, metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32)), !dbg !34 +; CHECK: %foxtrot.sroa.2.0.extract.trunc = trunc i64 %foxtrot.sroa.2.0.extract.shift to i32 +; CHECK: store i32 %foxtrot.sroa.2.0.extract.trunc, i32* @alpha, align 4, !dbg !25 +; CHECK-NOT: call void @llvm.dbg.value(metadata i32 0, metadata !15, metadata !DIExpression()), !dbg !43 + +; ModuleID = 'pr39243.cpp' +source_filename = "pr39243.cpp" +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-pc-linux-gnu" + +@alpha = dso_local local_unnamed_addr global i32 0, align 4, !dbg !0 + +define dso_local i32 @_Z7charliev() local_unnamed_addr #0 { +entry: + %call = tail call i32 @_Z5bravov() + ret i32 %call +} + +declare dso_local i32 @_Z5bravov() local_unnamed_addr #1 + +define dso_local i32 @_Z4echo5onion(i64 %foxtrot.coerce) local_unnamed_addr #0 !dbg !18 { +entry: + %foxtrot.sroa.0.0.extract.trunc = trunc i64 %foxtrot.coerce to i32 + store i32 %foxtrot.sroa.0.0.extract.trunc, i32* @alpha, align 4 + %tobool = icmp eq i32 %foxtrot.sroa.0.0.extract.trunc, 0 + br i1 %tobool, label %if.end, label %return + +if.end: ; preds = %entry + %foxtrot.sroa.2.0.extract.shift = lshr i64 %foxtrot.coerce, 32 + %foxtrot.sroa.2.0.extract.trunc = trunc i64 %foxtrot.sroa.2.0.extract.shift to i32 + call void @llvm.dbg.value(metadata i32 %foxtrot.sroa.2.0.extract.trunc, metadata !30, metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32)), !dbg !34 + store i32 %foxtrot.sroa.2.0.extract.trunc, i32* @alpha, align 4, !dbg !42 + call void @llvm.dbg.value(metadata i32 0, metadata !15, metadata !DIExpression()), !dbg !43 + br label %return + +return: ; preds = %entry, %if.end + %call.i = tail call i32 @_Z5bravov() + %retval.0 = sub nsw i32 0, %call.i + ret i32 %retval.0 +} + +declare void @llvm.dbg.value(metadata, metadata, metadata) #2 + +!llvm.dbg.cu = !{!2} +!llvm.module.flags = !{!7, !8, !9} +!llvm.ident = !{!10} + +!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression()) +!1 = distinct !DIGlobalVariable(name: "alpha", scope: !2, file: !3, line: 6, type: !6, isLocal: false, isDefinition: true) +!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 8.0.0 (trunk 344502)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: None) +!3 = !DIFile(filename: "pr39243.cpp", 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 8.0.0 (trunk 344502)"} +!11 = distinct !DISubprogram(name: "charlie", linkageName: "_Z7charliev", scope: !3, file: !3, line: 9, type: !12, isLocal: false, isDefinition: true, scopeLine: 9, flags: DIFlagPrototyped, isOptimized: true, unit: !2, retainedNodes: !14) +!12 = !DISubroutineType(types: !13) +!13 = !{!6} +!14 = !{!15} +!15 = !DILocalVariable(name: "delta", scope: !11, file: !3, line: 10, type: !6) +!18 = distinct !DISubprogram(name: "echo", linkageName: "_Z4echo5onion", scope: !3, file: !3, line: 14, type: !19, isLocal: false, isDefinition: true, scopeLine: 14, flags: DIFlagPrototyped, isOptimized: true, unit: !2, retainedNodes: !29) +!19 = !DISubroutineType(types: !20) +!20 = !{!6, !21} +!21 = distinct !DICompositeType(tag: DW_TAG_union_type, name: "onion", file: !3, line: 1, size: 64, flags: DIFlagTypePassByValue | DIFlagTrivial, elements: !22, identifier: "_ZTS5onion") +!22 = !{!23, !25} +!23 = !DIDerivedType(tag: DW_TAG_member, name: "dd", scope: !21, file: !3, line: 2, baseType: !24, size: 64) +!24 = !DIBasicType(name: "double", size: 64, encoding: DW_ATE_float) +!25 = !DIDerivedType(tag: DW_TAG_member, name: "ii", scope: !21, file: !3, line: 3, baseType: !26, size: 64) +!26 = !DICompositeType(tag: DW_TAG_array_type, baseType: !6, size: 64, elements: !27) +!27 = !{!28} +!28 = !DISubrange(count: 2) +!29 = !{!30} +!30 = !DILocalVariable(name: "foxtrot", arg: 1, scope: !18, file: !3, line: 14, type: !21) +!34 = !DILocation(line: 14, scope: !18) +!42 = !DILocation(line: 20, scope: !18) +!43 = !DILocation(line: 10, scope: !11, inlinedAt: !44) +!44 = distinct !DILocation(line: 21, scope: !18) -- 2.7.4