[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:05:42 PDT 2018
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);
More information about the llvm-commits
mailing list