From: Vedant Kumar Date: Wed, 6 Jun 2018 19:05:42 +0000 (+0000) Subject: [Debugify] Move debug value intrinsics closer to their operand defs X-Git-Tag: llvmorg-7.0.0-rc1~4326 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6d354ed72e6e10b73c6a1d8481129a153434cdb4;p=platform%2Fupstream%2Fllvm.git [Debugify] Move debug value intrinsics closer to their operand defs Before this patch, debugify would insert debug value intrinsics before the terminating instruction in a block. This had the advantage of being simple, but was a bit too simple/unrealistic. This patch teaches debugify to insert debug values immediately after their operand defs. This enables better testing of the compiler. For example, with this patch, `opt -debugify-each` is able to identify a vectorizer DI-invariance bug fixed in llvm.org/PR32761. In this bug, the vectorizer produced different output with/without debug info present. Reverting Davide's bugfix locally, I see: $ ~/scripts/opt-check-dbg-invar.sh ./bin/opt \ .../SLPVectorizer/AArch64/spillcost-di.ll -slp-vectorizer Comparing: -slp-vectorizer .../SLPVectorizer/AArch64/spillcost-di.ll Baseline: /var/folders/j8/t4w0bp8j6x1g6fpghkcb4sjm0000gp/T/tmp.iYYeL1kf With DI : /var/folders/j8/t4w0bp8j6x1g6fpghkcb4sjm0000gp/T/tmp.sQtQSeet 9,11c9,11 < %5 = getelementptr inbounds %0, %0* %2, i64 %0, i32 1 < %6 = bitcast i64* %4 to <2 x i64>* < %7 = load <2 x i64>, <2 x i64>* %6, align 8, !tbaa !0 --- > %5 = load i64, i64* %4, align 8, !tbaa !0 > %6 = getelementptr inbounds %0, %0* %2, i64 %0, i32 1 > %7 = load i64, i64* %6, align 8, !tbaa !5 12a13 > store i64 %5, i64* %8, align 8, !tbaa !0 14,15c15 < %10 = bitcast i64* %8 to <2 x i64>* < store <2 x i64> %7, <2 x i64>* %10, align 8, !tbaa !0 --- > store i64 %7, i64* %9, align 8, !tbaa !5 :: Found a test case ^ Running this over the *.ll files in tree, I found four additional examples which compile differently with/without DI present. I plan on filing bugs for these. llvm-svn: 334118 --- diff --git a/llvm/test/Transforms/DeadStoreElimination/debuginfo.ll b/llvm/test/Transforms/DeadStoreElimination/debuginfo.ll index 813c0425d01e..2d9b4fe3ee83 100644 --- a/llvm/test/Transforms/DeadStoreElimination/debuginfo.ll +++ b/llvm/test/Transforms/DeadStoreElimination/debuginfo.ll @@ -7,17 +7,17 @@ declare noalias i8* @malloc(i32) declare void @test_f() define i32* @test_salvage() { +; Check that all four original local variables have their values preserved. ; CHECK-LABEL: @test_salvage() ; CHECK-NEXT: malloc -; CHECK-NEXT: bitcast -; CHECK-NEXT: call void @test_f() -; CHECK-NEXT: store i32 0, i32* %P - -; Check that all four original local variables have their values preserved. ; CHECK-NEXT: @llvm.dbg.value(metadata i8* %p, metadata ![[p:.*]], metadata !DIExpression()) +; CHECK-NEXT: bitcast ; CHECK-NEXT: @llvm.dbg.value(metadata i32* %P, metadata ![[P:.*]], metadata !DIExpression()) ; CHECK-NEXT: @llvm.dbg.value(metadata i32* %P, metadata ![[DEAD:.*]], metadata !DIExpression(DW_OP_deref)) ; CHECK-NEXT: call void @llvm.dbg.value(metadata i32* %P, metadata ![[DEAD2:.*]], metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 1, DW_OP_stack_value)) +; CHECK-NEXT: call void @test_f() +; CHECK-NEXT: store i32 0, i32* %P + %p = tail call i8* @malloc(i32 4) %P = bitcast i8* %p to i32* %DEAD = load i32, i32* %P diff --git a/llvm/test/Transforms/GVN/PRE/phi-translate-2.ll b/llvm/test/Transforms/GVN/PRE/phi-translate-2.ll index 2edc2c10b27e..7911b525625f 100644 --- a/llvm/test/Transforms/GVN/PRE/phi-translate-2.ll +++ b/llvm/test/Transforms/GVN/PRE/phi-translate-2.ll @@ -141,9 +141,9 @@ critedge.loopexit: ; CHECK: br label %if.end3 ; CHECK: if.end3: ; CHECK: %[[PREPHI:.*]] = phi i64 [ %sub.ptr.sub, %if.else ], [ %[[SUB]], %if.then2 ], [ %sub.ptr.sub, %entry ] -; CHECK: %[[DIV:.*]] = ashr exact i64 %[[PREPHI]], 2 ; CHECK: call void @llvm.dbg.value(metadata i32* %p.0, metadata [[var_p0:![0-9]+]], metadata !DIExpression()) ; CHECK: call void @llvm.dbg.value(metadata i64 %sub.ptr.rhs.cast5.pre-phi, metadata [[var_sub_ptr:![0-9]+]], metadata !DIExpression()) +; CHECK: %[[DIV:.*]] = ashr exact i64 %[[PREPHI]], 2 ; CHECK: ret i64 %[[DIV]] declare void @bar(...) local_unnamed_addr #1 diff --git a/llvm/test/Transforms/GVN/fence.ll b/llvm/test/Transforms/GVN/fence.ll index 273c3b669481..ab8e2bbe72ba 100644 --- a/llvm/test/Transforms/GVN/fence.ll +++ b/llvm/test/Transforms/GVN/fence.ll @@ -18,10 +18,10 @@ define i32 @test(i32* %addr.i) { ; Same as above define i32 @test2(i32* %addr.i) { ; CHECK-LABEL: @test2 -; CHECK-NEXT: fence ; CHECK-NEXT: call void @llvm.dbg.value(metadata i32* %addr.i, metadata [[var_a:![0-9]+]], metadata !DIExpression(DW_OP_deref)) -; CHECK-NEXT: call void @llvm.dbg.value(metadata i32* %addr.i, metadata [[var_a2:![0-9]+]], metadata !DIExpression(DW_OP_deref)) +; CHECK-NEXT: fence ; CHECK-NOT: load +; CHECK-NEXT: call void @llvm.dbg.value(metadata i32* %addr.i, metadata [[var_a2:![0-9]+]], metadata !DIExpression(DW_OP_deref)) ; CHECK: ret %a = load i32, i32* %addr.i, align 4 fence release diff --git a/llvm/tools/opt/Debugify.cpp b/llvm/tools/opt/Debugify.cpp index 5ed3de3622b1..32e0023b153a 100644 --- a/llvm/tools/opt/Debugify.cpp +++ b/llvm/tools/opt/Debugify.cpp @@ -44,12 +44,11 @@ bool isFunctionSkipped(Function &F) { return F.isDeclaration() || !F.hasExactDefinition(); } -/// Find a suitable insertion point for debug values intrinsics. +/// Find the basic block's terminating instruction. /// -/// These must be inserted before the terminator. Special care is needed to -/// handle musttail and deopt calls, as these behave like (but are in fact not) -/// terminators. -Instruction *findDebugValueInsertionPoint(BasicBlock &BB) { +/// Special care is needed to handle musttail and deopt calls, as these behave +/// like (but are in fact not) terminators. +Instruction *findTerminatingInstruction(BasicBlock &BB) { if (auto *I = BB.getTerminatingMustTailCall()) return I; if (auto *I = BB.getTerminatingDeoptimizeCall()) @@ -109,28 +108,35 @@ bool applyDebugifyMetadata(Module &M, if (BB.isEHPad()) continue; - Instruction *LastInst = findDebugValueInsertionPoint(BB); + // Find the terminating instruction, after which no debug values are + // attached. + Instruction *LastInst = findTerminatingInstruction(BB); + assert(LastInst && "Expected basic block with a terminator"); - // Attach debug values. - for (auto It = BB.begin(), End = LastInst->getIterator(); It != End; - ++It) { - Instruction &I = *It; + // Maintain an insertion point which can't be invalidated when updates + // are made. + BasicBlock::iterator InsertPt = BB.getFirstInsertionPt(); + assert(InsertPt != BB.end() && "Expected to find an insertion point"); + Instruction *InsertBefore = &*InsertPt; + // Attach debug values. + for (Instruction *I = &*BB.begin(); I != LastInst; I = I->getNextNode()) { // Skip void-valued instructions. - if (I.getType()->isVoidTy()) + if (I->getType()->isVoidTy()) continue; - // Skip any just-inserted intrinsics. - if (isa(&I)) - break; + // Phis and EH pads must be grouped at the beginning of the block. + // Only advance the insertion point when we finish visiting these. + if (!isa(I) && !I->isEHPad()) + InsertBefore = I->getNextNode(); std::string Name = utostr(NextVar++); - const DILocation *Loc = I.getDebugLoc().get(); + const DILocation *Loc = I->getDebugLoc().get(); auto LocalVar = DIB.createAutoVariable(SP, Name, File, Loc->getLine(), - getCachedDIType(I.getType()), + getCachedDIType(I->getType()), /*AlwaysPreserve=*/true); - DIB.insertDbgValueIntrinsic(&I, LocalVar, DIB.createExpression(), Loc, - LastInst); + DIB.insertDbgValueIntrinsic(I, LocalVar, DIB.createExpression(), Loc, + InsertBefore); } } DIB.finalizeSubprogram(SP);