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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 4 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8606
+    VPValue *BlockInMask = createBlockInMask(I->getParent(), Plan);
+    Recipe->addOperand(BlockInMask);
+  }
----------------
Ayal wrote:
> nit: worth recording as Mask to be returned by getMask(), initially set to null, rather than having the latter return the last operand blindly below?
I updated the code to add the mask in the constructer, iff IsPredicated is set.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8618
   assert(Instr->getParent() && "Predicated instruction not in any basic block");
+  auto *BlockInMask = PredRecipe->getOperand(PredRecipe->getNumOperands() - 1);
+  auto *RecipeWithoutMask = new VPReplicateRecipe(
----------------
Ayal wrote:
> nit: worth adding a comment that the masked replicate recipe is replaced by a non-masked one?
Added a comment, thanks!


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/consecutive-ptr-uniforms.ll:95
 ; FORCE-NEXT:    br i1 [[TMP3]], label [[PRED_LOAD_IF:%.*]], label [[PRED_LOAD_CONTINUE:%.*]]
-; FORCE:       pred.load.if:
+; FORCE:       pred.store.if:
 ; FORCE-NEXT:    [[TMP0:%.*]] = add i32 [[INDEX]], 0
----------------
Ayal wrote:
> nit: name of label fixed? Worth fixing them PRED_LOAD* names as well for consistency?
Fixed, thanks!


================
Comment at: llvm/test/Transforms/LoopVectorize/reduction-inloop-cond.ll:411
+; CHECK-NEXT:    [[TMP37:%.*]] = load i32, ptr [[TMP36]], align 4
+; CHECK-NEXT:    [[TMP38:%.*]] = insertelement <4 x i32> [[TMP30]], i32 [[TMP37]], i64 3
 ; CHECK-NEXT:    br label [[PRED_LOAD_CONTINUE6]]
----------------
Ayal wrote:
> Two independent replicating regions with a reduce.add between them that were kept separate are now being merged? This patch seems conceptually an NFC, according to its current commit message.
In this case, all recipes are in the same VPBasicBlock when `adjustRecipesForReductions` is called, which will add the reduction recipes at the end of the block containing the original widen recipes, hence the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143865