[PATCH] D101916: [LoopVectorize] Fix crach for predicated instruction with scalable VF

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7792
+        continue;
+      if (isScalarWithPredication(&I))
+        return true;
----------------
sdesmalen wrote:
> There is a problem with this approach, in that `isScalarWithPredication` calls `blockNeedsPredication`, which in turn calls `foldTailByMasking`, whose return value is determined //after// calling `computeFeasibleMaxVF`, which ends up calling this function in order to determine the min/max VF. This may lead to unexpected results when we want to use scalable vectors for tail-folded (predicated) loops, so you'll need to figure out a better place to call `loopHasPredicatedScalar`.
> 
> Additionally, it might be useful to add a mechanism that records if FoldTailByMasking already has its final value, and then assert that in `isScalarWithPredication`.
OK that's a good point. It's tricky because we also need to ensure we call this before attempting to calculate any costs as we'll hit asserts in computePredInstDiscount again. If we didn't have to worry about `blockNeedsPredication`, this seems like a sensible place to bail out because we have to also worry about the general case without hints, i.e. we should really be telling the computeMaxVF code the max legal scalable VF = 0 as I believe this is how you've been recently structuring the code to work?

I realise this is not an ideal solution, but what if for now we could avoid concerns about tail folding by somehow marking tail folding as not an option for scalable vectors, since we don't support it anyway for scalable vectors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101916