Addressed comments

Author: nikolaypanchenko

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -1458,7 +1458,7 @@ class VPWidenRecipe : public VPRecipeWithIRFlags {
 #endif
 };
 
-/// A recipe for widening operations with vector-predication intrinsics.with
+/// A recipe for widening operations with vector-predication intrinsics with
 /// explicit vector length (EVL).
 class VPWidenEVLRecipe : public VPWidenRecipe {
   using VPRecipeWithIRFlags::transferFlags;
@@ -1469,9 +1469,9 @@ class VPWidenEVLRecipe : public VPWidenRecipe {
       : VPWidenRecipe(VPDef::VPWidenEVLSC, I, Operands) {
     addOperand(&EVL);
   }
-  VPWidenEVLRecipe(VPWidenRecipe *W, VPValue &EVL)
-      : VPWidenEVLRecipe(*W->getUnderlyingInstr(), W->operands(), EVL) {
-    this->transferFlags(*W);
+  VPWidenEVLRecipe(VPWidenRecipe &W, VPValue &EVL)
+      : VPWidenEVLRecipe(*W.getUnderlyingInstr(), W.operands(), EVL) {
+    transferFlags(W);
   }
 
   ~VPWidenEVLRecipe() override = default;
@@ -1491,7 +1491,13 @@ class VPWidenEVLRecipe : public VPWidenRecipe {
   void execute(VPTransformState &State) override final;
 
   /// Returns true if the recipe only uses the first lane of operand \p Op.
-  bool onlyFirstLaneUsed(const VPValue *Op) const override;
+  bool onlyFirstLaneUsed(const VPValue *Op) const override {
+    assert(is_contained(operands(), Op) &&
+           "Op must be an operand of the recipe");
+    // EVL in that recipe is always the last operand, thus any use before means
+    // the VPValue should be vectorized.
+    return getEVL() == Op;
+  }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   /// Print the recipe.
@@ -2645,11 +2651,6 @@ struct VPWidenLoadEVLRecipe final : public VPWidenMemoryRecipe, public VPValue {
     setMask(Mask);
   }
 
-  VPWidenLoadEVLRecipe *clone() override {
-    llvm_unreachable("VPWidenLoadEVLRecipe cannot be cloned");
-    return nullptr;
-  }
-
   VP_CLASSOF_IMPL(VPDef::VPWidenLoadEVLSC)
 
   /// Return the EVL operand.
@@ -2725,11 +2726,6 @@ struct VPWidenStoreEVLRecipe final : public VPWidenMemoryRecipe {
     setMask(Mask);
   }
 
-  VPWidenStoreEVLRecipe *clone() override {
-    llvm_unreachable("VPWidenStoreEVLRecipe cannot be cloned");
-    return nullptr;
-  }
-
   VP_CLASSOF_IMPL(VPDef::VPWidenStoreEVLSC)
 
   /// Return the address accessed by this recipe.

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

@@ -1212,12 +1212,6 @@ InstructionCost VPWidenRecipe::computeCost(ElementCount VF,
   }
 }
 
-VPWidenEVLRecipe *VPWidenEVLRecipe::create(VPWidenRecipe *W, VPValue &EVL) {
-  auto *R = new VPWidenEVLRecipe(*W->getUnderlyingInstr(), W->operands(), EVL);
-  R->transferFlags(*W);
-  return R;
-}
-
 void VPWidenEVLRecipe::execute(VPTransformState &State) {
   State.setDebugLocFrom(getDebugLoc());
   assert(State.UF == 1 && "Expected only UF == 1 when vectorizing with "
@@ -1259,13 +1253,6 @@ void VPWidenEVLRecipe::execute(VPTransformState &State) {
   State.addMetadata(VPInst, I);
 }
 
-bool VPWidenEVLRecipe::onlyFirstLaneUsed(const VPValue *Op) const {
-  assert(is_contained(operands(), Op) && "Op must be an operand of the recipe");
-  // EVL in that recipe is always the last operand, thus any use before means
-  // the VPValue should be vectorized.
-  return getEVL() == Op;
-}
-
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 void VPWidenRecipe::print(raw_ostream &O, const Twine &Indent,
                           VPSlotTracker &SlotTracker) const {

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

@@ -1373,29 +1373,30 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
                 if (!Instruction::isBinaryOp(Opcode) &&
                     !Instruction::isUnaryOp(Opcode))
                   return nullptr;
-                return new VPWidenEVLRecipe(W, EVL);
+                return new VPWidenEVLRecipe(*W, EVL);
               })
               .Case<VPReductionRecipe>([&](VPReductionRecipe *Red) {
-                return new VPReductionEVLRecipe(*Red, EVL,
-                                                GetNewMask(Red->getCondOp()));
+                VPValue *NewMask = GetNewMask(Red->getCondOp());
+                return new VPReductionEVLRecipe(*Red, EVL, NewMask);
               })
               .Default([&](VPRecipeBase *R) { return nullptr; });
 
-      if (NewRecipe) {
-        [[maybe_unused]] unsigned NumDefVal = NewRecipe->getNumDefinedValues();
-        assert(NumDefVal == CurRecipe->getNumDefinedValues() &&
-               "New recipe must define the same number of values as the "
-               "original.");
-        assert(
-            NumDefVal <= 1 &&
-            "Only supports recipes with a single definition or without users.");
-        NewRecipe->insertBefore(CurRecipe);
-        if (isa<VPSingleDefRecipe, VPWidenLoadEVLRecipe>(NewRecipe)) {
-          VPValue *CurVPV = CurRecipe->getVPSingleValue();
-          CurVPV->replaceAllUsesWith(NewRecipe->getVPSingleValue());
-        }
-        CurRecipe->eraseFromParent();
+      if (!NewRecipe)
+        continue;
+
+      [[maybe_unused]] unsigned NumDefVal = NewRecipe->getNumDefinedValues();
+      assert(NumDefVal == CurRecipe->getNumDefinedValues() &&
+             "New recipe must define the same number of values as the "
+             "original.");
+      assert(
+          NumDefVal <= 1 &&
+          "Only supports recipes with a single definition or without users.");
+      NewRecipe->insertBefore(CurRecipe);
+      if (isa<VPSingleDefRecipe, VPWidenLoadEVLRecipe>(NewRecipe)) {
+        VPValue *CurVPV = CurRecipe->getVPSingleValue();
+        CurVPV->replaceAllUsesWith(NewRecipe->getVPSingleValue());
       }
+      CurRecipe->eraseFromParent();
     }
     recursivelyDeleteDeadRecipes(HeaderMask);
   }