[PATCH] D143865: [VPlan] Add predicate to VPReplicateRecipe, expand region later.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8701
   auto *Entry = new VPBasicBlock(Twine(RegionName) + ".entry", BOMRecipe);
   auto *PHIRecipe = Instr->getType()->isVoidTy()
                         ? nullptr
----------------
Ayal wrote:
> nit: now that this is a VPlan2VPlan transformation, check if PredRecipe has users instead of Instr's type?
> 
> While we're here, could perhaps be simplified into (independent of this patch):
> 
> ```
> VPRecipeBase *PHIRecipe = nullptr;
> if (PredRecipe has users outside its block) {
>   PHIRecipe = new VPPredInstPHIRecipe(RecipeWithoutMask);
>   RecipeWithoutMask->replaceAllUsesWith(PHIRecipe);
>   PHIRecipe->setOperand(0, RecipeWithoutMask);
> }
> ```
Updated to check the users, I'll do the other simplification separately.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9186
     ShouldSimplify |= VPlanTransforms::mergeBlocksIntoPredecessors(*Plan);
   }
 
----------------
Ayal wrote:
> nit: the above creation and iterative optimization of replicate regions could be folded into some `VPlanTransforms::createAndOptimizeReplicateRegions(*Plan, RecipeBuilder);`
Will do separately!


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1513
+        IsUniform(IsUniform), IsPredicated(IsPredicated) {
+    assert((!IsPredicated || Mask) && "Mask must be set if IsPredicate is set");
+    if (IsPredicated)
----------------
Ayal wrote:
> Passing `IsPredicated` is now redundant - can be inferred by checking if given `Mask` is null?
yes, I removed the option, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143865