[PATCH] D103700: [LV] Fix bug when unrolling (only) a loop with non-latch exit

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:3175
+  // iterations check ensures that N >= Step.
+  if (Cost->requiresScalarEpilogue()) {
     auto *IsZero = Builder.CreateICmpEQ(R, ConstantInt::get(R->getType(), 0));
----------------
Ayal wrote:
> What if a loop has a single exiting block - the loop latch, and an interleave group that requires a scalar epilog, but we decide to unroll the loop w/o vectorizing it? Such a (test)case should be unrolled w/o a scalar epilog.
> 
> Note that InterleaveInfo.requiresScalarEpilogue() is relevant only when vectorizing but is otherwise independent of VF, so should force a scalar epilog in conjunction with the original "if (VF > 1)" of D19487. Exiting from a non-latch block, OTOH, as introduced in D93317, should force a scalar epilog for any VF, including 1.
> 
> Also curious if D94892 should be applicable to epilog vectorization, as commented there.
> What if a loop has a single exiting block - the loop latch, and an interleave group that requires a scalar epilog, but we decide to unroll the loop w/o vectorizing it? Such a (test)case should be unrolled w/o a scalar epilog.
We could reasonably decide that such a loop does not require a scalar epilogue, but if the cost model decides it does (as it might today), code generation had better be consistent about it.  That's all this patch does.  
> 
> Note that InterleaveInfo.requiresScalarEpilogue() is relevant only when vectorizing but is otherwise independent of VF, so should force a scalar epilog in conjunction with the original "if (VF > 1)" of D19487. Exiting from a non-latch block, OTOH, as introduced in D93317, should force a scalar epilog for any VF, including 1.
I don't follow your comment here.  As demonstrated by the test case, we do need to generate an epilogue loop in some cases even when not vectorizing. 
> 
> Also curious if D94892 should be applicable to epilog vectorization, as commented there.
(will reply there)



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103700/new/

https://reviews.llvm.org/D103700