!fixup address comments, thanks

Author: fhahn

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

@@ -1268,6 +1268,9 @@ bool VPInstruction::onlyFirstLaneUsed(const VPValue *Op) const {
     return true;
   case VPInstruction::BuildStructVector:
   case VPInstruction::BuildVector:
+    // Before replicating by VF, Build(Struct)Vector uses all lanes of the
+    // operand, after replicating its operands only the first lane is used.
+    // Before replicating, it will only have a single operand.
     return getNumOperands() > 1;
   case VPInstruction::PtrAdd:
     return Op == getOperand(0) || vputils::onlyFirstLaneUsed(this);

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

@@ -3706,7 +3706,7 @@ void VPlanTransforms::materializeBuildVectors(VPlan &Plan) {
     for (VPRecipeBase &R : make_early_inc_range(*VPBB)) {
       if (!isa<VPReplicateRecipe, VPInstruction>(&R))
         continue;
-      auto *DefR = dyn_cast<VPRecipeWithIRFlags>(&R);
+      auto *DefR = cast<VPRecipeWithIRFlags>(&R);
       auto UsesVectorOrInsideReplicateRegion = [DefR, LoopRegion](VPUser *U) {
         VPRegionBlock *ParentRegion =
             cast<VPRecipeBase>(U)->getParent()->getParent();
@@ -3715,8 +3715,8 @@ void VPlanTransforms::materializeBuildVectors(VPlan &Plan) {
       if ((isa<VPReplicateRecipe>(DefR) &&
            cast<VPReplicateRecipe>(DefR)->isSingleScalar()) ||
           (isa<VPInstruction>(DefR) &&
-           !cast<VPInstruction>(DefR)->doesGeneratePerAllLanes()) ||
-          vputils::onlyFirstLaneUsed(DefR) ||
+           (vputils::onlyFirstLaneUsed(DefR) ||
+            !cast<VPInstruction>(DefR)->doesGeneratePerAllLanes())) ||
           none_of(DefR->users(), UsesVectorOrInsideReplicateRegion))
         continue;
 

llvm/lib/Transforms/Vectorize/VPlanTransforms.h

@@ -160,8 +160,8 @@ struct VPlanTransforms {
 
   /// Replace each replicating VPReplicateRecipe and VPInstruction outside of
   /// any replicate region in \p Plan with \p VF single-scalar recipes.
-  /// TODO: Also replicate VPReplicateRecipes inside replicate regions, thereby
-  /// dissolving the latter.
+  /// TODO: Also replicate VPScalarIVSteps and VPReplicateRecipes inside
+  /// replicate regions, thereby dissolving the latter.
   static void replicateByVF(VPlan &Plan, ElementCount VF);
 
   /// Optimize \p Plan based on \p BestVF and \p BestUF. This may restrict the

llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp

@@ -463,8 +463,8 @@ void VPlanTransforms::unrollByUF(VPlan &Plan, unsigned UF) {
   VPlanTransforms::removeDeadRecipes(Plan);
 }
 
-/// Create a single-scalar clone of \p DefR (must either be a VPReplicateRecipe
-/// or VPInstruction) for lane \p Lane. Use \p Def2LaneDefs to look up scalar
+/// Create a single-scalar clone of \p DefR (must be a VPReplicateRecipe or
+/// VPInstruction) for lane \p Lane. Use \p Def2LaneDefs to look up scalar
 /// definitions for operands of \DefR.
 static VPRecipeWithIRFlags *
 cloneForLane(VPlan &Plan, VPBuilder &Builder, Type *IdxTy,
@@ -512,7 +512,7 @@ cloneForLane(VPlan &Plan, VPBuilder &Builder, Type *IdxTy,
                               /*IsSingleScalar=*/true, /*Mask=*/nullptr, *RepR);
   } else {
     assert(isa<VPInstruction>(DefR) &&
-           "DefR must either be a VPReplicateRecipe or VPInstruction");
+           "DefR must be a VPReplicateRecipe or VPInstruction");
     New = DefR->clone();
     for (const auto &[Idx, Op] : enumerate(NewOps)) {
       New->setOperand(Idx, Op);
@@ -551,7 +551,7 @@ void VPlanTransforms::replicateByVF(VPlan &Plan, ElementCount VF) {
            !cast<VPInstruction>(&R)->doesGeneratePerAllLanes()))
         continue;
 
-      auto *DefR = dyn_cast<VPRecipeWithIRFlags>(&R);
+      auto *DefR = cast<VPRecipeWithIRFlags>(&R);
       VPBuilder Builder(DefR);
       if (DefR->getNumUsers() == 0) {
         // Create single-scalar version of DefR for all lanes.