[LV] Address review

Author: artagnon

llvm/include/llvm/Analysis/IVDescriptors.h

@@ -54,9 +54,12 @@ enum class RecurKind {
   FMulAdd,  ///< Sum of float products with llvm.fmuladd(a * b + sum).
   AnyOf,    ///< AnyOf reduction with select(cmp(),x,y) where one of (x,y) is
             ///< loop invariant, and both x and y are integer type.
-  FindLastIV, ///< FindLast reduction with select(cmp(),x,y) where one of
-              ///< (x,y) is increasing loop induction, and both x and y are
-              ///< integer type.
+  FindLastIVSMin, ///< FindLast reduction with select(cmp(),x,y) where one of
+                  ///< (x,y) is increasing loop induction, and both x and y
+                  ///< are integer type, producing a SMin reduction.
+  FindLastIVUMin, ///< FindLast reduction with select(cmp(),x,y) where one of
+                  ///< (x,y) is increasing loop induction, and both x and y
+                  ///< are integer type, producting a UMin reduction.
   // clang-format on
   // TODO: Any_of and FindLast reduction need not be restricted to integer type
   // only.
@@ -80,13 +83,12 @@ class RecurrenceDescriptor {
 
   RecurrenceDescriptor(Value *Start, Instruction *Exit, StoreInst *Store,
                        RecurKind K, FastMathFlags FMF, Instruction *ExactFP,
-                       Type *RT, bool IsResultSigned, bool IsReduxSigned,
-                       bool Ordered, SmallPtrSetImpl<Instruction *> &CI,
+                       Type *RT, bool Signed, bool Ordered,
+                       SmallPtrSetImpl<Instruction *> &CI,
                        unsigned MinWidthCastToRecurTy)
       : IntermediateStore(Store), StartValue(Start), LoopExitInstr(Exit),
         Kind(K), FMF(FMF), ExactFPMathInst(ExactFP), RecurrenceType(RT),
-        IsResultSigned(IsResultSigned), IsReduxSigned(IsReduxSigned),
-        IsOrdered(Ordered),
+        IsSigned(Signed), IsOrdered(Ordered),
         MinWidthCastToRecurrenceType(MinWidthCastToRecurTy) {
     CastInsts.insert_range(CI);
   }
@@ -98,14 +100,12 @@ class RecurrenceDescriptor {
         : IsRecurrence(IsRecur), PatternLastInst(I),
           RecKind(RecurKind::None), ExactFPMathInst(ExactFP) {}
 
-    InstDesc(Instruction *I, RecurKind K, bool IsSigned = false)
-        : IsRecurrence(true), IsSigned(IsSigned), PatternLastInst(I),
-          RecKind(K) {}
+    InstDesc(Instruction *I, RecurKind K, Instruction *ExactFP = nullptr)
+        : IsRecurrence(true), PatternLastInst(I), RecKind(K),
+          ExactFPMathInst(ExactFP) {}
 
     bool isRecurrence() const { return IsRecurrence; }
 
-    bool isSigned() const { return IsSigned; }
-
     bool needsExactFPMath() const { return ExactFPMathInst != nullptr; }
 
     Instruction *getExactFPMathInst() const { return ExactFPMathInst; }
@@ -117,15 +117,13 @@ class RecurrenceDescriptor {
   private:
     // Is this instruction a recurrence candidate.
     bool IsRecurrence;
-    // Is this recurrence a signed variant.
-    bool IsSigned = false;
     // The last instruction in a min/max pattern (select of the select(icmp())
     // pattern), or the current recurrence instruction otherwise.
     Instruction *PatternLastInst;
     // If this is a min/max pattern.
     RecurKind RecKind;
     // Recurrence does not allow floating-point reassociation.
-    Instruction *ExactFPMathInst = nullptr;
+    Instruction *ExactFPMathInst;
   };
 
   /// Returns a struct describing if the instruction 'I' can be a recurrence
@@ -265,7 +263,14 @@ class RecurrenceDescriptor {
   /// Returns true if the recurrence kind is of the form
   ///   select(cmp(),x,y) where one of (x,y) is increasing loop induction.
   static bool isFindLastIVRecurrenceKind(RecurKind Kind) {
-    return Kind == RecurKind::FindLastIV;
+    return Kind == RecurKind::FindLastIVSMin ||
+           Kind == RecurKind::FindLastIVUMin;
+  }
+
+  /// Returns true if recurrece kind is a signed redux kind.
+  static bool isSignedRecurrenceKind(RecurKind Kind) {
+    return Kind == RecurKind::SMax || Kind == RecurKind::SMin ||
+           Kind == RecurKind::FindLastIVSMin;
   }
 
   /// Returns the type of the recurrence. This type can be narrower than the
@@ -278,8 +283,9 @@ class RecurrenceDescriptor {
     assert(isFindLastIVRecurrenceKind(Kind) && "Unexpected recurrence kind");
     Type *Ty = StartValue->getType();
     unsigned BW = Ty->getIntegerBitWidth();
-    return ConstantInt::get(Ty, isReduxSigned() ? APInt::getSignedMinValue(BW)
-                                                : APInt::getMinValue(BW));
+    return ConstantInt::get(Ty, isSignedRecurrenceKind(Kind)
+                                    ? APInt::getSignedMinValue(BW)
+                                    : APInt::getMinValue(BW));
   }
 
   /// Returns a reference to the instructions used for type-promoting the
@@ -291,11 +297,8 @@ class RecurrenceDescriptor {
     return MinWidthCastToRecurrenceType;
   }
 
-  /// Returns true if the reduction result is signed.
-  bool isResultSigned() const { return IsResultSigned; }
-
-  /// Returns true if the reduction redux is signed.
-  bool isReduxSigned() const { return IsReduxSigned; }
+  /// Returns true if all source operands of the recurrence are SExtInsts.
+  bool isSigned() const { return IsSigned; }
 
   /// Expose an ordered FP reduction to the instance users.
   bool isOrdered() const { return IsOrdered; }
@@ -331,10 +334,8 @@ class RecurrenceDescriptor {
   Instruction *ExactFPMathInst = nullptr;
   // The type of the recurrence.
   Type *RecurrenceType = nullptr;
-  // True if reduction result is signed.
-  bool IsResultSigned = false;
-  // True if reduction redux is signed.
-  bool IsReduxSigned = false;
+  // True if all source operands of the recurrence are SExtInsts.
+  bool IsSigned = false;
   // True if this recurrence can be treated as an in-order reduction.
   // Currently only a non-reassociative FAdd can be considered in-order,
   // if it is also the only FAdd in the PHI's use chain.

llvm/lib/Analysis/IVDescriptors.cpp

@@ -50,7 +50,8 @@ bool RecurrenceDescriptor::isIntegerRecurrenceKind(RecurKind Kind) {
   case RecurKind::UMax:
   case RecurKind::UMin:
   case RecurKind::AnyOf:
-  case RecurKind::FindLastIV:
+  case RecurKind::FindLastIVSMin:
+  case RecurKind::FindLastIVUMin:
     return true;
   }
   return false;
@@ -88,7 +89,7 @@ static Instruction *lookThroughAnd(PHINode *Phi, Type *&RT,
 }
 
 /// Compute the minimal bit width needed to represent a reduction whose exit
-/// instruction is given by Exit, along with its signedness.
+/// instruction is given by Exit.
 static std::pair<Type *, bool> computeRecurrenceType(Instruction *Exit,
                                                      DemandedBits *DB,
                                                      AssumptionCache *AC,
@@ -255,7 +256,7 @@ bool RecurrenceDescriptor::AddReductionVar(
   SmallPtrSet<Instruction *, 4> CastInsts;
   unsigned MinWidthCastToRecurrenceType;
   Instruction *Start = Phi;
-  bool IsResultSigned = false, IsReduxSigned = false;
+  bool IsSigned = false;
 
   SmallPtrSet<Instruction *, 8> VisitedInsts;
   SmallVector<Instruction *, 8> Worklist;
@@ -396,7 +397,6 @@ bool RecurrenceDescriptor::AddReductionVar(
       //       state accurate while processing the worklist?
       if (ReduxDesc.getRecKind() != RecurKind::None)
         Kind = ReduxDesc.getRecKind();
-      IsReduxSigned = ReduxDesc.isSigned();
     }
 
     bool IsASelect = isa<SelectInst>(Cur);
@@ -566,7 +566,7 @@ bool RecurrenceDescriptor::AddReductionVar(
     //       smaller type. We should just generate a correctly typed expression
     //       to begin with.
     Type *ComputedType;
-    std::tie(ComputedType, IsResultSigned) =
+    std::tie(ComputedType, IsSigned) =
         computeRecurrenceType(ExitInstruction, DB, AC, DT);
     if (ComputedType != RecurrenceType)
       return false;
@@ -596,9 +596,8 @@ bool RecurrenceDescriptor::AddReductionVar(
 
   // Save the description of this reduction variable.
   RecurrenceDescriptor RD(RdxStart, ExitInstruction, IntermediateStore, Kind,
-                          FMF, ExactFPMathInst, RecurrenceType, IsResultSigned,
-                          IsReduxSigned, IsOrdered, CastInsts,
-                          MinWidthCastToRecurrenceType);
+                          FMF, ExactFPMathInst, RecurrenceType, IsSigned,
+                          IsOrdered, CastInsts, MinWidthCastToRecurrenceType);
   RedDes = RD;
 
   return true;
@@ -704,7 +703,7 @@ RecurrenceDescriptor::isFindLastIVPattern(Loop *TheLoop, PHINode *OrigPhi,
 
   // Returns a non-nullopt boolean indicating the signedness of the recurrence
   // when a valid FindLastIV pattern is found.
-  auto GetInductionSignedness = [&](Value *V) -> std::optional<bool> {
+  auto GetRecurKind = [&](Value *V) -> std::optional<RecurKind> {
     Type *Ty = V->getType();
     if (!SE.isSCEVable(Ty))
       return std::nullopt;
@@ -741,18 +740,18 @@ RecurrenceDescriptor::isFindLastIVPattern(Loop *TheLoop, PHINode *OrigPhi,
       return ValidRange.contains(IVRange);
     };
     if (CheckRange(true))
-      return true;
+      return RecurKind::FindLastIVSMin;
     if (CheckRange(false))
-      return false;
+      return RecurKind::FindLastIVUMin;
     return std::nullopt;
   };
 
   // We are looking for selects of the form:
   //   select(cmp(), phi, increasing_loop_induction) or
   //   select(cmp(), increasing_loop_induction, phi)
   // TODO: Support for monotonically decreasing induction variable
-  if (auto IsSigned = GetInductionSignedness(NonRdxPhi))
-    return InstDesc(I, RecurKind::FindLastIV, *IsSigned);
+  if (auto RK = GetRecurKind(NonRdxPhi))
+    return InstDesc(I, *RK);
 
   return InstDesc(false, I);
 }
@@ -999,8 +998,8 @@ bool RecurrenceDescriptor::isReductionPHI(PHINode *Phi, Loop *TheLoop,
                       << "\n");
     return true;
   }
-  if (AddReductionVar(Phi, RecurKind::FindLastIV, TheLoop, FMF, RedDes, DB, AC,
-                      DT, SE)) {
+  if (AddReductionVar(Phi, RecurKind::FindLastIVSMin, TheLoop, FMF, RedDes, DB,
+                      AC, DT, SE)) {
     LLVM_DEBUG(dbgs() << "Found a FindLastIV reduction PHI." << *Phi << "\n");
     return true;
   }
@@ -1151,7 +1150,8 @@ unsigned RecurrenceDescriptor::getOpcode(RecurKind Kind) {
   case RecurKind::Mul:
     return Instruction::Mul;
   case RecurKind::AnyOf:
-  case RecurKind::FindLastIV:
+  case RecurKind::FindLastIVSMin:
+  case RecurKind::FindLastIVUMin:
   case RecurKind::Or:
     return Instruction::Or;
   case RecurKind::And:

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -9215,7 +9215,7 @@ void LoopVectorizationPlanner::adjustRecipesForReductions(
       auto *Trunc =
           new VPWidenCastRecipe(Instruction::Trunc, NewExitingVPV, RdxTy);
       auto *Extnd =
-          RdxDesc.isResultSigned()
+          RdxDesc.isSigned()
               ? new VPWidenCastRecipe(Instruction::SExt, Trunc, PhiTy)
               : new VPWidenCastRecipe(Instruction::ZExt, Trunc, PhiTy);
 

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

@@ -23105,7 +23105,8 @@ class HorizontalReduction {
         case RecurKind::FMul:
         case RecurKind::FMulAdd:
         case RecurKind::AnyOf:
-        case RecurKind::FindLastIV:
+        case RecurKind::FindLastIVSMin:
+        case RecurKind::FindLastIVUMin:
         case RecurKind::FMaximumNum:
         case RecurKind::FMinimumNum:
         case RecurKind::None:
@@ -23239,7 +23240,8 @@ class HorizontalReduction {
     case RecurKind::FMul:
     case RecurKind::FMulAdd:
     case RecurKind::AnyOf:
-    case RecurKind::FindLastIV:
+    case RecurKind::FindLastIVSMin:
+    case RecurKind::FindLastIVUMin:
     case RecurKind::FMaximumNum:
     case RecurKind::FMinimumNum:
     case RecurKind::None:
@@ -23338,7 +23340,8 @@ class HorizontalReduction {
     case RecurKind::FMul:
     case RecurKind::FMulAdd:
     case RecurKind::AnyOf:
-    case RecurKind::FindLastIV:
+    case RecurKind::FindLastIVSMin:
+    case RecurKind::FindLastIVUMin:
     case RecurKind::FMaximumNum:
     case RecurKind::FMinimumNum:
     case RecurKind::None:

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

@@ -634,10 +634,12 @@ Value *VPInstruction::generate(VPTransformState &State) {
     // each part of the reduction.
     unsigned UF = getNumOperands() - 2;
     Value *ReducedPartRdx = State.get(getOperand(2));
+    auto MinMaxOp = RecurrenceDescriptor::isSignedRecurrenceKind(RK)
+                        ? RecurKind::SMax
+                        : RecurKind::UMax;
     for (unsigned Part = 1; Part < UF; ++Part) {
-      ReducedPartRdx = createMinMaxOp(
-          Builder, RdxDesc.isReduxSigned() ? RecurKind::SMax : RecurKind::UMax,
-          ReducedPartRdx, State.get(getOperand(2 + Part)));
+      ReducedPartRdx = createMinMaxOp(Builder, MinMaxOp, ReducedPartRdx,
+                                      State.get(getOperand(2 + Part)));
     }
 
     return createFindLastIVReduction(Builder, ReducedPartRdx,
@@ -705,7 +707,7 @@ Value *VPInstruction::generate(VPTransformState &State) {
       // If the reduction can be performed in a smaller type, we need to extend
       // the reduction to the wider type before we branch to the original loop.
       if (PhiTy != RdxDesc.getRecurrenceType())
-        ReducedPartRdx = RdxDesc.isResultSigned()
+        ReducedPartRdx = RdxDesc.isSigned()
                              ? Builder.CreateSExt(ReducedPartRdx, PhiTy)
                              : Builder.CreateZExt(ReducedPartRdx, PhiTy);
     }