[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

1 files changed, 24 insertions, 18 deletions

diff --git a/llvm/tools/opt/Debugify.cpp b/llvm/tools/opt/Debugify.cpp
index 5ed3de3622b..32e0023b153 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<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);