[LoopVectorize] LLVM fails to vectorise loops with multi-bool varables #89226

dtemirbulatov · 2024-04-18T12:31:34Z

This patch allows to consider compare instructions in the loop with multiple use inside the loop and outside.

This change allows to vectorise this loop:
int foo(float* a, int n) {
_Bool any = 0;
_Bool all = 1;
for (int i = 0; i < n; i++) {
if (a[i] < 0.0f) {
any = 1;
} else {
all = 0;
}
}
return all ? 1 : any ? 2 : 3;
}

llvmbot · 2024-04-18T12:32:06Z

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: Dinar Temirbulatov (dtemirbulatov)

Changes

This patch allows to consider compare instructions in the loop with multiple use inside the loop and outside, if we can prove that compare instruction user is a recurrent reduction or used in branching or outside the loop then it is safe to consider to vectorise.

This change allows to vectorise this loop:
int foo(float* a, int n) {
_Bool any = 0;
_Bool all = 1;
for (int i = 0; i < n; i++) {
if (a[i] < 0.0f) {
any = 1;
} else {
all = 0;
}
}
return all ? 1 : any ? 2 : 3;
}

Patch is 58.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89226.diff

4 Files Affected:

(modified) llvm/include/llvm/Analysis/IVDescriptors.h (+14-6)
(modified) llvm/lib/Analysis/IVDescriptors.cpp (+58-7)
(modified) llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp (+25)
(added) llvm/test/Transforms/LoopVectorize/multicmp.ll (+794)

diff --git a/llvm/include/llvm/Analysis/IVDescriptors.h b/llvm/include/llvm/Analysis/IVDescriptors.h
index 5c7b613ac48c40..f18ab500c4d9fa 100644
--- a/llvm/include/llvm/Analysis/IVDescriptors.h
+++ b/llvm/include/llvm/Analysis/IVDescriptors.h
@@ -76,11 +76,11 @@ class RecurrenceDescriptor {
                        RecurKind K, FastMathFlags FMF, Instruction *ExactFP,
                        Type *RT, bool Signed, bool Ordered,
                        SmallPtrSetImpl<Instruction *> &CI,
-                       unsigned MinWidthCastToRecurTy)
+                       unsigned MinWidthCastToRecurTy, Instruction *Cmp)
       : IntermediateStore(Store), StartValue(Start), LoopExitInstr(Exit),
         Kind(K), FMF(FMF), ExactFPMathInst(ExactFP), RecurrenceType(RT),
         IsSigned(Signed), IsOrdered(Ordered),
