[PATCH] D157034: [VPlan] Model masked assumes as replicate recipes, drop them (NFCI).

[Ayal Zaks via Phabricator via llvm-commits]

Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

Looks good to me! Adding a couple of last nits.



================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:534
   /// While vectorizing these instructions we have to generate a
-  /// call to the appropriate masked intrinsic
+  /// call to the appropriate masked intrinsic or them in case of conditional
+  /// assumes.
----------------
"drop" got dropped


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:1348
     if (blockNeedsPredication(BB)) {
-      if (!blockCanBePredicated(BB, SafePointers, MaskedOp,
-                                ConditionalAssumes)) {
+      if (!blockCanBePredicated(BB, SafePointers, MaskedOp)) {
         reportVectorizationFailure(
----------------
nit: combine into a single `if`


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:1560
   for (BasicBlock *BB : TheLoop->blocks()) {
-    if (!blockCanBePredicated(BB, SafePointers, TmpMaskedOp,
-                              TmpConditionalAssumes)) {
+    if (!blockCanBePredicated(BB, SafePointers, TmpMaskedOp)) {
       LLVM_DEBUG(dbgs() << "LV: Cannot fold tail by masking as requested.\n");
----------------
fhahn wrote:
> Ayal wrote:
> > nit (independent of this patch): pass MaskedOp instead of filling TmpMaskedOp and copying it over?
> Hmm, this might be intentional, to avoid populating `MaskedOp` partially if any block cannot be predicated.
Ahh, right, returning false may still end up vectorizing. Worth a comment next to TmpMaskedOp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157034