[llvm] IVDesc: unify RecurKinds IAnyOf and FAnyOf (PR #118393)

Ramkumar Ramachandra via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 3 10:39:03 PST 2024


https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/118393

>From f730979509612522f115f671caea02f72c15e9b2 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 2 Dec 2024 19:37:06 +0000
Subject: [PATCH 1/3] IVDescriptors: cut wasteful FAnyOf checking (NFC)

Checking RecurKind::FAnyOf in isRecurrenceDescriptor() is wasted work
when it already checks RecurKind::IAnyOf. Affect a minor adjustment to
the code to facilitate skipping the RecurKind::FAnyOf check, and strip
the check. The patch has the side-effect of rewriting some flaky code,
which would match an ICmp having the reduction phi as an operand with
IAnyOf, and due to NumCmpSelectPatternInst != 1 (the select redux is
also matched), it would have to rely on failing to match an FCmp with
FAnyOf, setting NumCmpSelectPatternInst = 1, and successfully
vectorizing an IAnyOf pattern with the incorrect debug output. There is
a test for this already in select-cmp.ll:
select_i32_from_icmp_same_inputs.
---
 llvm/lib/Analysis/IVDescriptors.cpp | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/llvm/lib/Analysis/IVDescriptors.cpp b/llvm/lib/Analysis/IVDescriptors.cpp
index e1eb219cf977e1..ce045389ff815d 100644
--- a/llvm/lib/Analysis/IVDescriptors.cpp
+++ b/llvm/lib/Analysis/IVDescriptors.cpp
@@ -415,12 +415,9 @@ bool RecurrenceDescriptor::AddReductionVar(
     if (IsAPhi && Cur != Phi && !areAllUsesIn(Cur, VisitedInsts))
       return false;
 
-    if ((isIntMinMaxRecurrenceKind(Kind) || Kind == RecurKind::IAnyOf) &&
-        (isa<ICmpInst>(Cur) || isa<SelectInst>(Cur)))
-      ++NumCmpSelectPatternInst;
-    if ((isFPMinMaxRecurrenceKind(Kind) || Kind == RecurKind::FAnyOf) &&
-        (isa<FCmpInst>(Cur) || isa<SelectInst>(Cur)))
-      ++NumCmpSelectPatternInst;
+    if (isIntMinMaxRecurrenceKind(Kind) || isAnyOfRecurrenceKind(Kind))
+      if (match(Cur, m_Select(m_Cmp(), m_Value(), m_Value())))
+        ++NumCmpSelectPatternInst;
 
     // Check  whether we found a reduction operator.
     FoundReduxOp |= !IsAPhi && Cur != Start;
@@ -500,7 +497,7 @@ bool RecurrenceDescriptor::AddReductionVar(
   // This means we have seen one but not the other instruction of the
   // pattern or more than just a select and cmp. Zero implies that we saw a
   // llvm.min/max intrinsic, which is always OK.
-  if (isMinMaxRecurrenceKind(Kind) && NumCmpSelectPatternInst != 2 &&
+  if (isMinMaxRecurrenceKind(Kind) && NumCmpSelectPatternInst != 1 &&
       NumCmpSelectPatternInst != 0)
     return false;
 
@@ -889,8 +886,8 @@ bool RecurrenceDescriptor::isReductionPHI(PHINode *Phi, Loop *TheLoop,
   }
   if (AddReductionVar(Phi, RecurKind::IAnyOf, TheLoop, FMF, RedDes, DB, AC, DT,
                       SE)) {
-    LLVM_DEBUG(dbgs() << "Found an integer conditional select reduction PHI."
-                      << *Phi << "\n");
+    LLVM_DEBUG(dbgs() << "Found an conditional select reduction PHI." << *Phi
+                      << "\n");
     return true;
   }
   if (AddReductionVar(Phi, RecurKind::FMul, TheLoop, FMF, RedDes, DB, AC, DT,
@@ -913,12 +910,6 @@ bool RecurrenceDescriptor::isReductionPHI(PHINode *Phi, Loop *TheLoop,
     LLVM_DEBUG(dbgs() << "Found a float MIN reduction PHI." << *Phi << "\n");
     return true;
   }
-  if (AddReductionVar(Phi, RecurKind::FAnyOf, TheLoop, FMF, RedDes, DB, AC, DT,
-                      SE)) {
-    LLVM_DEBUG(dbgs() << "Found a float conditional select reduction PHI."
-                      << " PHI." << *Phi << "\n");
-    return true;
-  }
   if (AddReductionVar(Phi, RecurKind::FMulAdd, TheLoop, FMF, RedDes, DB, AC, DT,
                       SE)) {
     LLVM_DEBUG(dbgs() << "Found an FMulAdd reduction PHI." << *Phi << "\n");

>From 05ee143fc1ca8a7127f50111b98d1ab5350e20d5 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Tue, 3 Dec 2024 11:06:18 +0000
Subject: [PATCH 2/3] IVDescriptors: strip FAnyOf altogether

---
 llvm/include/llvm/Analysis/IVDescriptors.h         | 14 +++++++-------
 llvm/lib/Analysis/IVDescriptors.cpp                | 14 ++++++--------
 .../Target/AArch64/AArch64TargetTransformInfo.cpp  |  3 +--
 llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h   |  3 +--
 llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp    | 10 ++++------
 5 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/llvm/include/llvm/Analysis/IVDescriptors.h b/llvm/include/llvm/Analysis/IVDescriptors.h
index 5d992faf99d27f..7e8fa85093ff24 100644
--- a/llvm/include/llvm/Analysis/IVDescriptors.h
+++ b/llvm/include/llvm/Analysis/IVDescriptors.h
@@ -48,10 +48,8 @@ enum class RecurKind {
   FMinimum, ///< FP min with llvm.minimum semantics
   FMaximum, ///< FP max with llvm.maximum semantics
   FMulAdd,  ///< Sum of float products with llvm.fmuladd(a * b + sum).
-  IAnyOf,   ///< Any_of reduction with select(icmp(),x,y) where one of (x,y) is
-            ///< loop invariant, and both x and y are integer type.
-  FAnyOf    ///< Any_of reduction with select(fcmp(),x,y) where one of (x,y) is
-            ///< loop invariant, and both x and y are integer type.
+  AnyOf,    ///< Any_of reduction with select(cmp(),x,y) where one of (x,y) is
+            ///< loop invariant.
   // TODO: Any_of reduction need not be restricted to integer type only.
 };
 
@@ -156,7 +154,7 @@ class RecurrenceDescriptor {
   static InstDesc isConditionalRdxPattern(RecurKind Kind, Instruction *I);
 
   /// Returns the opcode corresponding to the RecurrenceKind.
-  static unsigned getOpcode(RecurKind Kind);
+  static unsigned getOpcode(RecurKind Kind, Type *Ty);
 
   /// Returns true if Phi is a reduction of type Kind and adds it to the
   /// RecurrenceDescriptor. If either \p DB is non-null or \p AC and \p DT are
@@ -192,7 +190,9 @@ class RecurrenceDescriptor {
 
   RecurKind getRecurrenceKind() const { return Kind; }
 
-  unsigned getOpcode() const { return getOpcode(getRecurrenceKind()); }
+  unsigned getOpcode() const {
+    return getOpcode(getRecurrenceKind(), getRecurrenceType());
+  }
 
   FastMathFlags getFastMathFlags() const { return FMF; }
 
@@ -233,7 +233,7 @@ class RecurrenceDescriptor {
   /// Returns true if the recurrence kind is of the form
   ///   select(cmp(),x,y) where one of (x,y) is loop invariant.
   static bool isAnyOfRecurrenceKind(RecurKind Kind) {
-    return Kind == RecurKind::IAnyOf || Kind == RecurKind::FAnyOf;
+    return Kind == RecurKind::AnyOf;
   }
 
   /// Returns the type of the recurrence. This type can be narrower than the
diff --git a/llvm/lib/Analysis/IVDescriptors.cpp b/llvm/lib/Analysis/IVDescriptors.cpp
index ce045389ff815d..f1a4690d443fc0 100644
--- a/llvm/lib/Analysis/IVDescriptors.cpp
+++ b/llvm/lib/Analysis/IVDescriptors.cpp
@@ -49,8 +49,7 @@ bool RecurrenceDescriptor::isIntegerRecurrenceKind(RecurKind Kind) {
   case RecurKind::SMin:
   case RecurKind::UMax:
   case RecurKind::UMin:
-  case RecurKind::IAnyOf:
-  case RecurKind::FAnyOf:
+  case RecurKind::AnyOf:
     return true;
   }
   return false;
@@ -651,8 +650,7 @@ 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, RecurKind::AnyOf);
 }
 
 RecurrenceDescriptor::InstDesc
@@ -884,7 +882,7 @@ bool RecurrenceDescriptor::isReductionPHI(PHINode *Phi, Loop *TheLoop,
     LLVM_DEBUG(dbgs() << "Found a UMIN reduction PHI." << *Phi << "\n");
     return true;
   }
-  if (AddReductionVar(Phi, RecurKind::IAnyOf, TheLoop, FMF, RedDes, DB, AC, DT,
+  if (AddReductionVar(Phi, RecurKind::AnyOf, TheLoop, FMF, RedDes, DB, AC, DT,
                       SE)) {
     LLVM_DEBUG(dbgs() << "Found an conditional select reduction PHI." << *Phi
                       << "\n");
@@ -1017,7 +1015,7 @@ bool RecurrenceDescriptor::isFixedOrderRecurrence(PHINode *Phi, Loop *TheLoop,
   return true;
 }
 
-unsigned RecurrenceDescriptor::getOpcode(RecurKind Kind) {
+unsigned RecurrenceDescriptor::getOpcode(RecurKind Kind, Type *Ty) {
   switch (Kind) {
   case RecurKind::Add:
     return Instruction::Add;
@@ -1038,14 +1036,14 @@ unsigned RecurrenceDescriptor::getOpcode(RecurKind Kind) {
   case RecurKind::SMin:
   case RecurKind::UMax:
   case RecurKind::UMin:
-  case RecurKind::IAnyOf:
     return Instruction::ICmp;
   case RecurKind::FMax:
   case RecurKind::FMin:
   case RecurKind::FMaximum:
   case RecurKind::FMinimum:
-  case RecurKind::FAnyOf:
     return Instruction::FCmp;
+  case RecurKind::AnyOf:
+    return Ty->isIntegerTy() ? Instruction::ICmp : Instruction::FCmp;
   default:
     llvm_unreachable("Unknown recurrence operation");
   }
diff --git a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
index 5b333d33cffd52..aa3af71672eae3 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -4180,8 +4180,7 @@ bool AArch64TTIImpl::isLegalToVectorizeReduction(
   case RecurKind::FMin:
   case RecurKind::FMax:
   case RecurKind::FMulAdd:
-  case RecurKind::IAnyOf:
-  case RecurKind::FAnyOf:
+  case RecurKind::AnyOf:
     return true;
   default:
     return false;
diff --git a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
index bd90bfed6e2c95..e9dbd32d998f2d 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
@@ -348,8 +348,7 @@ class RISCVTTIImpl : public BasicTTIImplBase<RISCVTTIImpl> {
     case RecurKind::FMin:
     case RecurKind::FMax:
     case RecurKind::FMulAdd:
-    case RecurKind::IAnyOf:
-    case RecurKind::FAnyOf:
+    case RecurKind::AnyOf:
       return true;
     default:
       return false;
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 33657c26356d65..7e42575b725869 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -19241,7 +19241,7 @@ class HorizontalReduction {
   /// Creates reduction operation with the current opcode.
   static Value *createOp(IRBuilderBase &Builder, RecurKind Kind, Value *LHS,
                          Value *RHS, const Twine &Name, bool UseSelect) {
-    unsigned RdxOpcode = RecurrenceDescriptor::getOpcode(Kind);
+    unsigned RdxOpcode = RecurrenceDescriptor::getOpcode(Kind, LHS->getType());
     switch (Kind) {
     case RecurKind::Or:
       if (UseSelect &&
@@ -20384,7 +20384,7 @@ class HorizontalReduction {
     case RecurKind::Xor:
     case RecurKind::FAdd:
     case RecurKind::FMul: {
-      unsigned RdxOpcode = RecurrenceDescriptor::getOpcode(RdxKind);
+      unsigned RdxOpcode = RecurrenceDescriptor::getOpcode(RdxKind, ScalarTy);
       if (!AllConsts) {
         if (auto *VecTy = dyn_cast<FixedVectorType>(ScalarTy)) {
           assert(SLPReVec && "FixedVectorType is not expected.");
@@ -20513,8 +20513,7 @@ class HorizontalReduction {
     case RecurKind::Mul:
     case RecurKind::FMul:
     case RecurKind::FMulAdd:
-    case RecurKind::IAnyOf:
-    case RecurKind::FAnyOf:
+    case RecurKind::AnyOf:
     case RecurKind::None:
       llvm_unreachable("Unexpected reduction kind for repeated scalar.");
     }
@@ -20610,8 +20609,7 @@ class HorizontalReduction {
     case RecurKind::Mul:
     case RecurKind::FMul:
     case RecurKind::FMulAdd:
-    case RecurKind::IAnyOf:
-    case RecurKind::FAnyOf:
+    case RecurKind::AnyOf:
     case RecurKind::None:
       llvm_unreachable("Unexpected reduction kind for reused scalars.");
     }

>From 71d0bc859710334cca08a6ee17d271cc7cd4d4bb Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Tue, 3 Dec 2024 18:37:50 +0000
Subject: [PATCH 3/3] IVDesc: fix serious editing error; tighten checks

---
 llvm/lib/Analysis/IVDescriptors.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Analysis/IVDescriptors.cpp b/llvm/lib/Analysis/IVDescriptors.cpp
index f1a4690d443fc0..c2a961a102d5d5 100644
--- a/llvm/lib/Analysis/IVDescriptors.cpp
+++ b/llvm/lib/Analysis/IVDescriptors.cpp
@@ -414,7 +414,7 @@ bool RecurrenceDescriptor::AddReductionVar(
     if (IsAPhi && Cur != Phi && !areAllUsesIn(Cur, VisitedInsts))
       return false;
 
-    if (isIntMinMaxRecurrenceKind(Kind) || isAnyOfRecurrenceKind(Kind))
+    if (isMinMaxRecurrenceKind(Kind) || isAnyOfRecurrenceKind(Kind))
       if (match(Cur, m_Select(m_Cmp(), m_Value(), m_Value())))
         ++NumCmpSelectPatternInst;
 



More information about the llvm-commits mailing list