From 61b35e4111160fe834a00c33d040e01150b576ac Mon Sep 17 00:00:00 2001 From: stozer Date: Wed, 12 Feb 2020 13:41:18 +0000 Subject: [PATCH] Re-reapply: Recover debug intrinsics when killing duplicated/empty blocks This reverts commit 636c93ed11a5f98b5c3ff88241460d61cb7785bb. The original patch caused build failures on TSan buildbots. Commit 6ded69f294a9 fixes this issue by reducing the rate at which empty debug intrinsics propagate, reducing the memory footprint and preventing a fatal spike. --- llvm/include/llvm/Transforms/Utils/Local.h | 2 + llvm/lib/Transforms/Utils/Local.cpp | 27 +++++-- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 59 ++++++++++----- .../SimplifyCFG/hoist-dbgvalue-else.ll | 67 +++++++++++++++++ .../SimplifyCFG/hoist-dbgvalue-empty.ll | 71 +++++++++++++++++++ 5 files changed, 204 insertions(+), 22 deletions(-) create mode 100644 llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-else.ll create mode 100644 llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-empty.ll diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h index 38e3c14f7ef9c..c0e79b115e5fb 100644 --- a/llvm/include/llvm/Transforms/Utils/Local.h +++ b/llvm/include/llvm/Transforms/Utils/Local.h @@ -187,6 +187,8 @@ bool SimplifyInstructionsInBlock(BasicBlock *BB, /// Returns true if the dbg values have been changed. bool replaceDbgUsesWithUndef(Instruction *I); +void setDbgVariableUndef(DbgVariableIntrinsic *DVI); + //===----------------------------------------------------------------------===// // Control Flow Graph Restructuring. // diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index aad960230f5ce..23f38549a2494 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -505,15 +505,19 @@ void llvm::RecursivelyDeleteTriviallyDeadInstructions( I->eraseFromParent(); } } +void llvm::setDbgVariableUndef(DbgVariableIntrinsic *DVI) { + Value *DbgValue = DVI->getVariableLocation(false); + Value *Undef = UndefValue::get(DbgValue ? DbgValue->getType() + : Type::getInt1Ty(DVI->getContext())); + DVI->setOperand( + 0, MetadataAsValue::get(DVI->getContext(), ValueAsMetadata::get(Undef))); +} bool llvm::replaceDbgUsesWithUndef(Instruction *I) { SmallVector DbgUsers; findDbgUsers(DbgUsers, I); - for (auto *DII : DbgUsers) { - Value *Undef = UndefValue::get(I->getType()); - DII->setOperand(0, MetadataAsValue::get(DII->getContext(), - ValueAsMetadata::get(Undef))); - } + for (auto *DII : DbgUsers) + setDbgVariableUndef(DII); return !DbgUsers.empty(); } @@ -1061,6 +1065,19 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB, assert(PN->use_empty() && "There shouldn't be any uses here!"); PN->eraseFromParent(); } + // If Succ has multiple predecessors, each debug intrinsic in BB may or may + // not be valid when we reach Succ, so the debug variable should be set + // undef since its value is unknown. + Instruction *DbgInsertPoint = Succ->getFirstNonPHI(); + while (DbgInfoIntrinsic *DI = dyn_cast(&BB->front())) { + if (auto DVI = dyn_cast(DI)) { + if (!isa(DVI)) + setDbgVariableUndef(DVI); + DVI->moveBefore(DbgInsertPoint); + } else { + break; + } + } } // If the unconditional branch we replaced contains llvm.loop metadata, we diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 222f8af6547b2..0f8a9b4bc4f53 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -13,6 +13,7 @@ #include "llvm/ADT/APInt.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/MapVector.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SetOperations.h" @@ -38,6 +39,7 @@ #include "llvm/IR/ConstantRange.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" +#include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/Function.h" #include "llvm/IR/GlobalValue.h" @@ -1250,14 +1252,38 @@ static bool HoistThenElseCodeToIf(BranchInst *BI, Instruction *I1 = &*BB1_Itr++, *I2 = &*BB2_Itr++; // Skip debug info if it is not identical. - DbgInfoIntrinsic *DBI1 = dyn_cast(I1); - DbgInfoIntrinsic *DBI2 = dyn_cast(I2); - if (!DBI1 || !DBI2 || !DBI1->isIdenticalToWhenDefined(DBI2)) { - while (isa(I1)) - I1 = &*BB1_Itr++; - while (isa(I2)) - I2 = &*BB2_Itr++; - } + + // If the terminator instruction is hoisted, and any variable locations have + // non-identical debug intrinsics, then those variable locations must be set + // as undef. + // FIXME: If each block contains identical debug variable intrinsics in a + // different order, they will be considered non-identical and be dropped. + MapVector UndefDVIs; + + auto SkipDbgInfo = [&UndefDVIs](Instruction *&I, + BasicBlock::iterator &BB_Itr) { + while (isa(I)) { + if (DbgVariableIntrinsic *DVI = dyn_cast(I)) + UndefDVIs.insert( + {DebugVariable(DVI->getVariable(), DVI->getExpression(), + DVI->getDebugLoc()->getInlinedAt()), + DVI}); + I = &*BB_Itr++; + } + }; + + auto SkipNonIdenticalDbgInfo = + [&BB1_Itr, &BB2_Itr, &SkipDbgInfo](Instruction *&I1, Instruction *&I2) { + DbgInfoIntrinsic *DBI1 = dyn_cast(I1); + DbgInfoIntrinsic *DBI2 = dyn_cast(I2); + if (!DBI1 || !DBI2 || !DBI1->isIdenticalToWhenDefined(DBI2)) { + SkipDbgInfo(I1, BB1_Itr); + SkipDbgInfo(I2, BB2_Itr); + } + }; + + SkipNonIdenticalDbgInfo(I1, I2); + // FIXME: Can we define a safety predicate for CallBr? if (isa(I1) || !I1->isIdenticalToWhenDefined(I2) || (isa(I1) && !isSafeToHoistInvoke(BB1, BB2, I1, I2)) || @@ -1330,15 +1356,7 @@ static bool HoistThenElseCodeToIf(BranchInst *BI, I1 = &*BB1_Itr++; I2 = &*BB2_Itr++; - // Skip debug info if it is not identical. - DbgInfoIntrinsic *DBI1 = dyn_cast(I1); - DbgInfoIntrinsic *DBI2 = dyn_cast(I2); - if (!DBI1 || !DBI2 || !DBI1->isIdenticalToWhenDefined(DBI2)) { - while (isa(I1)) - I1 = &*BB1_Itr++; - while (isa(I2)) - I2 = &*BB2_Itr++; - } + SkipNonIdenticalDbgInfo(I1, I2); } while (I1->isIdenticalToWhenDefined(I2)); return true; @@ -1374,6 +1392,13 @@ static bool HoistThenElseCodeToIf(BranchInst *BI, } // Okay, it is safe to hoist the terminator. + for (auto DIVariableInst : UndefDVIs) { + DbgVariableIntrinsic *DVI = DIVariableInst.second; + DVI->moveBefore(BI); + if (DVI->getVariableLocation()) + setDbgVariableUndef(DVI); + } + Instruction *NT = I1->clone(); BIParent->getInstList().insert(BI->getIterator(), NT); if (!NT->getType()->isVoidTy()) { diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-else.ll b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-else.ll new file mode 100644 index 0000000000000..32962886be5d7 --- /dev/null +++ b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-else.ll @@ -0,0 +1,67 @@ +; RUN: opt -simplifycfg -S < %s | FileCheck %s +; Checks that when the if.then and if.else blocks are both eliminated by +; SimplifyCFG, as the common code is hoisted out, the variables with a +; dbg.value in either of those blocks are set undef just before the +; conditional branch instruction. + +define i32 @"?fn@@YAHH@Z"(i32 %foo) !dbg !8 { +; CHECK-LABEL: entry: +entry: + call void @llvm.dbg.value(metadata i32 %foo, metadata !13, metadata !DIExpression()), !dbg !16 + call void @llvm.dbg.value(metadata i32 0, metadata !14, metadata !DIExpression()), !dbg !16 + call void @llvm.dbg.value(metadata i32 0, metadata !15, metadata !DIExpression()), !dbg !16 + %cmp = icmp eq i32 %foo, 4, !dbg !17 +; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata [[BEARDS:![0-9]+]] +; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata [[BIRDS:![0-9]+]] + br i1 %cmp, label %if.then, label %if.else, !dbg !17 + +if.then: ; preds = %entry + call void @llvm.dbg.value(metadata i32 8, metadata !14, metadata !DIExpression()), !dbg !16 + br label %if.end, !dbg !18 + +if.else: ; preds = %entry + call void @llvm.dbg.value(metadata i32 4, metadata !14, metadata !DIExpression()), !dbg !16 + call void @llvm.dbg.value(metadata i32 8, metadata !15, metadata !DIExpression()), !dbg !16 + br label %if.end, !dbg !21 + +if.end: ; preds = %if.else, %if.then + %beards.0 = phi i32 [ 8, %if.then ], [ 4, %if.else ], !dbg !23 + call void @llvm.dbg.value(metadata i32 %beards.0, metadata !14, metadata !DIExpression()), !dbg !16 + ret i32 %beards.0, !dbg !24 +} + +declare void @llvm.dbg.value(metadata, metadata, metadata) + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!3, !4, !5, !6} +!llvm.ident = !{!7} + +; CHECK-LABEL: } +; CHECK: [[BEARDS]] = !DILocalVariable(name: "beards" +; CHECK: [[BIRDS]] = !DILocalVariable(name: "birds" + +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None) +!1 = !DIFile(filename: "test.cpp", directory: "C:\5Cdev\5Cllvm-project", checksumkind: CSK_MD5, checksum: "64604a72fdf5b6db8aa2328236bedd6b") +!2 = !{} +!3 = !{i32 2, !"CodeView", i32 1} +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = !{i32 1, !"wchar_size", i32 2} +!6 = !{i32 7, !"PIC Level", i32 2} +!7 = !{!"clang version 10.0.0 "} +!8 = distinct !DISubprogram(name: "fn", linkageName: "?fn@@YAHH@Z", scope: !1, file: !1, line: 1, type: !9, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !12) +!9 = !DISubroutineType(types: !10) +!10 = !{!11, !11} +!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) +!12 = !{!13, !14, !15} +!13 = !DILocalVariable(name: "foo", arg: 1, scope: !8, file: !1, line: 1, type: !11) +!14 = !DILocalVariable(name: "beards", scope: !8, file: !1, line: 2, type: !11) +!15 = !DILocalVariable(name: "birds", scope: !8, file: !1, line: 3, type: !11) +!16 = !DILocation(line: 0, scope: !8) +!17 = !DILocation(line: 5, scope: !8) +!18 = !DILocation(line: 8, scope: !19) +!19 = distinct !DILexicalBlock(scope: !20, file: !1, line: 5) +!20 = distinct !DILexicalBlock(scope: !8, file: !1, line: 5) +!21 = !DILocation(line: 11, scope: !22) +!22 = distinct !DILexicalBlock(scope: !20, file: !1, line: 8) +!23 = !DILocation(line: 0, scope: !20) +!24 = !DILocation(line: 13, scope: !8) diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-empty.ll b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-empty.ll new file mode 100644 index 0000000000000..7d5e32a2d656a --- /dev/null +++ b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-empty.ll @@ -0,0 +1,71 @@ +; RUN: opt -simplifycfg -S < %s | FileCheck %s +; Checks that when the if.then block is eliminated due to containing no +; instructions, the debug intrinsics are hoisted out of the block before its +; deletion. The hoisted intrinsics should have undef values as the branch +; behaviour is unknown to the intrinsics after hoisting. + +define dso_local i32 @"?fn@@YAHH@Z"(i32 %foo) local_unnamed_addr !dbg !8 { +entry: + call void @llvm.dbg.value(metadata i32 %foo, metadata !13, metadata !DIExpression()), !dbg !16 + call void @llvm.dbg.value(metadata i32 0, metadata !14, metadata !DIExpression()), !dbg !16 + call void @llvm.dbg.value(metadata i32 0, metadata !15, metadata !DIExpression()), !dbg !16 + %cmp = icmp eq i32 %foo, 4, !dbg !17 + br i1 %cmp, label %if.then, label %if.else, !dbg !17 + +if.then: ; preds = %entry + call void @llvm.dbg.value(metadata i32 8, metadata !14, metadata !DIExpression()), !dbg !16 + call void @llvm.dbg.value(metadata i32 3, metadata !15, metadata !DIExpression()), !dbg !16 + br label %if.end, !dbg !18 + +if.else: ; preds = %entry + call void @"?side@@YAXXZ"(), !dbg !21 + call void @llvm.dbg.value(metadata i32 4, metadata !14, metadata !DIExpression()), !dbg !16 + call void @llvm.dbg.value(metadata i32 6, metadata !15, metadata !DIExpression()), !dbg !16 + br label %if.end, !dbg !23 + +; CHECK-LABEL: if.end: +if.end: ; preds = %if.else, %if.then +; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata [[BEARDS:![0-9]+]] +; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata [[BIRDS:![0-9]+]] + %beards.0 = phi i32 [ 8, %if.then ], [ 4, %if.else ], !dbg !24 + call void @llvm.dbg.value(metadata i32 %beards.0, metadata !14, metadata !DIExpression()), !dbg !16 + ret i32 %beards.0, !dbg !25 +} + +declare dso_local void @"?side@@YAXXZ"() local_unnamed_addr + +declare void @llvm.dbg.value(metadata, metadata, metadata) + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!3, !4, !5, !6} +!llvm.ident = !{!7} + +; CHECK: [[BEARDS]] = !DILocalVariable(name: "beards" +; CHECK: [[BIRDS]] = !DILocalVariable(name: "birds" + +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None) +!1 = !DIFile(filename: "test2.cpp", directory: "C:\5Cdev\5Cllvm-project", checksumkind: CSK_MD5, checksum: "8ac5d40fcc9914d6479c1a770dfdc176") +!2 = !{} +!3 = !{i32 2, !"CodeView", i32 1} +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = !{i32 1, !"wchar_size", i32 2} +!6 = !{i32 7, !"PIC Level", i32 2} +!7 = !{!"clang version 10.0.0 "} +!8 = distinct !DISubprogram(name: "fn", linkageName: "?fn@@YAHH@Z", scope: !1, file: !1, line: 3, type: !9, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !12) +!9 = !DISubroutineType(types: !10) +!10 = !{!11, !11} +!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) +!12 = !{!13, !14, !15} +!13 = !DILocalVariable(name: "foo", arg: 1, scope: !8, file: !1, line: 3, type: !11) +!14 = !DILocalVariable(name: "beards", scope: !8, file: !1, line: 4, type: !11) +!15 = !DILocalVariable(name: "birds", scope: !8, file: !1, line: 5, type: !11) +!16 = !DILocation(line: 0, scope: !8) +!17 = !DILocation(line: 7, scope: !8) +!18 = !DILocation(line: 10, scope: !19) +!19 = distinct !DILexicalBlock(scope: !20, file: !1, line: 7) +!20 = distinct !DILexicalBlock(scope: !8, file: !1, line: 7) +!21 = !DILocation(line: 11, scope: !22) +!22 = distinct !DILexicalBlock(scope: !20, file: !1, line: 10) +!23 = !DILocation(line: 14, scope: !22) +!24 = !DILocation(line: 0, scope: !20) +!25 = !DILocation(line: 16, scope: !8)