[LV] Fix PR45525: Incorrect assert in blend recipe

[gilr]

Summary

Fix an assert introduced in 41ed5d856c1: a phi with a single predecessor and a mask is a valid case, already supported.

[fhahn]

I think this is related to PR44800, which has similarly redundant phis, but with 2 incoming values from the same block. Relaxing the assertion seems a fine workaround for now, but I think ideally we would eliminate such redundant PHIs in VPlan later on.

LGTM, with some suggestions for the test. It would be good to wait with committing a day or so, in case there are additional comments.

llvm/test/Transforms/LoopVectorize/pr45525.ll

3

Should this have -force-vector-width=4 or something, to not rely on the cost model?

45

it would be nice to rename the values to have a bit more compact/descriptive names, e.g. %iv and %iv.next and so on instead of the longer names.

46

Branch on undef is UB, please use a different branch condition (e.g. through a parameter).

49

Better to also have a concrete value instead of undef. Also, might be good to have a user of the value to make the test more robust (in case we decide to remove unused values in a VP2VP transform later on).

[fhahn]

Oh and it would be great if you could add a bit more details to the commit message title :)

[gilr]

Addressed comments and narrowed LIT checks to the relevant vectorized instructions.
Thanks @fhahn !

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

void VPBlendRecipe::execute(VPTransformState &State) {
   // At this point we generate the predication tree. There may be
   // duplications since this is a simple recursive scan, but future
   // optimizations will clean it up.
 
   unsigned NumIncoming = getNumIncomingValues();
 
   // Generate a sequence of selects of the form:
   // SELECT(Mask3, In3,
-  //      SELECT(Mask2, In2,
-  //                   ( ...)))
+  //        SELECT(Mask2, In2,
+  //               SELECT(Mask1, In1,
+  //                      In0)))
+  // Note that Mask0 is never used: lanes for which no path reaches this phi and
+  // are essentially undef are taken from In0.
   InnerLoopVectorizer::VectorParts Entry(State.UF);
   for (unsigned In = 0; In < NumIncoming; ++In) {
     for (unsigned Part = 0; Part < State.UF; ++Part) {
       // We might have single edge PHIs (blocks) - use an identity
       // 'select' for the first PHI operand.
       Value *In0 = State.get(getIncomingValue(In), Part);
       if (In == 0)
         Entry[Part] = In0; // Initialize with the first incoming value.

llvm/lib/Transforms/Vectorize/VPlan.h

 
 /// A recipe for vectorizing a phi-node as a sequence of mask-based select
 /// instructions.
 class VPBlendRecipe : public VPRecipeBase {
   PHINode *Phi;
 
   /// The blend operation is a User of the incoming values and of their
   /// respective masks, ordered [I0, M0, I1, M1, ...]. Note that a single value
-  /// would be incoming with a full mask for which there is no VPValue.
+  /// might be incoming with a full mask for which there is no VPValue.
   VPUser User;
 
 public:
   VPBlendRecipe(PHINode *Phi, ArrayRef<VPValue *> Operands)
       : VPRecipeBase(VPBlendSC), Phi(Phi), User(Operands) {
-    assert(((Operands.size() == 1) ||
-            (Operands.size() > 2 && Operands.size() % 2 == 0)) &&
-           "Expected either a single incoming value or a greater than two and "
-           "even number of operands");
+    assert(Operands.size() > 0 &&
+           ((Operands.size() == 1) || (Operands.size() % 2 == 0)) &&
+           "Expected either a single incoming value or a positive even number "
+           "of operands");
   }
 
   /// Method to support type inquiry through isa, cast, and dyn_cast.
   static inline bool classof(const VPRecipeBase *V) {
     return V->getVPRecipeID() == VPRecipeBase::VPBlendSC;
   }
 
   /// Return the number of incoming values, taking into account that a single

llvm/test/Transforms/LoopVectorize/pr45525.ll

+; RUN: opt < %s -loop-vectorize -force-vector-width=4 -S | FileCheck %s
+
+; Test case for PR45525. Checks that phi's with a single predecessor and a mask are supported.
+
+define void @main(i1 %cond, i32* %arr) {
+; CHECK-LABEL: @main(
+; CHECK-NEXT:  bb.0:
+; CHECK-NEXT:    br i1 false, label [[SCALAR_PH:%.*]], label [[VECTOR_PH:%.*]]
+; CHECK:       vector.ph:
+; CHECK:         br label [[VECTOR_BODY:%.*]]
+; CHECK:       vector.body:
+; CHECK:         [[VEC_IND:%.*]] = phi <4 x i32> [ <i32 0, i32 1, i32 2, i32 3>, [[VECTOR_PH]] ], [ [[VEC_IND_NEXT:%.*]], [[VECTOR_BODY]] ]
+; CHECK:         [[TMP5:%.*]] = mul <4 x i32> [[VEC_IND]], <i32 3, i32 3, i32 3, i32 3>
+;
+bb.0:
+  br label %bb.1
+
+bb.1:                                             ; preds = %bb.3, %bb.0
+  %iv = phi i32 [ 0, %bb.0 ], [ %iv.next, %bb.3 ]
+  br i1 %cond, label %bb.3, label %bb.2
+
+bb.2:                                             ; preds = %bb.1
+  %single.pred = phi i32 [ %iv, %bb.1 ]
+  %mult = mul i32 %single.pred, 3
+  br label %bb.3
+
+bb.3:                                             ; preds = %bb.2, %bb.1
+  %stored.value = phi i32 [ 7, %bb.1 ], [ %mult, %bb.2 ]
+  %arrayidx = getelementptr inbounds i32, i32* %arr, i32 %iv
+  store i32 %stored.value, i32* %arrayidx
+  %iv.next = add i32 %iv, 1
+  %continue = icmp ult i32 %iv.next, 32
+  br i1 %continue, label %bb.1, label %bb.4
+
+bb.4:                                             ; preds = %bb.3
+  ret void
+}