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

1651

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

1678

add assert(IC > 1).

[tyler.nowicki]

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

[hfinkel]

lib/Transforms/Vectorize/LoopVectorize.cpp

1679

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

1682

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

1691

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

1573

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

1576–1578

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

1573

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.

1576–1578

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

1573

Ok. Please keep that thought in a FIXME.

1576–1578

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) {
       DEBUG(dbgs() << "LV: Not vectorizing: Disabled/already vectorized.\n");
       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);
 
+    // Get user interleave count.
+    unsigned UserIC = Hints.getInterleave();
+
+    if (VF.Width == 1 &&
+        ((IC == 1 && UserIC <= 1) || (IC > 1 && UserIC == 1))) {
+      DEBUG(dbgs() << "LV: Vectorization is possible but not beneficial and "
+                      "interleaving is not beneficial or is explicitly "
+                      "disabled.\n");
+      Twine interleaveDiagMsg =
+          Twine("vectorization ") +
+          (IC == 1
+               ? Twine("and interleaving are not beneficial") +
+                     (UserIC == 1
+                          ? Twine(", interleaving is also explicitly disabled "
+                                  "or interleave count is set to 1")
+                          : Twine(""))
+               : Twine(" is not beneficial, interleaving is beneficial but is "
+                       "explicitly disabled or interleave count is set to 1"));
+      emitOptimizationRemarkAnalysis(F->getContext(), DEBUG_TYPE, *F,
+                                     L->getStartLoc(), interleaveDiagMsg);
+      return false;
+    }
+
+    // Override IC if user provided an interleave count.
+    IC = UserIC > 0 ? UserIC : IC;
+
     DEBUG(dbgs() << "LV: Found a vectorizable loop (" << VF.Width << ") in "
                  << DebugLocStr << '\n');
     DEBUG(dbgs() << "LV: Interleave Count is " << IC << '\n');
 
     if (VF.Width == 1) {
-      DEBUG(dbgs() << "LV: Vectorization is possible but not beneficial\n");
-
-      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");
+      DEBUG(dbgs()
+            << "LV: Vectorization not beneficial, interleaving the loop.\n");
 
       // Report the unrolling decision.
       emitOptimizationRemark(F->getContext(), DEBUG_TYPE, *F, L->getStartLoc(),
                              Twine("interleaved by " + Twine(IC) +
                                    " (vectorization not beneficial)"));
 
       InnerLoopUnroller Unroller(L, SE, LI, DT, TLI, TTI, IC);
       Unroller.vectorize(&LVL);
     } 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,
       // scalar loop is rarely used not worthy to be unrolled.
       if (!LB.IsSafetyChecksAdded())
         AddRuntimeUnrollDisableMetaData(L);

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: vectorization and interleaving are not beneficial, interleaving is also explicitly disabled or interleave count is set to 1
+; CHECK: remark: vectorization-remarks-profitable.c:11:14: vectorization and interleaving are 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
 
-; VECTORIZED: remark: vectorization-remarks.c:17:8: vectorized loop (vectorization factor: 4, unrolling interleave factor: 1)
-; UNROLLED: remark: vectorization-remarks.c:17:8: unrolled with interleaving factor 4 (vectorization not beneficial)
-; 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)
+; 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