[llvm] [SLP][NFC] Avoid the getRdxOperand hack (PR #148672)

via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 14 10:03:10 PDT 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-vectorizers

Author: Piotr Fusik (pfusik)

<details>
<summary>Changes</summary>

We visit two operands of TreeN:
- 1, 2 if isCmpSelMinMax(TreeN)
- 0, 2 if TreeN is (select X, true, Y)
- 0, 1 otherwise

in the reverse order.

Replace the loop in CheckOperands and a substitution of the second index
in getRdxOperand with simply calling the code for the two determined indexes.

---
Full diff: https://github.com/llvm/llvm-project/pull/148672.diff


1 Files Affected:

- (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+32-47) 


``````````diff
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index c61e1135524b6..1b78e79b6b419 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -21791,16 +21791,6 @@ class HorizontalReduction {
     return I->isAssociative();
   }
 
-  static Value *getRdxOperand(Instruction *I, unsigned Index) {
-    // Poison-safe 'or' takes the form: select X, true, Y
-    // To make that work with the normal operand processing, we skip the
-    // true value operand.
-    // TODO: Change the code and data structures to handle this without a hack.
-    if (getRdxKind(I) == RecurKind::Or && isa<SelectInst>(I) && Index == 1)
-      return I->getOperand(2);
-    return I->getOperand(Index);
-  }
-
   /// Creates reduction operation with the current opcode.
   static Value *createOp(IRBuilderBase &Builder, RecurKind Kind, Value *LHS,
                          Value *RHS, const Twine &Name, bool UseSelect) {
@@ -21992,11 +21982,6 @@ class HorizontalReduction {
   }
 
 private:
-  /// Total number of operands in the reduction operation.
-  static unsigned getNumberOfOperands(Instruction *I) {
-    return isCmpSelMinMax(I) ? 3 : 2;
-  }
-
   /// Checks if the instruction is in basic block \p BB.
   /// For a cmp+sel min/max reduction check that both ops are in \p BB.
   static bool hasSameParent(Instruction *I, BasicBlock *BB) {
@@ -22102,31 +22087,27 @@ class HorizontalReduction {
     // Checks if the operands of the \p TreeN instruction are also reduction
     // operations or should be treated as reduced values or an extra argument,
     // which is not part of the reduction.
-    auto CheckOperands = [&](Instruction *TreeN,
-                             SmallVectorImpl<Value *> &PossibleReducedVals,
-                             SmallVectorImpl<Instruction *> &ReductionOps,
-                             unsigned Level) {
-      for (int I : reverse(seq<int>(getFirstOperandIndex(TreeN),
-                                    getNumberOfOperands(TreeN)))) {
-        Value *EdgeVal = getRdxOperand(TreeN, I);
-        ReducedValsToOps[EdgeVal].push_back(TreeN);
-        auto *EdgeInst = dyn_cast<Instruction>(EdgeVal);
-        // If the edge is not an instruction, or it is different from the main
-        // reduction opcode or has too many uses - possible reduced value.
-        // Also, do not try to reduce const values, if the operation is not
-        // foldable.
-        if (!EdgeInst || Level > RecursionMaxDepth ||
-            getRdxKind(EdgeInst) != RdxKind ||
-            IsCmpSelMinMax != isCmpSelMinMax(EdgeInst) ||
-            !hasRequiredNumberOfUses(IsCmpSelMinMax, EdgeInst) ||
-            !isVectorizable(RdxKind, EdgeInst) ||
-            (R.isAnalyzedReductionRoot(EdgeInst) &&
-             all_of(EdgeInst->operands(), IsaPred<Constant>))) {
-          PossibleReducedVals.push_back(EdgeVal);
-          continue;
-        }
+    auto CheckOperand = [&](Instruction *TreeN, int OperandIndex,
+                            SmallVectorImpl<Value *> &PossibleReducedVals,
+                            SmallVectorImpl<Instruction *> &ReductionOps,
+                            unsigned Level) {
+      Value *EdgeVal = TreeN->getOperand(OperandIndex);
+      ReducedValsToOps[EdgeVal].push_back(TreeN);
+      auto *EdgeInst = dyn_cast<Instruction>(EdgeVal);
+      // If the edge is not an instruction, or it is different from the main
+      // reduction opcode or has too many uses - possible reduced value.
+      // Also, do not try to reduce const values, if the operation is not
+      // foldable.
+      if (!EdgeInst || Level > RecursionMaxDepth ||
+          getRdxKind(EdgeInst) != RdxKind ||
+          IsCmpSelMinMax != isCmpSelMinMax(EdgeInst) ||
+          !hasRequiredNumberOfUses(IsCmpSelMinMax, EdgeInst) ||
+          !isVectorizable(RdxKind, EdgeInst) ||
+          (R.isAnalyzedReductionRoot(EdgeInst) &&
+           all_of(EdgeInst->operands(), IsaPred<Constant>)))
+        PossibleReducedVals.push_back(EdgeVal);
+      else
         ReductionOps.push_back(EdgeInst);
-      }
     };
     // Try to regroup reduced values so that it gets more profitable to try to
     // reduce them. Values are grouped by their value ids, instructions - by
@@ -22177,7 +22158,12 @@ class HorizontalReduction {
       auto [TreeN, Level] = Worklist.pop_back_val();
       SmallVector<Value *> PossibleRedVals;
       SmallVector<Instruction *> PossibleReductionOps;
-      CheckOperands(TreeN, PossibleRedVals, PossibleReductionOps, Level);
+      int I1 = getFirstOperandIndex(TreeN);
+      int I2 = isa<SelectInst>(TreeN) && match(TreeN, m_LogicalOr())
+                   ? 2 // Skip "true" in "select X, true, Y"
+                   : I1 + 1;
+      CheckOperand(TreeN, I2, PossibleRedVals, PossibleReductionOps, Level);
+      CheckOperand(TreeN, I1, PossibleRedVals, PossibleReductionOps, Level);
       addReductionOps(TreeN);
       // Add reduction values. The values are sorted for better vectorization
       // results.
@@ -22295,15 +22281,14 @@ class HorizontalReduction {
               isGuaranteedNotToBePoison(VectorizedTree, AC) ||
               (It != ReducedValsToOps.end() &&
                any_of(It->getSecond(), [&](Instruction *I) {
-                 return isBoolLogicOp(I) &&
-                        getRdxOperand(I, 0) == VectorizedTree;
+                 return isBoolLogicOp(I) && I->getOperand(0) == VectorizedTree;
                }))) {
             ;
           } else if (isGuaranteedNotToBePoison(Res, AC) ||
                      (It1 != ReducedValsToOps.end() &&
-                     any_of(It1->getSecond(), [&](Instruction *I) {
-                       return isBoolLogicOp(I) && getRdxOperand(I, 0) == Res;
-                     }))) {
+                      any_of(It1->getSecond(), [&](Instruction *I) {
+                        return isBoolLogicOp(I) && I->getOperand(0) == Res;
+                      }))) {
             std::swap(VectorizedTree, Res);
           } else {
             VectorizedTree = Builder.CreateFreeze(VectorizedTree);
@@ -22786,11 +22771,11 @@ class HorizontalReduction {
         if (!AnyBoolLogicOp)
           return;
         if (isBoolLogicOp(RedOp1) && ((!InitStep && LHS == VectorizedTree) ||
-                                      getRdxOperand(RedOp1, 0) == LHS ||
+                                      RedOp1->getOperand(0) == LHS ||
                                       isGuaranteedNotToBePoison(LHS, AC)))
           return;
         if (isBoolLogicOp(RedOp2) && ((!InitStep && RHS == VectorizedTree) ||
-                                      getRdxOperand(RedOp2, 0) == RHS ||
+                                      RedOp2->getOperand(0) == RHS ||
                                       isGuaranteedNotToBePoison(RHS, AC))) {
           std::swap(LHS, RHS);
           return;

``````````

</details>


https://github.com/llvm/llvm-project/pull/148672


More information about the llvm-commits mailing list