[PATCH] D25953: [IndVarSimplify][DebugLoc] When widening the exit loop condition, correctly reuse the debug location of the original comparison.

[Andrea Di Biagio via llvm-commits]

andreadb created this revision.
andreadb added reviewers: aprantl, dblaikie, probinson, wolfgangp, echristo.
andreadb added a subscriber: llvm-commits.
Herald added a subscriber: mehdi_amini.

This is a follow-up of https://reviews.llvm.org/D25872.

When the loop exit condition is canonicalized as a != comparison, reuse the debug loc associated to the original (non canonical) comparison.

  extern int foo(int);  // line 1
  
  int bar(int X, int *A) {
    for (int i = 0; i < X; ++i)  // line 4
      A[i] = foo(i);
  
    return A[0];
  }

Before IndVarSimplify is run, the optimized LLVM IR of the loop latch looks like this:

  for.inc:
    %inc = add nsw i32 %i.06, 1, !dbg !13
    %cmp = icmp slt i32 %inc, %X, !dbg !7
    br i1 %cmp, label %for.body, label %for.cond.for.end_crit_edge, !dbg !9, !llvm.loop !15

Where !dbg !7 is defined as:

  !7 = !DILocation(line: 4, column: 21, scope: !8)
  !8 = !DILexicalBlockFile(scope: !5, file: !1, discriminator: 1)

IndVarSimplify rewrites the exit condition of the loop to be a canonical != comparison. In this particular case, it rewrites the comparison in the loop latch as follows:

  %wide.trip.count = zext i32 %X to i64, !dbg !9
  %exitcond = icmp ne i64 %indvars.iv.next, %wide.trip.count, !dbg !9

However, !dbg !9 is defined as:

  !8 = !DILexicalBlockFile(scope: !5, file: !1, discriminator: 1)
  !9 = !DILocation(line: 4, column: 3, scope: !8)

Instead, it should be:

  !7 = !DILocation(line: 4, column: 21, scope: !8)
  !8 = !DILexicalBlockFile(scope: !5, file: !1, discriminator: 1)

The problem is indirectly caused by the fact that the debug location of the new icmp is obtained from the loop latch terminator (i.e. our IRBuilder "insertion point").

This patch fixes the issue by correctly setting the IRBuilder's "current debug location"  to the location of the original compare instruction.

Please let me know if okay to commit.

Thanks,
Andrea


https://reviews.llvm.org/D25953

Files:
  lib/Transforms/Scalar/IndVarSimplify.cpp
  test/DebugInfo/Generic/indvar-discriminator.ll


Index: test/DebugInfo/Generic/indvar-discriminator.ll
===================================================================
--- test/DebugInfo/Generic/indvar-discriminator.ll
+++ test/DebugInfo/Generic/indvar-discriminator.ll
@@ -2,6 +2,8 @@
 ;
 ; When the induction variable is widened by indvars, check that the debug loc
 ; associated with the loop increment is correctly propagated.
+; Also, when the exit condition of a loop is rewritten to be a canonical !=
+; comparison, check that the debug loc of the orginal comparison is reused.
 ;
 ; Test case obtained from the following source code:
 ;
@@ -19,10 +21,16 @@
 ; Check that the debug location for the loop increment still refers to
 ; line 4, column 26, discriminator 2.
 ;
+; Check that the canonicalized compare instruction for the loop exit
+; condition still refers to line 4, column 21, discriminator 1.
+;
 ; CHECK-LABEL: for.body:
 ; CHECK: [[PHI:%[0-9a-zA-Z.]+]] = phi i64 [
 ; CHECK-LABEL: for.inc:
 ; CHECK: add nuw nsw i64 [[PHI]], 1, !dbg ![[INDVARMD:[0-9]+]]
+; CHECK: icmp ne i64 %{{.+}}, %{{.+}}, !dbg ![[ICMPMD:[0-9]+]]
+; CHECK-DAG: ![[ICMPMD]] = !DILocation(line: 4, column: 21, scope: ![[ICMPSCOPEMD:[0-9]+]]
+; CHECK-DAG: ![[ICMPSCOPEMD]] = !DILexicalBlockFile(scope: !{{[0-9]+}}, file: !{{[0-9]+}}, discriminator: 1)
 ; CHECK-DAG: ![[INDVARMD]] = !DILocation(line: 4, column: 26, scope: ![[INDVARSCOPEMD:[0-9]+]])
 ; CHECK-DAG: ![[INDVARSCOPEMD]] = !DILexicalBlockFile(scope: !{{[0-9]+}}, file: !{{[0-9]+}}, discriminator: 2)
 
Index: lib/Transforms/Scalar/IndVarSimplify.cpp
===================================================================
--- lib/Transforms/Scalar/IndVarSimplify.cpp
+++ lib/Transforms/Scalar/IndVarSimplify.cpp
@@ -2189,6 +2189,11 @@
 
   IRBuilder<> Builder(BI);
 
+  // The new loop exit condition should reuse the debug location of the
+  // original loop exit condition.
+  if (auto *Cond = dyn_cast<Instruction>(BI->getCondition()))
+    Builder.SetCurrentDebugLocation(Cond->getDebugLoc());
+
   // LFTR can ignore IV overflow and truncate to the width of
   // BECount. This avoids materializing the add(zext(add)) expression.
   unsigned CmpIndVarSize = SE->getTypeSizeInBits(CmpIndVar->getType());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D25953.75734.patch
Type: text/x-patch
Size: 2210 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161025/34ed49ea/attachment.bin>