[PATCH] D10714: Modify interleave diagnostics to clearly indicate why interleaving wasn't done.

[Tyler Nowicki]

tyler.nowicki marked 2 inline comments as done.
tyler.nowicki added a comment.

See comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1575
@@ -1574,3 +1574,3 @@
     if (Hints.getWidth() == 1 && Hints.getInterleave() == 1) {
       DEBUG(dbgs() << "LV: Not vectorizing: Disabled/already vectorized.\n");
       emitOptimizationRemarkAnalysis(
----------------
Gerolf wrote:
> The comment in DEBUG ('already vectorized') is inconsistent with your comment.
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.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1578
@@ -1577,3 +1577,3 @@
           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 "
----------------
Gerolf wrote:
> Could there be two separated remarks something like a) not vectorized because turned off and b) not vectorized because of low thresholds
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.


http://reviews.llvm.org/D10714