Modify interleave diagnostics to clearly indicate why interleaving wasn't done.

[tyler.nowicki]

Summary

Currently the message that is produced implies that the user disabled interleaving. This isn't always true however. Sometimes interleaving it is not beneficial, as determined by the vectorizers cost model. With this patch the interleaving profitability and disabled state of the optimization are separately indicated by the message.

[tyler.nowicki]

In this update, and the previous update, renaming parts of the work are removed/moved to the renaming patch.

Sorry about the spam. I am getting used to phabricator.

[Gerolf]

lib/Transforms/Vectorize/LoopVectorize.cpp

1663

It would be easier to read if you handled the checks separately

1667–1674

add assert(IC > 1).

[tyler.nowicki]

Thanks for the review Gerolf. I've expanded / separate the diagnostic section.

[hfinkel]

lib/Transforms/Vectorize/LoopVectorize.cpp

1663

These variable should start with a capital letter (per the coding conventions).

1677

Same here (and below)... capital letters at the start of variable names.

1686

Saying "is beneficial" seems a bit strong, especially considering that the user explicitly turned it off. We could say, "The cost model indicates that interleaving is beneficial, but...".

[tyler.nowicki]

Hi Hal,

Thanks for the review, and sorry for the variable name mistakes.

Since the diagnostic messages are a little longer now I generate two separate messages for vectorization and interleaving rather than concatenating them together.

Here is the updated patch.

Tyler

[Gerolf]

lib/Transforms/Vectorize/LoopVectorize.cpp

1582–1585

The comment in DEBUG ('already vectorized') is inconsistent with your comment.

1588–1590

Could there be two separated remarks something like a) not vectorized because turned off and b) not vectorized because of low thresholds

[tyler.nowicki]

Hi Gerolf,

Thanks for the comments. I've replied inline.

Tyler

lib/Transforms/Vectorize/LoopVectorize.cpp

1582–1585

The 'already vectorized' comment is there because the vector width and interleave count are set to 1 after the loop is vectorized. If the loop is, for some reason, run through the vectorizer again then we won't try to vectorize it again.

I think a separate fix is needed for this. I think we may need to add metadata like loop.vectorize.complete = true to do this instead.

1588–1590

The metadata doesn't currently allow us to differentiate between those cases. The pragmas vectorization(disable) and vectorization_width(1) do the same thing. They set loop.vectorize.width = 1.

This is because loop.vectorize.enable is used disable both vectorization and interleaving. I was working on a patch to change this. However, without diagnostics like this there was no way to test it. So I stopped working on that and moved to this.

I'll differentiate between these cases in a subsequent patch.

[Gerolf]

With one FIXME LGTM.

lib/Transforms/Vectorize/LoopVectorize.cpp

1582–1585

Ok. Please keep that thought in a FIXME.

1588–1590

Ok, thanks.

[tyler.nowicki]

Hi Hal,

I updated the comments as Gerolf requested. Please let me know if you have any other comments.

Thanks,

Tyler

[Gerolf]

I think all comments including Hal’s have been addressed. Adding Michael and/or Arnold to double check.

-Gerolf

[hfinkel]

LGTM.

[Eugene.Zelenko]

Committed in r244485.

lib/Transforms/Vectorize/LoopVectorize.cpp

