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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
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.
----------------
Ayal wrote:
> "drop" got dropped
Adjusted in the committed version, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:1348
     if (blockNeedsPredication(BB)) {
-      if (!blockCanBePredicated(BB, SafePointers, MaskedOp,
-                                ConditionalAssumes)) {
+      if (!blockCanBePredicated(BB, SafePointers, MaskedOp)) {
         reportVectorizationFailure(
----------------
Ayal wrote:
> nit: combine into a single `if`
Adjusted in the committed version ,thanks!


================
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");
----------------
Ayal wrote:
> 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.
Added in the committed version, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157034