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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1266
 
+  /// \return true when there is a scalar instruction with predicate in a loop.
+  bool loopHasPredicatedScalar();
----------------
nit: when the loop contains a predicated instruction that requires scalarization.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1267
+  /// \return true when there is a scalar instruction with predicate in a loop.
+  bool loopHasPredicatedScalar();
+
----------------
nit: `loopHasScalarWithPredication`


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7792
+        continue;
+      if (isScalarWithPredication(&I))
+        return true;
----------------
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`.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7789-7791
+      // Skip ignored values.
+      if (ValuesToIgnore.count(&I) || (VecValuesToIgnore.count(&I)))
+        continue;
----------------
is this condition necessary? I would expect any instructions that explicitly need predication+scalarization, to never be values that can be ignored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101916