#endif /* NDEBUG */
     if (!AlwaysVectorize && Hints.getForce() != LoopVectorizeHints::FK_Enabled) {
       DEBUG(dbgs() << "LV: Not vectorizing: No #pragma vectorize enable.\n");
       emitOptimizationRemarkAnalysis(F->getContext(), DEBUG_TYPE, *F,
                                      L->getStartLoc(), Hints.emitRemark());
       return false;
     }
 
     if (Hints.getWidth() == 1 && Hints.getInterleave() == 1) {
+      // FIXME: Add a separate metadata to indicate when the loop has already
+      // been vectorized instead of setting width and count to 1.
       DEBUG(dbgs() << "LV: Not vectorizing: Disabled/already vectorized.\n");
+      // FIXME: Add interleave.disable metadata. This will allow
+      // vectorize.disable to be used without disabling the pass and errors
+      // to differentiate between disabled vectorization and a width of 1.
       emitOptimizationRemarkAnalysis(
           F->getContext(), DEBUG_TYPE, *F, L->getStartLoc(),
-          "loop not vectorized: vector width and interleave count are "
-          "explicitly set to 1");
+          "loop not vectorized: vectorization and interleaving are explicitly "
+          "disabled, or vectorize width and interleave count are both set to "
+          "1");
       return false;
     }
 
     // Check the loop for a trip count threshold:
     // do not vectorize loops with a tiny trip count.
     const unsigned TC = SE->getSmallConstantTripCount(L);
     if (TC > 0u && TC < TinyTripCountVectorThreshold) {
       DEBUG(dbgs() << "LV: Found a loop with a very small trip count. "
#endif /* NDEBUG */
 
     // Select the optimal vectorization factor.
     const LoopVectorizationCostModel::VectorizationFactor VF =
         CM.selectVectorizationFactor(OptForSize);
 
     // Select the interleave count.
     unsigned IC = CM.selectInterleaveCount(OptForSize, VF.Width, VF.Cost);
 
-    DEBUG(dbgs() << "LV: Found a vectorizable loop (" << VF.Width << ") in "
-                 << DebugLocStr << '\n');
-    DEBUG(dbgs() << "LV: Interleave Count is " << IC << '\n');
+    // Get user interleave count.
+    unsigned UserIC = Hints.getInterleave();
 
-    if (VF.Width == 1) {
-      DEBUG(dbgs() << "LV: Vectorization is possible but not beneficial\n");
+    // Identify the diagnostic messages that should be produced.
+    std::string VecDiagMsg, IntDiagMsg;
+    bool VectorizeLoop = true, InterleaveLoop = true;
 
-      if (IC == 1) {
-        emitOptimizationRemarkAnalysis(
-            F->getContext(), DEBUG_TYPE, *F, L->getStartLoc(),
-            "not beneficial to vectorize and user disabled interleaving");
-        return false;
-      }
-      DEBUG(dbgs() << "LV: Trying to at least unroll the loops.\n");
-
-      // Report the unrolling decision.
-      emitOptimizationRemark(F->getContext(), DEBUG_TYPE, *F, L->getStartLoc(),
-                             Twine("interleaved by " + Twine(IC) +
-                                   " (vectorization not beneficial)"));
+    if (VF.Width == 1) {
+      DEBUG(dbgs() << "LV: Vectorization is possible but not beneficial.\n");
+      VecDiagMsg =
+          "the cost-model indicates that vectorization is not beneficial";
+      VectorizeLoop = false;
+    }
+
+    if (IC == 1 && UserIC <= 1) {
+      // Tell the user interleaving is not beneficial.
+      DEBUG(dbgs() << "LV: Interleaving is not beneficial.\n");
+      IntDiagMsg =
+          "the cost-model indicates that interleaving is not beneficial";
+      InterleaveLoop = false;
+      if (UserIC == 1)
+        IntDiagMsg +=
+            " and is explicitly disabled or interleave count is set to 1";
+    } else if (IC > 1 && UserIC == 1) {
+      // Tell the user interleaving is beneficial, but it explicitly disabled.
+      DEBUG(dbgs()
+            << "LV: Interleaving is beneficial but is explicitly disabled.");
+      IntDiagMsg = "the cost-model indicates that interleaving is beneficial "
+                   "but is explicitly disabled or interleave count is set to 1";
+      InterleaveLoop = false;
+    }
+
+    // Override IC if user provided an interleave count.
+    IC = UserIC > 0 ? UserIC : IC;
+
+    // Emit diagnostic messages, if any.
+    if (!VectorizeLoop && !InterleaveLoop) {
+      // Do not vectorize or interleaving the loop.
+      emitOptimizationRemarkAnalysis(F->getContext(), DEBUG_TYPE, *F,
+                                     L->getStartLoc(), VecDiagMsg);
+      emitOptimizationRemarkAnalysis(F->getContext(), DEBUG_TYPE, *F,
+                                     L->getStartLoc(), IntDiagMsg);
+    } else if (!VectorizeLoop && InterleaveLoop) {
+      DEBUG(dbgs() << "LV: Interleave Count is " << IC << '\n');
+      emitOptimizationRemarkAnalysis(F->getContext(), DEBUG_TYPE, *F,
+                                     L->getStartLoc(), VecDiagMsg);
+    } else if (VectorizeLoop && !InterleaveLoop) {
+      DEBUG(dbgs() << "LV: Found a vectorizable loop (" << VF.Width << ") in "
+                   << DebugLocStr << '\n');
+      emitOptimizationRemarkAnalysis(F->getContext(), DEBUG_TYPE, *F,
+                                     L->getStartLoc(), IntDiagMsg);
+    } else if (VectorizeLoop && InterleaveLoop) {
+      DEBUG(dbgs() << "LV: Found a vectorizable loop (" << VF.Width << ") in "
+                   << DebugLocStr << '\n');
+      DEBUG(dbgs() << "LV: Interleave Count is " << IC << '\n');
 
+    if (!VectorizeLoop) {
+      assert(IC > 1 && "interleave count should not be 1 or 0");
+      // If we decided that it is not legal to vectorize the loop then
+      // interleave it.
       InnerLoopUnroller Unroller(L, SE, LI, DT, TLI, TTI, IC);
       Unroller.vectorize(&LVL);
+
+      emitOptimizationRemark(F->getContext(), DEBUG_TYPE, *F, L->getStartLoc(),
+                             Twine("interleaved loop (interleaved count: ") +
+                                 Twine(IC) + ")");
     } else {
       // If we decided that it is *legal* to vectorize the loop then do it.
       InnerLoopVectorizer LB(L, SE, LI, DT, TLI, TTI, VF.Width, IC);
       LB.vectorize(&LVL);
       ++LoopsVectorized;
 
       // Add metadata to disable runtime unrolling scalar loop when there's no
       // runtime check about strides and memory. Because at this situation,
unsigned LoopVectorizationCostModel::selectInterleaveCount(bool OptForSize,
   // We use the following heuristics to select the interleave count:
   // 1. If the code has reductions, then we interleave to break the cross
   // iteration dependency.
   // 2. If the loop is really small, then we interleave to reduce the loop
   // overhead.
   // 3. We don't interleave if we think that we will spill registers to memory
   // due to the increased register pressure.
 
-  // Use the user preference, unless 'auto' is selected.
-  int UserUF = Hints->getInterleave();
-  if (UserUF != 0)
-    return UserUF;
-
   // When we optimize for size, we don't interleave.
   if (OptForSize)
     return 1;
 
   // We used the distance for the interleave count.
   if (Legal->getMaxSafeDepDistBytes() != -1U)
     return 1;
 

test/Transforms/LoopVectorize/X86/vectorization-remarks-missed.ll

 ; #pragma clang loop vectorize(enable)
 ;   for (int i = 0; i < Length; i++)
 ;     A[i] = A[B[i]];
 ; }
 
 ; File, line, and column should match those specified in the metadata
 ; CHECK: remark: source.cpp:4:5: loop not vectorized: could not determine number of loop iterations
 ; CHECK: remark: source.cpp:4:5: loop not vectorized: use -Rpass-analysis=loop-vectorize for more info
-; CHECK: remark: source.cpp:13:5: loop not vectorized: vector width and interleave count are explicitly set to 1
+; CHECK: remark: source.cpp:13:5: loop not vectorized: vectorization and interleaving are explicitly disabled, or vectorize width and interleave count are both set to 1
 ; CHECK: remark: source.cpp:19:5: loop not vectorized: cannot identify array bounds
 ; CHECK: remark: source.cpp:19:5: loop not vectorized: use -Rpass-analysis=loop-vectorize for more info
 ; CHECK: warning: source.cpp:19:5: loop not vectorized: failed explicitly specified loop vectorization
 
 ; CHECK: _Z4testPii
 ; CHECK-NOT: x i32>
 ; CHECK: ret
 

test/Transforms/LoopVectorize/X86/vectorization-remarks-profitable.ll

+; RUN: opt < %s -loop-vectorize -pass-remarks-analysis='loop-vectorize' -mtriple=x86_64-unknown-linux -S 2>&1 | FileCheck %s
+
+; Verify analysis remarks are generated when interleaving is not beneficial.
+; CHECK: remark: vectorization-remarks-profitable.c:4:14: the cost-model indicates that vectorization is not beneficial
+; CHECK: remark: vectorization-remarks-profitable.c:4:14: the cost-model indicates that interleaving is not beneficial and is explicitly disabled or interleave count is set to 1
+; CHECK: remark: vectorization-remarks-profitable.c:11:14: the cost-model indicates that vectorization is not beneficial
+; CHECK: remark: vectorization-remarks-profitable.c:11:14: the cost-model indicates that interleaving is not beneficial
+
+; First loop.
+;  #pragma clang loop interleave(disable) unroll(disable)
+;  for(int i = 0; i < n; i++) {
+;    out[i] = in[i];
+;  }
+
+; Second loop.
+;  #pragma clang loop unroll(disable)
+;  for(int i = 0; i < n; i++) {
+;    out[i] = in[i];
+;  }
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.10.0"
+
+; Function Attrs: nounwind ssp uwtable
+define void @do_not_interleave(float* nocapture %out, float* nocapture readonly %in, i32 %n) #0 {
+entry:
+  %cmp.7 = icmp sgt i32 %n, 0, !dbg !3
+  br i1 %cmp.7, label %for.body.preheader, label %for.cond.cleanup, !dbg !8
+
+for.body.preheader:                               ; preds = %entry
+  br label %for.body, !dbg !9
+
+for.cond.cleanup.loopexit:                        ; preds = %for.body
+  br label %for.cond.cleanup, !dbg !10
+
+for.cond.cleanup:                                 ; preds = %for.cond.cleanup.loopexit, %entry
+  ret void, !dbg !10
+
+for.body:                                         ; preds = %for.body.preheader, %for.body
+  %indvars.iv = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %for.body.preheader ]
+  %arrayidx = getelementptr inbounds float, float* %in, i64 %indvars.iv, !dbg !9
+  %0 = bitcast float* %arrayidx to i32*, !dbg !9
+  %1 = load i32, i32* %0, align 4, !dbg !9, !tbaa !11
+  %arrayidx2 = getelementptr inbounds float, float* %out, i64 %indvars.iv, !dbg !15
+  %2 = bitcast float* %arrayidx2 to i32*, !dbg !16
+  store i32 %1, i32* %2, align 4, !dbg !16, !tbaa !11
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1, !dbg !8
+  %lftr.wideiv = trunc i64 %indvars.iv.next to i32, !dbg !8
+  %exitcond = icmp eq i32 %lftr.wideiv, %n, !dbg !8
+  br i1 %exitcond, label %for.cond.cleanup.loopexit, label %for.body, !dbg !8, !llvm.loop !17
+}
+
+; Function Attrs: nounwind ssp uwtable
+define void @interleave_not_profitable(float* nocapture %out, float* nocapture readonly %in, i32 %n) #0 {
+entry:
+  %cmp.7 = icmp sgt i32 %n, 0, !dbg !20
+  br i1 %cmp.7, label %for.body.preheader, label %for.cond.cleanup, !dbg !22
+
+for.body.preheader:                               ; preds = %entry
+  br label %for.body, !dbg !23
+
+for.cond.cleanup.loopexit:                        ; preds = %for.body
+  br label %for.cond.cleanup, !dbg !24
+
+for.cond.cleanup:                                 ; preds = %for.cond.cleanup.loopexit, %entry
+  ret void, !dbg !24
+
+for.body:                                         ; preds = %for.body.preheader, %for.body
+  %indvars.iv = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %for.body.preheader ]
+  %arrayidx = getelementptr inbounds float, float* %in, i64 %indvars.iv, !dbg !23
+  %0 = bitcast float* %arrayidx to i32*, !dbg !23
+  %1 = load i32, i32* %0, align 4, !dbg !23, !tbaa !11
+  %arrayidx2 = getelementptr inbounds float, float* %out, i64 %indvars.iv, !dbg !25
+  %2 = bitcast float* %arrayidx2 to i32*, !dbg !26
+  store i32 %1, i32* %2, align 4, !dbg !26, !tbaa !11
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1, !dbg !22
+  %lftr.wideiv = trunc i64 %indvars.iv.next to i32, !dbg !22
+  %exitcond = icmp eq i32 %lftr.wideiv, %n, !dbg !22
+  br i1 %exitcond, label %for.cond.cleanup.loopexit, label %for.body, !dbg !22, !llvm.loop !27
+}
+
+attributes #0 = { nounwind }
+
+!llvm.module.flags = !{!0, !1}
+!llvm.ident = !{!2}
+
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = !{i32 1, !"PIC Level", i32 2}
+!2 = !{!"clang version 3.7.0"}
+!3 = !DILocation(line: 3, column: 20, scope: !4)
+!4 = !DISubprogram(name: "do_not_interleave", scope: !5, file: !5, line: 1, type: !6, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: true, function: void (float*, float*, i32)* @do_not_interleave, variables: !7)
+!5 = !DIFile(filename: "vectorization-remarks-profitable.c", directory: "")
+!6 = !DISubroutineType(types: !7)
+!7 = !{}
+!8 = !DILocation(line: 3, column: 3, scope: !4)
+!9 = !DILocation(line: 4, column: 14, scope: !4)
+!10 = !DILocation(line: 6, column: 1, scope: !4)
+!11 = !{!12, !12, i64 0}
+!12 = !{!"float", !13, i64 0}
+!13 = !{!"omnipotent char", !14, i64 0}
+!14 = !{!"Simple C/C++ TBAA"}
+!15 = !DILocation(line: 4, column: 5, scope: !4)
+!16 = !DILocation(line: 4, column: 12, scope: !4)
+!17 = distinct !{!17, !18, !19}
+!18 = !{!"llvm.loop.interleave.count", i32 1}
+!19 = !{!"llvm.loop.unroll.disable"}
+!20 = !DILocation(line: 10, column: 20, scope: !21)
+!21 = !DISubprogram(name: "interleave_not_profitable", scope: !5, file: !5, line: 8, type: !6, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: true, function: void (float*, float*, i32)* @interleave_not_profitable, variables: !7)
+!22 = !DILocation(line: 10, column: 3, scope: !21)
+!23 = !DILocation(line: 11, column: 14, scope: !21)
+!24 = !DILocation(line: 13, column: 1, scope: !21)
+!25 = !DILocation(line: 11, column: 5, scope: !21)
+!26 = !DILocation(line: 11, column: 12, scope: !21)
+!27 = distinct !{!27, !19}

test/Transforms/LoopVectorize/X86/vectorization-remarks.ll

 ; RUN: opt < %s -loop-vectorize -mtriple=x86_64-unknown-linux -S -pass-remarks='loop-vectorize' 2>&1 | FileCheck -check-prefix=VECTORIZED %s
 ; RUN: opt < %s -loop-vectorize -force-vector-width=1 -force-vector-interleave=4 -mtriple=x86_64-unknown-linux -S -pass-remarks='loop-vectorize' 2>&1 | FileCheck -check-prefix=UNROLLED %s
 ; RUN: opt < %s -loop-vectorize -force-vector-width=1 -force-vector-interleave=1 -mtriple=x86_64-unknown-linux -S -pass-remarks-analysis='loop-vectorize' 2>&1 | FileCheck -check-prefix=NONE %s
 
 ; This code has all the !dbg annotations needed to track source line information,
 ; but is missing the llvm.dbg.cu annotation. This prevents code generation from
 ; emitting debug info in the final output.
 ; RUN: llc < %s -mtriple x86_64-pc-linux-gnu -o - | FileCheck -check-prefix=DEBUG-OUTPUT %s
 ; DEBUG-OUTPUT-NOT: .loc
 ; DEBUG-OUTPUT-NOT: {{.*}}.debug_info
 
-; NONE: remark: vectorization-remarks.c:17:8: loop not vectorized: vector width and interleave count are explicitly set to 1
 ; VECTORIZED: remark: vectorization-remarks.c:17:8: vectorized loop (vectorization width: 4, interleaved count: 1)
-; UNROLLED: remark: vectorization-remarks.c:17:8: interleaved by 4 (vectorization not beneficial)
+; UNROLLED: remark: vectorization-remarks.c:17:8: interleaved loop (interleaved count: 4)
+; NONE: remark: vectorization-remarks.c:17:8: loop not vectorized: vectorization and interleaving are explicitly disabled, or vectorize width and interleave count are both set to 1
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 define i32 @foo(i32 %n) #0 {
 entry:
   %diff = alloca i32, align 4
   %cb = alloca [16 x i8], align 16
   %cc = alloca [16 x i8], align 16