-        MinWidthCastToRecurrenceType(MinWidthCastToRecurTy) {
+        MinWidthCastToRecurrenceType(MinWidthCastToRecurTy), MultiCmp(Cmp) {
     CastInsts.insert(CI.begin(), CI.end());
   }
 
@@ -88,12 +88,13 @@ class RecurrenceDescriptor {
   class InstDesc {
   public:
     InstDesc(bool IsRecur, Instruction *I, Instruction *ExactFP = nullptr)
-        : IsRecurrence(IsRecur), PatternLastInst(I),
-          RecKind(RecurKind::None), ExactFPMathInst(ExactFP) {}
+        : IsRecurrence(IsRecur), PatternLastInst(I), RecKind(RecurKind::None),
+          ExactFPMathInst(ExactFP), Cmp(nullptr) {}
 
-    InstDesc(Instruction *I, RecurKind K, Instruction *ExactFP = nullptr)
+    InstDesc(Instruction *I, RecurKind K, Instruction *ExactFP = nullptr,
+             Instruction *MultiCmp = nullptr)
         : IsRecurrence(true), PatternLastInst(I), RecKind(K),
-          ExactFPMathInst(ExactFP) {}
+          ExactFPMathInst(ExactFP), Cmp(MultiCmp) {}
 
     bool isRecurrence() const { return IsRecurrence; }
 
@@ -105,6 +106,8 @@ class RecurrenceDescriptor {
 
     Instruction *getPatternInst() const { return PatternLastInst; }
 
+    Instruction *getMultiCmp() const { return Cmp; }
+
   private:
     // Is this instruction a recurrence candidate.
     bool IsRecurrence;
@@ -115,6 +118,8 @@ class RecurrenceDescriptor {
     RecurKind RecKind;
     // Recurrence does not allow floating-point reassociation.
     Instruction *ExactFPMathInst;
+    // Mult-user compare instruction.
+    Instruction *Cmp;
   };
 
   /// Returns a struct describing if the instruction 'I' can be a recurrence
@@ -270,6 +275,8 @@ class RecurrenceDescriptor {
            cast<IntrinsicInst>(I)->getIntrinsicID() == Intrinsic::fmuladd;
   }
 
+  Instruction *getMultiCmp() const { return MultiCmp; }
+
   /// Reductions may store temporary or final result to an invariant address.
   /// If there is such a store in the loop then, after successfull run of
   /// AddReductionVar method, this field will be assigned the last met store.
@@ -300,6 +307,7 @@ class RecurrenceDescriptor {
   SmallPtrSet<Instruction *, 8> CastInsts;
   // The minimum width used by the recurrence.
   unsigned MinWidthCastToRecurrenceType;
+  Instruction *MultiCmp = nullptr;
 };
 
 /// A struct for saving information about induction variables.
diff --git a/llvm/lib/Analysis/IVDescriptors.cpp b/llvm/lib/Analysis/IVDescriptors.cpp
index 055f121e743411..811c4b75e07052 100644
--- a/llvm/lib/Analysis/IVDescriptors.cpp
+++ b/llvm/lib/Analysis/IVDescriptors.cpp
@@ -256,6 +256,7 @@ bool RecurrenceDescriptor::AddReductionVar(
   SmallPtrSet<Instruction *, 4> CastInsts;
   unsigned MinWidthCastToRecurrenceType;
   Instruction *Start = Phi;
+  Instruction *MultiCMP = nullptr;
   bool IsSigned = false;
 
   SmallPtrSet<Instruction *, 8> VisitedInsts;
@@ -400,6 +401,8 @@ bool RecurrenceDescriptor::AddReductionVar(
     }
 
     bool IsASelect = isa<SelectInst>(Cur);
+    if (IsASelect)
+      MultiCMP = ReduxDesc.getMultiCmp();
 
     // A conditional reduction operation must only have 2 or less uses in
     // VisitedInsts.
@@ -597,7 +600,8 @@ bool RecurrenceDescriptor::AddReductionVar(
   // Save the description of this reduction variable.
   RecurrenceDescriptor RD(RdxStart, ExitInstruction, IntermediateStore, Kind,
                           FMF, ExactFPMathInst, RecurrenceType, IsSigned,
-                          IsOrdered, CastInsts, MinWidthCastToRecurrenceType);
+                          IsOrdered, CastInsts, MinWidthCastToRecurrenceType,
+                          MultiCMP);
   RedDes = RD;
 
   return true;
@@ -635,14 +639,59 @@ RecurrenceDescriptor::isAnyOfPattern(Loop *Loop, PHINode *OrigPhi,
       return InstDesc(Select, Prev.getRecKind());
   }
 
+  SelectInst *SI = dyn_cast<SelectInst>(I);
+  Instruction *Cmp = nullptr;
+
+  if (SI) {
+    bool HasOrigPhiUser = false;
+    bool SelectNonPHIUserInLoop = false;
+    auto Blocks = Loop->getBlocksVector();
+    for (User *U : SI->users()) {
+      Instruction *Inst = dyn_cast<Instruction>(U);
+      if (!Inst)
+        continue;
+      if (Inst == OrigPhi) {
+        HasOrigPhiUser = true;
+      } else {
+        if (std::find(Blocks.begin(), Blocks.end(), Inst->getParent()) !=
+            Blocks.end())
+          SelectNonPHIUserInLoop = true;
+      }
+    }
+    Cmp = dyn_cast<CmpInst>(SI->getOperand(0));
+    if (Cmp && !Cmp->hasOneUse() && HasOrigPhiUser && !SelectNonPHIUserInLoop) {
+      bool IsSafeCMP = true;
+      for (User *U : Cmp->users()) {
+        Instruction *UInst = dyn_cast<Instruction>(U);
+        if (!UInst)
+          continue;
+        if (SelectInst *SI1 = dyn_cast<SelectInst>(U)) {
+          if (!llvm::all_of(SI1->users(), [Blocks](User *USI) {
+                Instruction *Inst1 = dyn_cast<Instruction>(USI);
+                if (!Inst1 || (std::find(Blocks.begin(), Blocks.end(),
+                                         Inst1->getParent()) == Blocks.end() ||
+                               isa<PHINode>(Inst1)))
+                  return true;
+                return false;
+              }))
+            IsSafeCMP = false;
+        }
+        if (IsSafeCMP && !isa<BranchInst>(UInst) && !isa<SelectInst>(UInst) &&
+            std::find(Blocks.begin(), Blocks.end(), UInst->getParent()) !=
+                Blocks.end())
+          IsSafeCMP = false;
+      }
+      if (!IsSafeCMP)
+        Cmp = nullptr;
+    }
+  }
+
   // Only match select with single use cmp condition.
-  if (!match(I, m_Select(m_OneUse(m_Cmp(Pred, m_Value(), m_Value())), m_Value(),
-                         m_Value())))
+  if (!Cmp && !match(I, m_Select(m_OneUse(m_Cmp(Pred, m_Value(), m_Value())),
+                                 m_Value(), m_Value())))
     return InstDesc(false, I);
 
-  SelectInst *SI = cast<SelectInst>(I);
   Value *NonPhi = nullptr;
-
   if (OrigPhi == dyn_cast<PHINode>(SI->getTrueValue()))
     NonPhi = SI->getFalseValue();
   else if (OrigPhi == dyn_cast<PHINode>(SI->getFalseValue()))
@@ -656,8 +705,10 @@ RecurrenceDescriptor::isAnyOfPattern(Loop *Loop, PHINode *OrigPhi,
   if (!Loop->isLoopInvariant(NonPhi))
     return InstDesc(false, I);
 
-  return InstDesc(I, isa<ICmpInst>(I->getOperand(0)) ? RecurKind::IAnyOf
-                                                     : RecurKind::FAnyOf);
+  return InstDesc(I,
+                  isa<ICmpInst>(I->getOperand(0)) ? RecurKind::IAnyOf
+                                                  : RecurKind::FAnyOf,
+                  nullptr, Cmp);
 }
 
 RecurrenceDescriptor::InstDesc
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
index d33743e74cbe31..257be42be0d4f8 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -787,6 +787,7 @@ static bool isTLIScalarize(const TargetLibraryInfo &TLI, const CallInst &CI) {
 
 bool LoopVectorizationLegality::canVectorizeInstrs() {
   BasicBlock *Header = TheLoop->getHeader();
+  DenseMap<Instruction *, unsigned> MultiCmpsRed;
 
   // For each block in the loop.
   for (BasicBlock *BB : TheLoop->blocks()) {
@@ -830,6 +831,13 @@ bool LoopVectorizationLegality::canVectorizeInstrs() {
           Requirements->addExactFPMathInst(RedDes.getExactFPMathInst());
           AllowedExit.insert(RedDes.getLoopExitInstr());
           Reductions[Phi] = RedDes;
+          Instruction *Cmp = RedDes.getMultiCmp();
+          if (Cmp) {
+            if (MultiCmpsRed.contains(Cmp))
+              MultiCmpsRed[Cmp]++;
+            else
+              MultiCmpsRed[Cmp] = 1;
+          }
           continue;
         }
 
@@ -1045,6 +1053,23 @@ bool LoopVectorizationLegality::canVectorizeInstrs() {
     }
   }
 
+  // Make sure that all compare instruction users are recurrent if in loop's BB.
+  if (MultiCmpsRed.size() > 0) {
+    auto Blocks = TheLoop->getBlocksVector();
+    for (auto const &C : MultiCmpsRed) {
+      Instruction *Cmp = C.first;
+      unsigned Counter = 0;
+      for (User *U : Cmp->users()) {
+        SelectInst *Inst = dyn_cast<SelectInst>(U);
+        if (Inst && std::find(Blocks.begin(), Blocks.end(),
+                              Inst->getParent()) != Blocks.end())
+          Counter++;
+      }
+      if (Counter != C.second)
+        return false;
+    }
+  }
+
   // Now we know the widest induction type, check if our found induction
   // is the same size. If it's not, unset it here and InnerLoopVectorizer
   // will create another.
diff --git a/llvm/test/Transforms/LoopVectorize/multicmp.ll b/llvm/test/Transforms/LoopVectorize/multicmp.ll
new file mode 100644
index 00000000000000..30ef11b8b4b309
--- /dev/null
+++ b/llvm/test/Transforms/LoopVectorize/multicmp.ll
@@ -0,0 +1,794 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -passes=loop-vectorize -force-vector-interleave=1 -force-vector-width=4 -S < %s | FileCheck %s --check-prefix=CHECK
+
+define i32 @multi_user_cmp(ptr readonly %a, i32 noundef %n) {
+; CHECK-LABEL: define i32 @multi_user_cmp(
+; CHECK-SAME: ptr readonly [[A:%.*]], i32 noundef [[N:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CMP8:%.*]] = icmp sgt i32 [[N]], 0
+; CHECK-NEXT:    br i1 [[CMP8]], label [[FOR_BODY_PREHEADER:%.*]], label [[FOR_COND_CLEANUP:%.*]]
+; CHECK:       for.body.preheader:
+; CHECK-NEXT:    [[WIDE_TRIP_COUNT:%.*]] = zext nneg i32 [[N]] to i64
+; CHECK-NEXT:    [[MIN_ITERS_CHECK:%.*]] = icmp ult i64 [[WIDE_TRIP_COUNT]], 4
+; CHECK-NEXT:    br i1 [[MIN_ITERS_CHECK]], label [[SCALAR_PH:%.*]], label [[VECTOR_PH:%.*]]
+; CHECK:       vector.ph:
+; CHECK-NEXT:    [[N_MOD_VF:%.*]] = urem i64 [[WIDE_TRIP_COUNT]], 4
+; CHECK-NEXT:    [[N_VEC:%.*]] = sub i64 [[WIDE_TRIP_COUNT]], [[N_MOD_VF]]
+; CHECK-NEXT:    br label [[VECTOR_BODY:%.*]]
+; CHECK:       vector.body:
+; CHECK-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, [[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], [[VECTOR_BODY]] ]
+; CHECK-NEXT:    [[VEC_PHI:%.*]] = phi <4 x i1> [ <i1 true, i1 true, i1 true, i1 true>, [[VECTOR_PH]] ], [ [[TMP5:%.*]], [[VECTOR_BODY]] ]
+; CHECK-NEXT:    [[VEC_PHI1:%.*]] = phi <4 x i1> [ zeroinitializer, [[VECTOR_PH]] ], [ [[TMP4:%.*]], [[VECTOR_BODY]] ]
+; CHECK-NEXT:    [[TMP0:%.*]] = add i64 [[INDEX]], 0
+; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds float, ptr [[A]], i64 [[TMP0]]
+; CHECK-NEXT:    [[TMP2:%.*]] = getelementptr inbounds float, ptr [[TMP1]], i32 0
+; CHECK-NEXT:    [[WIDE_LOAD:%.*]] = load <4 x float>, ptr [[TMP2]], align 4
+; CHECK-NEXT:    [[TMP3:%.*]] = fcmp olt <4 x float> [[WIDE_LOAD]], zeroinitializer
+; CHECK-NEXT:    [[TMP4]] = select <4 x i1> [[TMP3]], <4 x i1> <i1 true, i1 true, i1 true, i1 true>, <4 x i1> [[VEC_PHI1]]
+; CHECK-NEXT:    [[TMP5]] = select <4 x i1> [[TMP3]], <4 x i1> [[VEC_PHI]], <4 x i1> zeroinitializer
+; CHECK-NEXT:    [[INDEX_NEXT]] = add nuw i64 [[INDEX]], 4
+; CHECK-NEXT:    [[TMP6:%.*]] = icmp eq i64 [[INDEX_NEXT]], [[N_VEC]]
+; CHECK-NEXT:    br i1 [[TMP6]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP0:![0-9]+]]
+; CHECK:       middle.block:
+; CHECK-NEXT:    [[RDX_SELECT_CMP:%.*]] = icmp ne <4 x i1> [[TMP5]], <i1 true, i1 true, i1 true, i1 true>
+; CHECK-NEXT:    [[TMP7:%.*]] = call i1 @llvm.vector.reduce.or.v4i1(<4 x i1> [[RDX_SELECT_CMP]])
+; CHECK-NEXT:    [[RDX_SELECT:%.*]] = select i1 [[TMP7]], i1 false, i1 true
+; CHECK-NEXT:    [[RDX_SELECT_CMP2:%.*]] = icmp ne <4 x i1> [[TMP4]], zeroinitializer
+; CHECK-NEXT:    [[TMP8:%.*]] = call i1 @llvm.vector.reduce.or.v4i1(<4 x i1> [[RDX_SELECT_CMP2]])
+; CHECK-NEXT:    [[RDX_SELECT3:%.*]] = select i1 [[TMP8]], i1 true, i1 false
+; CHECK-NEXT:    [[CMP_N:%.*]] = icmp eq i64 [[WIDE_TRIP_COUNT]], [[N_VEC]]
+; CHECK-NEXT:    br i1 [[CMP_N]], label [[FOR_COND_CLEANUP_LOOPEXIT:%.*]], label [[SCALAR_PH]]
+; CHECK:       scalar.ph:
+; CHECK-NEXT:    [[BC_RESUME_VAL:%.*]] = phi i64 [ [[N_VEC]], [[MIDDLE_BLOCK]] ], [ 0, [[FOR_BODY_PREHEADER]] ]
+; CHECK-NEXT:    [[BC_MERGE_RDX:%.*]] = phi i1 [ true, [[FOR_BODY_PREHEADER]] ], [ [[RDX_SELECT]], [[MIDDLE_BLOCK]] ]
+; CHECK-NEXT:    [[BC_MERGE_RDX4:%.*]] = phi i1 [ false, [[FOR_BODY_PREHEADER]] ], [ [[RDX_SELECT3]], [[MIDDLE_BLOCK]] ]
+; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
+; CHECK:       for.cond.cleanup.loopexit:
+; CHECK-NEXT:    [[DOTANY_0_OFF0_LCSSA:%.*]] = phi i1 [ [[DOTANY_0_OFF0:%.*]], [[FOR_BODY]] ], [ [[RDX_SELECT3]], [[MIDDLE_BLOCK]] ]
+; CHECK-NEXT:    [[ALL_0_OFF0__LCSSA:%.*]] = phi i1 [ [[ALL_0_OFF0_:%.*]], [[FOR_BODY]] ], [ [[RDX_SELECT]], [[MIDDLE_BLOCK]] ]
+; CHECK-NEXT:    [[TMP9:%.*]] = select i1 [[DOTANY_0_OFF0_LCSSA]], i32 2, i32 3
+; CHECK-NEXT:    [[TMP10:%.*]] = select i1 [[ALL_0_OFF0__LCSSA]], i32 1, i32 [[TMP9]]
+; CHECK-NEXT:    br label [[FOR_COND_CLEANUP]]
+; CHECK:       for.cond.cleanup:
+; CHECK-NEXT:    [[ALL_0_OFF0_LCSSA:%.*]] = phi i32 [ 1, [[ENTRY:%.*]] ], [ [[TMP10]], [[FOR_COND_CLEANUP_LOOPEXIT]] ]
+; CHECK-NEXT:    ret i32 [[ALL_0_OFF0_LCSSA]]
+; CHECK:       for.body:
+; CHECK-NEXT:    [[INDVARS_IV:%.*]] = phi i64 [ [[BC_RESUME_VAL]], [[SCALAR_PH]] ], [ [[INDVARS_IV_NEXT:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[ALL_0_OFF010:%.*]] = phi i1 [ [[BC_MERGE_RDX]], [[SCALAR_PH]] ], [ [[ALL_0_OFF0_]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[ANY_0_OFF09:%.*]] = phi i1 [ [[BC_MERGE_RDX4]], [[SCALAR_PH]] ], [ [[DOTANY_0_OFF0]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds float, ptr [[A]], i64 [[INDVARS_IV]]
+; CHECK-NEXT:    [[TMP11:%.*]] = load float, ptr [[ARRAYIDX]], align 4
+; CHECK-NEXT:    [[CMP1:%.*]] = fcmp olt float [[TMP11]], 0.000000e+00
+; CHECK-NEXT:    [[DOTANY_0_OFF0]] = select i1 [[CMP1]], i1 true, i1 [[ANY_0_OFF09]]
+; CHECK-NEXT:    [[ALL_0_OFF0_]] = select i1 [[CMP1]], i1 [[ALL_0_OFF010]], i1 false
+; CHECK-NEXT:    [[INDVARS_IV_NEXT]] = add nuw nsw i64 [[INDVARS_IV]], 1
+; CHECK-NEXT:    [[EXITCOND_NOT:%.*]] = icmp eq i64 [[INDVARS_IV_NEXT]], [[WIDE_TRIP_COUNT]]
+; CHECK-NEXT:    br i1 [[EXITCOND_NOT]], label [[FOR_COND_CLEANUP_LOOPEXIT]], label [[FOR_BODY]], !llvm.loop [[LOOP3:![0-9]+]]
+;
+entry:
+  %cmp8 = icmp sgt i32 %n, 0
+  br i1 %cmp8, label %for.body.preheader, label %for.cond.cleanup
+
+for.body.preheader:
+  %wide.trip.count = zext nneg i32 %n to i64
+  br label %for.body
+
+for.cond.cleanup.loopexit:
+  %0 = select i1 %.any.0.off0, i32 2, i32 3
+  %1 = select i1 %all.0.off0., i32 1, i32 %0
+  br label %for.cond.cleanup
+
+for.cond.cleanup:
+  %all.0.off0.lcssa = phi i32 [ 1, %entry ], [ %1, %for.cond.cleanup.loopexit ]
+  ret i32 %all.0.off0.lcssa
+
+for.body:
+  %indvars.iv = phi i64 [ 0, %for.body.preheader ], [ %indvars.iv.next, %for.body ]
+  %all.0.off010 = phi i1 [ true, %for.body.preheader ], [ %all.0.off0., %for.body ]
+  %any.0.off09 = phi i1 [ false, %for.body.preheader ], [ %.any.0.off0, %for.body ]
+  %arrayidx = getelementptr inbounds float, ptr %a, i64 %indvars.iv
+  %2 = load float, ptr %arrayidx, align 4
+  %cmp1 = fcmp olt float %2, 0.000000e+00
+  %.any.0.off0 = select i1 %cmp1, i1 true, i1 %any.0.off09
+  %all.0.off0. = select i1 %cmp1, i1 %all.0.off010, i1 false
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond.not = icmp eq i64 %indvars.iv.next, %wide.trip.count
+  br i1 %exitcond.not, label %for.cond.cleanup.loopexit, label %for.body
+}
+
+define i32 @multi_user_cmp_int(ptr readonly %a, i32 noundef %n) {
+; CHECK-LABEL: define i32 @multi_user_cmp_int(
+; CHECK-SAME: ptr readonly [[A:%.*]], i32 noundef [[N:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CMP8:%.*]] = icmp sgt i32 [[N]], 0
+; CHECK-NEXT:    br i1 [[CMP8]], label [[FOR_BODY_PREHEADER:%.*]], label [[FOR_COND_CLEANUP:%.*]]
+; CHECK:       for.body.preheader:
+; CHECK-NEXT:    [[WIDE_TRIP_COUNT:%.*]] = zext nneg i32 [[N]] to i64
+; CHECK-NEXT:    [[MIN_ITERS_CHECK:%.*]] = icmp ult i64 [[WIDE_TRIP_COUNT]], 4
+; CHECK-NEXT:    br i1 [[MIN_ITERS_CHECK]], label [[SCALAR_PH:%.*]], label [[VECTOR_PH:%.*]]
+; CHECK:       vector.ph:
+; CHECK-NEXT:    [[N_MOD_VF:%.*]] = urem i64 [[WIDE_TRIP_COUNT]], 4
+; CHECK-NEXT:    [[N_VEC:%.*]] = sub i64 [[WIDE_TRIP_COUNT]], [[N_MOD_VF]]
+; CHECK-NEXT:    br label [[VECTOR_BODY:%.*]]
+; CHECK:       vector.body:
+; CHECK-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, [[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], [[VECTOR_BODY]] ]
+; CHECK-NEXT:    [[VEC_PHI:%.*]] = phi <4 x i1> [ <i1 true, i1 true, i1 true, i1 true>, [[VECTOR_PH]] ], [ [[TMP5:%.*]], [[VECTOR_BODY]] ]
+; CHECK-NEXT:    [[VEC_PHI1:%.*]] = phi <4 x i1> [ zeroinitializer, [[VECTOR_PH]] ], [ [[TMP4:%.*]], [[VECTOR_BODY]] ]
+; CHECK-NEXT:    [[TMP0:%.*]] = add i64 [[INDEX]], 0
+; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds i32, ptr [[A]], i64 [[TMP0]]
+; CHECK-NEXT:    [[TMP2:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i32 0
+; CHECK-NEXT:    [[WIDE_LOAD:%.*]] = load <4 x i32>, ptr [[TMP2]], align 4
+; CHECK-NEXT:    [[TMP3:%.*]] = icmp slt <4 x i32> [[WIDE_LOAD]], zeroinitializer
+; CHECK-NEXT:    [[TMP4]] = select <4 x i1> [[TMP3]], <4 x i1> <i1 true, i1 true, i1 true, i1 true>, <4 x i1> [[VEC_PHI1]]
+; CHECK-NEXT:    [[TMP5]] = select <4 x i1> [[TMP3]], <4 x i1> [[VEC_PHI]], <4 x i1> zeroinitializer
+; CHECK-NEXT:    [[INDEX_NEXT]] = add nuw i64 [[INDEX]], 4
+; CHECK-NEXT:    [[TMP6:%.*]] = icmp eq i64 [[INDEX_NEXT]], [[N_VEC]]
+; CHECK-NEXT:    br i1 [[TMP6]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP4:![0-9]+]]
+; CHECK:       middle.block:
+; CHECK-NEXT:    [[RDX_SELECT_CMP:%.*]] = icmp ne <4 x i1> [[TMP5]], <i1 true, i1 true, i1 true, i1 true>
+; CHECK-NEXT:    [[TMP7:%.*]] = call i1 @llvm.vector.reduce.or.v4i1(<4 x i1> [[RDX_SELECT_CMP]])
+; CHECK-NEXT:    [[RDX_SELECT:%.*]] = select i1 [[TMP7]], i1 false, i1 true
+; CHECK-NEXT:    [[RDX_SELECT_CMP2:%.*]] = icmp ne <4 x i1> [[TMP4]], zeroinitializer
+; CHECK-NEXT:    [[TMP8:%.*]] = call i1 @llvm.vector.reduce.or.v4i1(<4 x i1> [[RDX_SELECT_CMP2]])
+; CHECK-NEXT:    [[RDX_SELECT3:%.*]] = select i1 [[TMP8]], i1 true, i1 false
+; CHECK-NEXT:    [[CMP_N:%.*]] = icmp eq i64 [[WIDE_TRIP_COUNT]], [[N_VEC]]
+; CHECK-NEXT:    br i1 [[CMP_N]], label [[FOR_COND_CLEANUP_LOOPEXIT:%.*]], label [[SCALAR_PH]]
+; CHECK:       scalar.ph:
+; CHECK-NEXT:    [[BC_RESUME_VAL:%.*]] = phi i64 [ [[N_VEC]], [[MIDDLE_BLOCK]] ], [ 0, [[FOR_BODY_PREHEADER]] ]
+; CHECK-NEXT:    [[BC_MERGE_RDX:%.*]] = phi i1 [ true, [[FOR_BODY_PREHEADER]] ], [ [[RDX_SELECT]], [[MIDDLE_BLOCK]] ]
+; CHECK-NEXT:    [[BC_MERGE_RDX4:%.*]] = phi i1 [ false, [[FOR_BODY_PREHEADER]] ], [ [[RDX_SELECT3]], [[MIDDLE_BLOCK]] ]
+; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
+; CHECK:       for.cond.cleanup.loopexit:
+; CHECK-NEXT:    [[DOTANY_0_OFF0_LCSSA:%.*]] = phi i1 [ [[DOTANY_0_OFF0:%.*]], [[FOR_BODY]] ], [ [[RDX_SELECT3]], [[MIDDLE_BLOCK]] ]
+; CHECK-NEXT:    [[ALL_0_OFF0__LCSSA:%.*]] = phi i1 [ [[ALL_0_OFF0_:%.*]], [[FOR_BODY]] ], [ [[RDX_SELECT]], [[MIDDLE_BLOCK]] ]
+; CHECK-NEXT:    [[TMP9:%.*]] = select i1 [[DOTANY_0_OFF0_LCSSA]], i32 2, i32 3
+; CHECK-NEXT:    [[TMP10:%.*]] = select i1 [[ALL_0_OFF0__LCSSA]], i32 1, i32 [[TMP9]]
+; CHECK-NEXT:    br label [[FOR_COND_CLEANUP]]
+; CHECK:       for.cond.cleanup:
+; CHECK-NEXT:    [[ALL_0_OFF0_LCSSA:%.*]] = phi i32 [ 1, [[ENTRY:%.*]] ], [ [[TMP10]], [[FOR_COND_CLEANUP_LOOPEXIT]] ]
+; CHECK-NEXT:    ret i32 [[ALL_0_OFF0_LCSSA]]
+; CHECK:       for.body:
+; CHECK-NEXT:    [[INDVARS_IV:%.*]] = phi i64 [ [[BC_RESUME_VAL]], [[SCALAR_PH]] ], [ [[INDVARS_IV_NEXT:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[ALL_0_OFF010:%.*]] = phi i1 [ [[BC_MERGE_RDX]], [[SCALAR_PH]] ], [ [[ALL_0_OFF0_]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[ANY_0_OFF09:%.*]] =...
[truncated]

SamTebbs33 · 2024-04-18T13:35:44Z

llvm/lib/Analysis/IVDescriptors.cpp

@@ -635,14 +639,59 @@ RecurrenceDescriptor::isAnyOfPattern(Loop *Loop, PHINode *OrigPhi,
      return InstDesc(Select, Prev.getRecKind());
  }

+  SelectInst *SI = dyn_cast<SelectInst>(I);


I think this change could do with some comments.

SamTebbs33 · 2024-04-18T13:36:32Z

llvm/test/Transforms/LoopVectorize/multicmp.ll

@@ -0,0 +1,794 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4


Could you pre-commit this test so we can see the difference your patch makes?

Thanks, Sam. I just added test description or C representations. Is it ok?

The comments are useful but I meant that you could commit the tests to main and rebase this PR on top of that so that we can see the difference this PR makes. Florian made a similar suggestion but with a new PR instead of committing straight to main.

Or just take the simplest option of having two commits in this patch. Commit 1 - add tests, Commit 2 - add the code change which shows the test changes.

That's a better idea. I'm used to the fabricator days when you couldn't do that!

That's a better idea. I'm used to the fabricator days when you couldn't do that!

Splitting the test changes completely off the functional changes has a few advantages compared to adding them as separate commit to this PR:

Having a separate PR avoids cluttering the functional review with a large number of comments regarding the tests

Github's PR interface shows the changes from all commits in the Files changed view

As all commits in a PR get squashed together, the commit landing won't just show the diff on the tests caused by the patch, making it more difficult for people looking at the commit later (e.g. when doing post-commit review).

The only problem is that you'll probably still end up with lots of comments or requests to change the tests once reviewers see the actual code changes and, for example, realise that a test case is missing for a particular change. Then you still end up changing the tests in the second patch anyway and at that point it's too late to change the first test-only patch because it has already landed.

I realise the official solution to this is to create branches in the main llvm repo in order to do stacked pull requests. That technically is a solution that probably works well for some cases, but given this is a very frequent thing to do it's also more costly and it seems that people aren't deleting their branches afterwards. git branch -r -l | grep users | wc -l returns a count of 638.

Not trying to say which way is right or wrong, or what works best for the individual developer, but just pointing out there are pros and cons to each approach.

I agree with David's concern. Regarding your points fhahn, one can see the differences between commits in the "files changed" view and there is a link to the PR in the commit body so people can see the test diff during post-commit review.

dtemirbulatov · 2024-04-23T12:23:12Z

Ping.

david-arm

Thanks for this @dtemirbulatov - it looks like a useful addition! I've not done a full review of the code changes, but I've left some comments so far.

david-arm · 2024-04-23T12:32:24Z

llvm/test/Transforms/LoopVectorize/multicmp.ll

+; CHECK-NEXT:    br i1 [[EXITCOND_NOT]], label [[FOR_COND_CLEANUP_LOOPEXIT]], label [[FOR_BODY]], !llvm.loop [[LOOP3:![0-9]+]]
+;
+entry:
+  %cmp8 = icmp sgt i32 %n, 0


In all these tests can you:

Remove the greater-than comparison with 0 so that the entry block unconditionally jumps to the preheader? This will remove quite a lot of code and CHECK lines. The comparison isn't essential for testing your change I think.

Rewrite the blocks in a more logical order, which for this function at least means putting the cleanup blocks at the end of the function.

Maybe instead of for.cond.cleanup you can just call it 'exit' just to make it a bit more readable?

david-arm · 2024-04-23T12:36:23Z

llvm/test/Transforms/LoopVectorize/multicmp.ll

@@ -0,0 +1,883 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -passes=loop-vectorize -force-vector-interleave=1 -force-vector-width=4 -S < %s | FileCheck %s --check-prefix=CHECK


I think it probably makes sense to add these tests to the existing select-cmp.ll test file. However, if you prefer to leave them in this new file you should add some more RUN lines to cover more cases. For example, these two might test different scenarios ..

; RUN: opt -passes=loop-vectorize -force-vector-interleave=2 -force-vector-width=4 -S < %s | FileCheck %s --check-prefix=CHECK-VF4-IC2 ; RUN: opt -passes=loop-vectorize -force-vector-interleave=2 -force-vector-width=1 -S < %s | FileCheck %s --check-prefix=CHECK-VF1-IC2

david-arm · 2024-04-23T12:56:08Z

llvm/include/llvm/Analysis/IVDescriptors.h

@@ -270,6 +275,8 @@ class RecurrenceDescriptor {
           cast<IntrinsicInst>(I)->getIntrinsicID() == Intrinsic::fmuladd;
  }

+  Instruction *getMultiCmp() const { return MultiCmp; }


I must admit I haven't looked through all the code yet, but I'm a bit confused about why we need this? It looks like you're jumping on the back of the IAnyOf and FAnyOf RecurKind types, which by definition include a cmp instruction. The only difference here is that you are permitting multiple uses of that same cmp instruction.

Can you not simply obtain the cmp instruction from the IV descriptor itself? It just feels a bit strange to add Instruction *MultiCmp variable here because it doesn't make sense for any other RecurKinds. I guess what you are trying to do is add a different flavour of IAnyOf, FAnyOf without having to create a new RecurKind.

david-arm · 2024-04-23T13:56:07Z

llvm/lib/Analysis/IVDescriptors.cpp

+  return InstDesc(I,
+                  isa<ICmpInst>(I->getOperand(0)) ? RecurKind::IAnyOf
+                                                  : RecurKind::FAnyOf,
+                  nullptr, Cmp);


With reference to my question above about why we need this extra Cmp parameter, I think if the RecurKind is IAnyOf or FAnyOf you should be able to get access to it by calling getPatternInst.

fhahn · 2024-04-23T14:15:23Z

Could you split off the new tests in a separate PR so we can only see the test changes here? Also would probably be good to have at least some target specific tests to check cost modeling

dtemirbulatov · 2024-05-07T13:36:39Z

Ping.

sdesmalen-arm

Because of missing negative tests I am not really sure which cases you're trying to prevent from being vectorised. The code itself is a little too incomprehensible to make that purpose clear to me either.

sdesmalen-arm · 2024-05-13T08:50:58Z

llvm/lib/Analysis/IVDescriptors.cpp

-  if (!match(I, m_Select(m_OneUse(m_Cmp(Pred, m_Value(), m_Value())), m_Value(),
-                         m_Value())))


When I ignore the code changes in this PR and simply remove the m_OneUse in this line of code, then all the tests added in this PR still pass. The only test failure I see is in the existing test llvm/test/Transforms/LoopVectorize/select-cmp.ll, where I think the vectorized result is not incorrect. I'm not sure if that would be different if the resulting PHI node (that uses the zero-extended result of the icmp) is used, which it currently isn't.

In either case, the lack of test failures suggests to me that the complicated logic added in this patch isn't really being tested by any of the tests?

I think there are two negative tests in llvm/test/Transforms/LoopVectorize/multicmp.ll : multi_user_cmp_no_vectorise() and multi_user_cmp_no_vectorise(). I will add more.

sdesmalen-arm · 2024-05-13T09:34:15Z

llvm/lib/Analysis/IVDescriptors.cpp

+          SelectNonPHIUserInLoop = true;
+      }
+    }
+    Cmp = dyn_cast<CmpInst>(SI->getOperand(0));


If the compare used in the select has another use which is not part of any reduction, does that mean it's no longer an "any_of" pattern? I would expect isAnyOfPattern to still return true, but for any further checks to be done somewhere else.

sdesmalen-arm · 2024-05-13T09:36:39Z

llvm/lib/Analysis/IVDescriptors.cpp

+        if (!UInst)
+          continue;
+        if (SelectInst *SI1 = dyn_cast<SelectInst>(U)) {
+          if (!llvm::all_of(SI1->users(), [Loop](User *USI) {


Having a loop that goes through all users of all users of the Compare seems rather expensive.

sdesmalen-arm · 2024-05-13T09:41:43Z

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

@@ -1045,6 +1060,23 @@ bool LoopVectorizationLegality::canVectorizeInstrs() {
    }
  }

+  // Make sure that all compare instruction users are recurrent if in loop's BB.
+  if (MultiCmpsRed.size() > 0) {


Why do we need similar loops/checks in both LoopVectorizationLegality.cpp and IVDecriptors.cpp?

There might be other kind of reduction and compare instruction with multipe users, I have seen such a case. I think I will add a test for this case.

david-arm

I think the patch looks good and the tests are comprehensive! I just had a couple of minor comments, in particular the patch needs rebasing. Thanks!

david-arm · 2024-06-13T13:49:49Z

llvm/test/Transforms/LoopVectorize/AArch64/select-multi-cmp.ll

Perhaps you can move this test to Transforms/LoopVectorize/AArch64/select-costs.ll and delete this file?

david-arm · 2024-06-13T13:50:17Z

llvm/test/Transforms/LoopVectorize/multicmp.ll

I tried running these locally and the tests failed. I think this patch just needs a rebase.

david-arm

Thanks for the rebase and making the changes! I've got a few more comments, but once they're done I'm happy. :)

david-arm · 2024-06-14T08:28:18Z

llvm/test/Transforms/LoopVectorize/AArch64/select-costs.ll

+; CHECK-NEXT: LV: Found an estimated cost of 1 for VF 16 For instruction:   %.any.0.off0 = select i1 %cmp1, i1 true, i1 %any.0.off09
+; CHECK-NEXT: LV: Found an estimated cost of 1 for VF 16 For instruction:   %all.0.off0. = select i1 %cmp1, i1 %all.0.off010, i1 false


Thanks for moving the test! I think if what you really want to test is the cost of the select instructions, perhaps it's better to write the costs in the same way as @selects_1, i.e.

; CHECK-NEXT: LV: Found an estimated cost of 1 for VF 8 For instruction: %.any.0.off0 = select i1 %cmp1, i1 true, i1 %any.0.off09 ; CHECK-NEXT: LV: Found an estimated cost of 1 for VF 8 For instruction: %all.0.off0. = select i1 %cmp1, i1 %all.0.off010, i1 false ; CHECK-NEXT: LV: Found an estimated cost of 1 for VF 16 For instruction: %.any.0.off0 = select i1 %cmp1, i1 true, i1 %any.0.off09 ; CHECK-NEXT: LV: Found an estimated cost of 1 for VF 16 For instruction: %all.0.off0. = select i1 %cmp1, i1 %all.0.off010, i1 false

david-arm · 2024-06-14T08:31:44Z

llvm/test/Transforms/LoopVectorize/AArch64/select-costs.ll

+  ret i32 %1
+}
+
+; CHECK-LABEL: define void @selects_1(


I see why you had to move these CHECK lines. Perhaps you don't really need them at all? Since you're collecting the debug output from the vectoriser anyway, you can simply CHECK for the chosen VF, i.e.

LV: Found an estimated cost of 1 for VF 16 For instruction: %.any.0.off0 = select i1 %cmp1, i1 true, i1 %any.0.off09 LV: Found an estimated cost of 1 for VF 16 For instruction: %all.0.off0. = select i1 %cmp1, i1 %all.0.off010, i1 false ... LV: Selecting VF: 16

This way you don't need to check any lines of IR and might simplify the test.

I think this comment still stands. You can simplify the test by removing these check lines and simply adding a CHECK line for the selected VF to the existing CHECK lines above. If you do this then you can also simplify the test RUN line to be:

; RUN: opt < %s -passes=loop-vectorize -debug-only=loop-vectorize -disable-output -S 2>&1 | FileCheck %s

david-arm · 2024-06-14T08:35:14Z

llvm/test/Transforms/LoopVectorize/select-cmp.ll

@@ -247,6 +247,246 @@ exit:                                     ; preds = %for.body
 }


+define i32 @select_const_i32_from_icmp_mul_use(ptr nocapture readonly %v1, ptr %v2, i64 %n) {


It looks like you're already testing this in the new multicmp.ll test file. Perhaps you can just delete the test?

david-arm · 2024-06-14T08:36:36Z

llvm/test/Transforms/LoopVectorize/multicmp.ll

It might be worth renaming this file to select-cmp-multiuse.ll? That way it's a bit more consistent with the select-cmp.ll naming and shows the difference between them?

fhahn · 2024-06-14T08:50:02Z

@dtemirbulatov I think there was an earlier comment about an issue you found while testing this, but it seems to have disappeared? Just double checking if the latest version includes a fix + test for it?

dtemirbulatov · 2024-06-17T12:26:27Z

@dtemirbulatov I think there was an earlier comment about an issue you found while testing this, but it seems to have disappeared? Just double checking if the latest version includes a fix + test for it?

@fhahn, it looks like all tests are in place from the first version and the comment was a mistake on my side, so I deleted the comment.

david-arm

Thanks for the updates @dtemirbulatov! I just have a few minor comments left on the cost test file.

david-arm · 2024-06-25T10:52:52Z

llvm/test/Transforms/LoopVectorize/AArch64/select-costs.ll

@@ -47,3 +43,38 @@ for.cond.cleanup.loopexit:                        ; preds = %for.body
 for.cond.cleanup:                                 ; preds = %for.cond.cleanup.loopexit, %entry
  ret void
 }
+
+define i32 @multi_user_cmp(ptr readonly %a, i32 noundef %n) {
+; CHECK: LV: Found an estimated cost of 4 for VF 16 For instruction:   %cmp1 = fcmp olt float %load1, 0.000000e+00


Sorry @dtemirbulatov I just have one more comment. I guess it may be my fault you're hitting this issue, but at the moment these CHECK lines are fragile and it's not obvious which function we are actually testing. That's because these CHECK lines could in theory match the CHECK lines in @selects_1. In each function the CHECK line should start with:

define void @selects_1(ptr nocapture %dst, i32 %A, i32 %B, i32 %C, i32 %N) { ; CHECK-LABEL: LV: Checking a loop in 'selects_1' ... existing CHECKs ... ... } define i32 @multi_user_cmp(ptr readonly %a, i32 noundef %n) { ; CHECK-LABEL: LV: Checking a loop in 'multi_user_cmp' ... existing CHECKs ... ... }

david-arm · 2024-06-25T10:54:39Z

llvm/test/Transforms/LoopVectorize/AArch64/select-costs.ll

+  ret i32 %1
+}
+
+; CHECK-LABEL: define void @selects_1(


I think this comment still stands. You can simplify the test by removing these check lines and simply adding a CHECK line for the selected VF to the existing CHECK lines above. If you do this then you can also simplify the test RUN line to be:

; RUN: opt < %s -passes=loop-vectorize -debug-only=loop-vectorize -disable-output -S 2>&1 | FileCheck %s

david-arm

LGTM! Thanks for all the changes @dtemirbulatov.

fhahn

LGTM, thanks!

There's one place where we modify the matched compare for any_of reductions (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp#L9034) which I think relies on the select being the single user. I don't think cases where this triggers are currently supported by this change, but it would probably be good to add an assert there that the modified compare has a single user.

fhahn · 2024-07-02T08:12:29Z

llvm/test/Transforms/LoopVectorize/AArch64/select-costs.ll

+
+for.body:
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+  %all.0.off010 = phi i1 [ true, %entry ], [ %all.0.off0., %for.body ]


nit: names could be simplified by dropping .0, 010, %all.0.off0. could become %all.off.next

fhahn · 2024-07-02T08:12:44Z

llvm/test/Transforms/LoopVectorize/AArch64/select-costs.ll

+; CHECK: LV: Found an estimated cost of 1 for VF 16 For instruction:   %all.0.off0. = select i1 %cmp1, i1 %all.0.off010, i1 false
+; CHECK: LV: Selecting VF: 16.
+entry:
+  %wide.trip.count = zext nneg i32 %n to i64


nit: simpler to pass %n as i64

dtemirbulatov · 2024-07-12T14:49:31Z

@fhahn I don't think we need this extra assert at (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp#L9034).

…ables This patch allows to consider compare instructions in the loop with multiple use inside the loop and outside, if we can prove that compare instruction user is a recurrent reduction or used in branching or outside the loop then it is safe to consider to vectorise. This change allows to vectorise this loop: int foo(float* a, int n) { _Bool any = 0; _Bool all = 1; for (int i = 0; i < n; i++) { if (a[i] < 0.0f) { any = 1; } else { all = 0; } } return all ? 1 : any ? 2 : 3; }

Replaced std::find(Blocks.begin(), Blocks.end(),... to Loop->contains(Inst->getParent()), added comments.

fhahn · 2024-07-13T08:12:47Z

@fhahn I don't think we need this extra assert at (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp#L9034).

Yep I don't think it is needed in the VPlan-native path/VPInstructionsToVPRecipes

#89226) This change allows to consider compare instructions in the loop with multiple use inside the loop and outside. This change allows to vectorise this loop: int foo(float* a, int n) { _Bool any = 0; _Bool all = 1; for (int i = 0; i < n; i++) { if (a[i] < 0.0f) { any = 1; } else { all = 0; } } return all ? 1 : any ? 2 : 3; }

dtemirbulatov requested review from SamTebbs33, huntergr-arm, sdesmalen-arm, kmclaughlin-arm and david-arm April 18, 2024 12:31

llvmbot added vectorizers llvm:analysis llvm:transforms labels Apr 18, 2024

SamTebbs33 requested changes Apr 18, 2024

View reviewed changes

david-arm reviewed Apr 23, 2024

View reviewed changes

dtemirbulatov force-pushed the lv-multi-cmp branch from 6ce35f2 to db3c9af Compare May 3, 2024 13:39

sdesmalen-arm requested changes May 13, 2024

View reviewed changes

dtemirbulatov requested a review from davemgreen May 21, 2024 09:08

dtemirbulatov changed the title ~~[LoopVectorize] LLVM fails to vectorise loops with multi-bool varables~~ [WIP][LoopVectorize] LLVM fails to vectorise loops with multi-bool varables Jun 10, 2024

dtemirbulatov changed the title ~~[WIP][LoopVectorize] LLVM fails to vectorise loops with multi-bool varables~~ [LoopVectorize] LLVM fails to vectorise loops with multi-bool varables Jun 12, 2024

david-arm reviewed Jun 13, 2024

View reviewed changes

dtemirbulatov force-pushed the lv-multi-cmp branch from 9a626e6 to 26fe800 Compare June 14, 2024 08:22

david-arm reviewed Jun 14, 2024

View reviewed changes

dtemirbulatov force-pushed the lv-multi-cmp branch from 26fe800 to 7e650b6 Compare June 17, 2024 12:24

david-arm reviewed Jun 25, 2024

View reviewed changes

david-arm approved these changes Jul 1, 2024

View reviewed changes

fhahn approved these changes Jul 2, 2024

View reviewed changes

dtemirbulatov force-pushed the lv-multi-cmp branch from 356ab98 to cb9b126 Compare July 12, 2024 14:44

dtemirbulatov added 7 commits July 12, 2024 22:50

Original verison of llvm/test/Transforms/LoopVectorize/multicmp.ll

de0cd17

Resolved remarks.

9752839

Resolved remarks.

f1dfe17

Changed multi_user_cmp_max() function to use llvm.smax.i32 intrinsic,

e77247f

Replaced std::find(Blocks.begin(), Blocks.end(),... to Loop->contains(Inst->getParent()), added comments.

Fix a compile time failure, while testing proposed change.

e50cd3f

Update tests.

4135912

dtemirbulatov force-pushed the lv-multi-cmp branch from cb9b126 to f5a628f Compare July 13, 2024 00:09

dtemirbulatov force-pushed the lv-multi-cmp branch from f5a628f to 1da6a96 Compare July 14, 2024 20:50

Further test updates.

327f94d

dtemirbulatov force-pushed the lv-multi-cmp branch from 1da6a96 to 327f94d Compare July 15, 2024 11:49

dtemirbulatov merged commit 31d4c97 into llvm:main Jul 15, 2024
7 checks passed

dtemirbulatov deleted the lv-multi-cmp branch July 17, 2024 09:27

		@@ -0,0 +1,794 @@
		; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4

		@@ -0,0 +1,883 @@
		; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
		; RUN: opt -passes=loop-vectorize -force-vector-interleave=1 -force-vector-width=4 -S < %s \| FileCheck %s --check-prefix=CHECK

		if (!match(I, m_Select(m_OneUse(m_Cmp(Pred, m_Value(), m_Value())), m_Value(),
		m_Value())))

		; CHECK-NEXT: LV: Found an estimated cost of 1 for VF 16 For instruction: %.any.0.off0 = select i1 %cmp1, i1 true, i1 %any.0.off09
		; CHECK-NEXT: LV: Found an estimated cost of 1 for VF 16 For instruction: %all.0.off0. = select i1 %cmp1, i1 %all.0.off010, i1 false

		@@ -247,6 +247,246 @@ exit: ; preds = %for.body
		}


		define i32 @select_const_i32_from_icmp_mul_use(ptr nocapture readonly %v1, ptr %v2, i64 %n) {

[LoopVectorize] LLVM fails to vectorise loops with multi-bool varables #89226

[LoopVectorize] LLVM fails to vectorise loops with multi-bool varables #89226

Conversation

dtemirbulatov commented Apr 18, 2024 • edited Loading

llvmbot commented Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtemirbulatov commented Apr 23, 2024

david-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhahn commented Apr 23, 2024

dtemirbulatov commented May 7, 2024

sdesmalen-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtemirbulatov May 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhahn commented Jun 14, 2024

dtemirbulatov commented Jun 17, 2024 • edited Loading

david-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-arm left a comment

Choose a reason for hiding this comment

fhahn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtemirbulatov commented Jul 12, 2024

fhahn commented Jul 13, 2024

dtemirbulatov commented Apr 18, 2024 •

edited

Loading

llvmbot commented Apr 18, 2024 •

edited

Loading

dtemirbulatov May 13, 2024 •

edited

Loading

dtemirbulatov commented Jun 17, 2024 •

edited

Loading