[WIP][LV][DebugInfo] Set DL to the middle block Icmp instruction

[gramanas]

Summary

The DL comes from the entry basic block since the comparison
is essentially a check in the loop.

Please confirm if this debug location is the correct one.

---

The test is a WIP for testing various DL in instructions generated
while creating the VectorizedLoopSkeleton.

[hsaito]

This is essentially a zero trip check for the remainder loop and from that perspective, the most correct DL we should use would be the one related to trip count computation. InnerLoopVectorizer::getOrCreateTripCount() is where LV computes the trip count of the incoming scalar loop. It uses getBackedgeTakeCount(). As such, strictly speaking, taking the DL from the predicate (i.e., loop bottom test) fed to the loop backedge would make most sense. That would be OrigLoop->getLoopLatch()->getTerminator() to get to the backedge. In a relaxed thinking, however, this is the code executed when we know vector code executes. So, taking DL from VectorPH code isn't too bad. OrigLoop->getStartLoc() could be another viable enough alternative.

Does this make sense to you? Which one would you pick?

[gramanas]

In D49746#1174388, @hsaito wrote:

This is essentially a zero trip check for the remainder loop and from that perspective, the most correct DL we should use would be the one related to trip count computation. InnerLoopVectorizer::getOrCreateTripCount() is where LV computes the trip count of the incoming scalar loop. It uses getBackedgeTakeCount(). As such, strictly speaking, taking the DL from the predicate (i.e., loop bottom test) fed to the loop backedge would make most sense. That would be OrigLoop->getLoopLatch()->getTerminator() to get to the backedge. In a relaxed thinking, however, this is the code executed when we know vector code executes. So, taking DL from VectorPH code isn't too bad. OrigLoop->getStartLoc() could be another viable enough alternative.

Does this make sense to you? Which one would you pick?

Thank you for the comment.

I don't understand what zero trip means but I think the second case, OrigLoop->getStartLoc(), fits a bit better. I assigned the VectorPH->getTerminator() DL to get the same result basically. Your explanation along with a google search explained a lot and it makes more sense now to have this DL since it's the condition for a jump to the other part of the loop.

[gramanas]

• update to reflect the comments

[hsaito]

Let's start from this and see if practical usability issues pop up. This is where main vector loop just ended and right before the remainder loop starts. As long as the location info points to the top or the bottom of the loop, that won't surprise the user. Beyond that is just how much more precise we'd like to be. LGTM.

[gramanas]

http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/18826/steps/ninja%20check%201/logs/stdio

This results in many ICmp Instructions to have completely wrong !dbg attached, since it points to a different subprogram.

I reverted the commit in rL338109, but I don't understand why this happens.

Also I think the change of CmpN from Value to CmpInst triggered these warnings: http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/18826/steps/build%20stage%201/logs/warnings%20%284%29

[xbolva00]

In D49746#1177945, @gramanas wrote:

http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/18826/steps/ninja%20check%201/logs/stdio

This results in many ICmp Instructions to have completely wrong !dbg attached, since it points to a different subprogram.

I reverted the commit in rL338109, but I don't understand why this happens.

Also I think the change of CmpN from Value to CmpInst triggered these warnings: http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/18826/steps/build%20stage%201/logs/warnings%20%284%29

That -Woverloaded-virtual warnings are not related to your patch :)

lib/Transforms/Vectorize/LoopVectorize.cpp

for (auto &InductionEntry : *List) {
     for (BasicBlock *BB : LoopBypassBlocks)
       BCResumeVal->addIncoming(II.getStartValue(), BB);
     OrigPhi->setIncomingValue(BlockIdx, BCResumeVal);
   }
 
   // Add a check in the middle block to see if we have completed
   // all of the iterations in the first vector loop.
   // If (N - N%VF) == N, then we *don't* need to run the remainder.
-  Value *CmpN =
+  CmpInst *CmpN =
       CmpInst::Create(Instruction::ICmp, CmpInst::ICMP_EQ, Count,
                       CountRoundDown, "cmp.n", MiddleBlock->getTerminator());
+  // Copy the DL from loop start location to the check.
+  CmpN->setDebugLoc(OrigLoop->getStartLoc());
   ReplaceInstWithInst(MiddleBlock->getTerminator(),
                       BranchInst::Create(ExitBlock, ScalarPH, CmpN));
 
   // Get ready to start creating new instructions into the vectorized body.
   Builder.SetInsertPoint(&*VecBody->getFirstInsertionPt());
 
   // Save the state.
   LoopVectorPreHeader = Lp->getLoopPreheader();

test/Transforms/LoopVectorize/debugloc-skeleton.ll

+; RUN: opt -S -debugify -loop-vectorize -force-vector-width=2 -force-vector-interleave=1 < %s | FileCheck %s
+
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+
+; Test that the new vectorized loop has proper debug location.
+
+define i32 @vect(i32* %a) {
+entry:
+  br label %for.body
+
+; CHECK-LABEL: vector.body:
+; CHECK: [[index:%.*]] = phi i64 {{.*}}, !dbg ![[line2:[0-9]+]]
+
+for.body:
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+  %red.05 = phi i32 [ 0, %entry ], [ %add, %for.body ]
+  %arrayidx = getelementptr inbounds i32, i32* %a, i64 %indvars.iv
+  %0 = load i32, i32* %arrayidx, align 4
+  %add = add nsw i32 %0, %red.05
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond = icmp eq i64 %indvars.iv.next, 255
+  br i1 %exitcond, label %for.end, label %for.body
+
+; CHECK-LABEL: middle.block:
+; CHECK: %cmp.n = icmp {{.*}}, !dbg ![[line1:[0-9]+]]
+; CHECK: br i1 %cmp.n, {{.*}}, !dbg ![[line1]]
+
+for.end:
+  ret i32 %add
+}
+
+; CHECK: ![[line1]] = !DILocation(line: 1
+; CHECK: ![[line2]] = !DILocation(line: 2