[llvm] r334118 - [Debugify] Move debug value intrinsics closer to their operand defs
Vedant Kumar via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 6 12:26:39 PDT 2018
As promised, here are bug reports for differences in `opt -O1` output with/without debug info present:
https://bugs.llvm.org/show_bug.cgi?id=37713 <https://bugs.llvm.org/show_bug.cgi?id=37713> (instcombine, stacksaverestore.ll)
https://bugs.llvm.org/show_bug.cgi?id=37714 <https://bugs.llvm.org/show_bug.cgi?id=37714> (instcombine, call-guard.ll)
https://bugs.llvm.org/show_bug.cgi?id=37715 <https://bugs.llvm.org/show_bug.cgi?id=37715> (guard-widening, range-check-merging.ll)
vedant
> On Jun 6, 2018, at 12:05 PM, Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: vedantk
> Date: Wed Jun 6 12:05:42 2018
> New Revision: 334118
>
> URL: http://llvm.org/viewvc/llvm-project?rev=334118&view=rev
> Log:
> [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.
>
> Modified:
> llvm/trunk/test/Transforms/DeadStoreElimination/debuginfo.ll
> llvm/trunk/test/Transforms/GVN/PRE/phi-translate-2.ll
> llvm/trunk/test/Transforms/GVN/fence.ll
> llvm/trunk/tools/opt/Debugify.cpp
>
> Modified: llvm/trunk/test/Transforms/DeadStoreElimination/debuginfo.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/DeadStoreElimination/debuginfo.ll?rev=334118&r1=334117&r2=334118&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/DeadStoreElimination/debuginfo.ll (original)
> +++ llvm/trunk/test/Transforms/DeadStoreElimination/debuginfo.ll Wed Jun 6 12:05:42 2018
> @@ -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
>
> Modified: llvm/trunk/test/Transforms/GVN/PRE/phi-translate-2.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/PRE/phi-translate-2.ll?rev=334118&r1=334117&r2=334118&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/GVN/PRE/phi-translate-2.ll (original)
> +++ llvm/trunk/test/Transforms/GVN/PRE/phi-translate-2.ll Wed Jun 6 12:05:42 2018
> @@ -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
>
> Modified: llvm/trunk/test/Transforms/GVN/fence.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/fence.ll?rev=334118&r1=334117&r2=334118&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/GVN/fence.ll (original)
> +++ llvm/trunk/test/Transforms/GVN/fence.ll Wed Jun 6 12:05:42 2018
> @@ -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
>
> Modified: llvm/trunk/tools/opt/Debugify.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/Debugify.cpp?rev=334118&r1=334117&r2=334118&view=diff
> ==============================================================================
> --- llvm/trunk/tools/opt/Debugify.cpp (original)
> +++ llvm/trunk/tools/opt/Debugify.cpp Wed Jun 6 12:05:42 2018
> @@ -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");
> +
> + // 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 (auto It = BB.begin(), End = LastInst->getIterator(); It != End;
> - ++It) {
> - Instruction &I = *It;
> -
> + 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<DbgValueInst>(&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<PHINode>(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);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180606/1c7108fd/attachment.html>
More information about the llvm-commits
mailing list