Addressed comments

Author: nikolaypanchenko

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -8458,14 +8458,15 @@ void LoopVectorizationPlanner::buildVPlansWithVPRecipes(ElementCount MinVF,
   for (ElementCount VF = MinVF; ElementCount::isKnownLT(VF, MaxVFTimes2);) {
     VFRange SubRange = {VF, MaxVFTimes2};
     if (auto Plan = tryToBuildVPlanWithVPRecipes(SubRange)) {
+      bool IsScalarVPlan = Plan->hasVF(ElementCount::getFixed(1));
       // Now optimize the initial VPlan.
-      if (!Plan->hasVF(ElementCount::getFixed(1)))
+      if (!IsScalarVPlan)
         VPlanTransforms::truncateToMinimalBitwidths(
             *Plan, CM.getMinimalBitwidths(), PSE.getSE()->getContext());
       VPlanTransforms::optimize(*Plan, *PSE.getSE());
       // TODO: try to put it close to addActiveLaneMask().
       // Discard the plan if it is not EVL-compatible
-      if (CM.foldTailWithEVL() &&
+      if (!IsScalarVPlan && CM.foldTailWithEVL() &&
           !VPlanTransforms::tryAddExplicitVectorLength(*Plan))
         break;
       assert(verifyVPlanIsValid(*Plan) && "VPlan is invalid");

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -1347,9 +1347,9 @@ class VPInstruction : public VPRecipeWithIRFlags {
 /// ingredient. This recipe covers most of the traditional vectorization cases
 /// where each ingredient transforms into a vectorized version of itself.
 class VPWidenRecipe : public VPRecipeWithIRFlags {
-protected:
   unsigned Opcode;
 
+protected:
   template <typename IterT>
   VPWidenRecipe(unsigned VPDefOpcode, Instruction &I,
                 iterator_range<IterT> Operands)
@@ -1383,7 +1383,6 @@ class VPWidenRecipe : public VPRecipeWithIRFlags {
 };
 
 class VPWidenEVLRecipe : public VPWidenRecipe {
-private:
   using VPRecipeWithIRFlags::transferFlags;
 
 public:
@@ -1392,6 +1391,10 @@ 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() override = default;
 
@@ -1409,9 +1412,6 @@ class VPWidenEVLRecipe : public VPWidenRecipe {
   VPValue *getEVL() { return getOperand(getNumOperands() - 1); }
   const VPValue *getEVL() const { return getOperand(getNumOperands() - 1); }
 
-  /// A helper function to create widen EVL recipe from regular widen recipe.
-  static VPWidenEVLRecipe *create(VPWidenRecipe *W, VPValue &EVL);
-
   /// Produce widened copies of all Ingredients.
   void execute(VPTransformState &State) override final;
 

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

@@ -253,7 +253,8 @@ Type *VPTypeAnalysis::inferScalarType(const VPValue *V) {
             return inferScalarType(R->getOperand(0));
           })
           .Case<VPBlendRecipe, VPInstruction, VPWidenRecipe, VPReplicateRecipe,
-                VPWidenCallRecipe, VPWidenMemoryRecipe, VPWidenSelectRecipe>(
+                VPWidenCallRecipe, VPWidenMemoryRecipe, VPWidenSelectRecipe,
+                VPWidenEVLRecipe>(
               [this](const auto *R) { return inferScalarTypeForRecipe(R); })
           .Case<VPInterleaveRecipe>([V](const VPInterleaveRecipe *R) {
             // TODO: Use info from interleave group.

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

@@ -1054,20 +1054,14 @@ void VPWidenRecipe::execute(VPTransformState &State) {
 #endif
 }
 
-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) {
   assert(State.UF == 1 && "Expected only UF == 1 when vectorizing with "
                           "explicit vector length.");
   VPValue *Op0 = getOperand(0);
 
   // If it's scalar operation, hand translation over to VPWidenRecipe
-  if (!State.get(Op0, 0)->getType()->isVectorTy())
-    return VPWidenRecipe::execute(State);
+  assert(State.get(Op0, 0)->getType()->isVectorTy() &&
+         "VPWidenEVLRecipe should not be used for scalars");
 
   VPValue *EVL = getEVL();
   Value *EVLArg = State.get(EVL, 0, /*NeedsScalar=*/true);
@@ -1106,10 +1100,7 @@ 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.
-  for (unsigned I = 0, E = getNumOperands() - 1; I != E; ++I)
-    if (getOperand(I) == Op)
-      return false;
-  return true;
+  return getEVL() == Op;
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
@@ -1126,7 +1117,7 @@ void VPWidenEVLRecipe::print(raw_ostream &O, const Twine &Indent,
                              VPSlotTracker &SlotTracker) const {
   O << Indent << "WIDEN vp ";
   printAsOperand(O, SlotTracker);
-  O << " = " << Instruction::getOpcodeName(Opcode);
+  O << " = " << Instruction::getOpcodeName(getOpcode());
   printFlags(O);
   printOperands(O, SlotTracker);
 }

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

@@ -1385,16 +1385,19 @@ void VPlanTransforms::addActiveLaneMask(
 
 /// Replace recipes with their EVL variants.
 static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
+  VPDominatorTree VPDT;
+  VPDT.recalculate(Plan);
   DenseSet<VPRecipeBase *> ToRemove;
 
   ReversePostOrderTraversal<VPBlockDeepTraversalWrapper<VPBlockBase *>> RPOT(
       Plan.getEntry());
   DenseSet<VPValue *> HeaderMasks = collectAllHeaderMasks(Plan);
   for (VPBasicBlock *VPBB :
        reverse(VPBlockUtils::blocksOnly<VPBasicBlock>(RPOT))) {
-    // The recipes in the block are processed in reverse order, to catch chains
-    // of dead recipes.
     for (VPRecipeBase &R : make_early_inc_range(reverse(*VPBB))) {
+      if (!properlyDominates(EVL.getDefiningRecipe(), &R, VPDT))
+        continue;
+
       TypeSwitch<VPRecipeBase *>(&R)
           .Case<VPWidenLoadRecipe>([&](VPWidenLoadRecipe *L) {
             VPValue *NewMask =
@@ -1416,7 +1419,7 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
             if (!Instruction::isBinaryOp(Opcode) &&
                 !Instruction::isUnaryOp(Opcode))
               return;
-            auto *N = VPWidenEVLRecipe::create(W, EVL);
+            auto *N = new VPWidenEVLRecipe(W, EVL);
             N->insertBefore(W);
             W->replaceAllUsesWith(N);
             ToRemove.insert(W);
@@ -1431,8 +1434,6 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
     recursivelyDeleteDeadRecipes(HeaderMask);
 }
 
-
-
 /// Add a VPEVLBasedIVPHIRecipe and related recipes to \p Plan and
 /// replaces all uses except the canonical IV increment of
 /// VPCanonicalIVPHIRecipe with a VPEVLBasedIVPHIRecipe. VPCanonicalIVPHIRecipe

llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp

@@ -18,6 +18,7 @@
 #include "VPlanDominatorTree.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/TypeSwitch.h"
 #include "llvm/Support/CommandLine.h"
 
 #define DEBUG_TYPE "loop-vectorize"
@@ -35,6 +36,11 @@ class VPlanVerifier {
   // VPHeaderPHIRecipes.
   bool verifyPhiRecipes(const VPBasicBlock *VPBB);
 
+  // Verify that \p EVL is used correctly. The user must be  either in EVL-based
+  // recipes as a last operand or VPInstruction::Add which is incoming value
+  // into EVL's recipe.
+  bool verifyEVLRecipe(const VPInstruction &EVL) const;
+
   bool verifyVPBasicBlock(const VPBasicBlock *VPBB);
 
   bool verifyBlock(const VPBlockBase *VPB);
@@ -114,6 +120,64 @@ bool VPlanVerifier::verifyPhiRecipes(const VPBasicBlock *VPBB) {
   return true;
 }
 
+bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const {
+  if (EVL.getOpcode() != VPInstruction::ExplicitVectorLength) {
+    errs() << "verifyEVLRecipe should only be called on "
+              "VPInstruction::ExplicitVectorLength\n";
+    return false;
+  }
+  auto VerifyEVLUse = [&](const VPRecipeBase &R,
+                          const unsigned ExpectedIdx) -> bool {
+    SmallVector<const VPValue *> Ops(R.operands());
+    unsigned UseCount = count(Ops, &EVL);
+    if (UseCount != 1 || Ops[ExpectedIdx] != &EVL) {
+      errs() << "EVL is used as non-last operand in EVL-based recipe\n";
+      return false;
+    }
+    return true;
+  };
+  for (const VPUser *U : EVL.users()) {
+    if (!TypeSwitch<const VPUser *, bool>(U)
+             .Case<VPWidenStoreEVLRecipe>([&](const VPWidenStoreEVLRecipe *S) {
+               return VerifyEVLUse(*S, 2);
+             })
+             .Case<VPWidenLoadEVLRecipe>([&](const VPWidenLoadEVLRecipe *L) {
+               return VerifyEVLUse(*L, 1);
+             })
+             .Case<VPWidenEVLRecipe>([&](const VPWidenEVLRecipe *W) {
+               return VerifyEVLUse(
+                   *W, Instruction::isUnaryOp(W->getOpcode()) ? 1 : 2);
+             })
+             .Case<VPScalarCastRecipe>(
+                 [&](const VPScalarCastRecipe *S) { return true; })
+             .Case<VPInstruction>([&](const VPInstruction *I) {
+               if (I->getOpcode() != Instruction::Add) {
+                 errs()
+                     << "EVL is used as an operand in non-VPInstruction::Add\n";
+                 return false;
+               }
+               if (I->getNumUsers() != 1) {
+                 errs() << "EVL is used in VPInstruction:Add with multiple "
+                           "users\n";
+                 return false;
+               }
+               if (!isa<VPEVLBasedIVPHIRecipe>(*I->users().begin())) {
+                 errs() << "Result of VPInstruction::Add with EVL operand is "
+                           "not used by VPEVLBasedIVPHIRecipe\n";
+                 return false;
+               }
+               return true;
+             })
+             .Default([&](const VPUser *U) {
+               errs() << "EVL has unexpected user\n";
+               return false;
+             })) {
+      return false;
+    }
+  }
+  return true;
+}
+
 bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
   if (!verifyPhiRecipes(VPBB))
     return false;
@@ -150,6 +214,13 @@ bool VPlanVerifier::verifyVPBasicBlock(const VPBasicBlock *VPBB) {
         }
       }
     }
+    if (const auto *EVL = dyn_cast<VPInstruction>(&R)) {
+      if (EVL->getOpcode() == VPInstruction::ExplicitVectorLength &&
+          !verifyEVLRecipe(*EVL)) {
+        errs() << "EVL VPValue is not used correctly\n";
+        return false;
+      }
+    }
   }
 
   auto *IRBB = dyn_cast<VPIRBasicBlock>(VPBB);

llvm/test/Transforms/LoopVectorize/RISCV/vectorize-force-tail-with-evl-bin-unary-ops-args.ll

@@ -2,12 +2,12 @@
 ; RUN: opt -passes=loop-vectorize \
 ; RUN: -force-tail-folding-style=data-with-evl \
 ; RUN: -prefer-predicate-over-epilogue=predicate-dont-vectorize \
-; RUN: -mtriple=riscv64 -mattr=+v -S < %s | FileCheck %s --check-prefix=IF-EVL
+; RUN: -mtriple=riscv64 -mattr=+v -S %s | FileCheck %s --check-prefix=IF-EVL
 
 ; RUN: opt -passes=loop-vectorize \
 ; RUN: -force-tail-folding-style=none \
 ; RUN: -prefer-predicate-over-epilogue=predicate-dont-vectorize \
-; RUN: -mtriple=riscv64 -mattr=+v -S < %s | FileCheck %s --check-prefix=NO-VP
+; RUN: -mtriple=riscv64 -mattr=+v -S %s | FileCheck %s --check-prefix=NO-VP
 
 
 define void @test_and(ptr nocapture %a, ptr nocapture readonly %b) {