[llvm] [SLP] Ensure TreeCost is scaled for ordered fadd reductions (PR #199388)

Kavin Gnanapandithan via llvm-commits llvm-commits at lists.llvm.org
Sun May 24 10:57:42 PDT 2026


https://github.com/KavinTheG updated https://github.com/llvm/llvm-project/pull/199388

>From fa691097d3528e1aacd8b32e5c62b3333f038cad Mon Sep 17 00:00:00 2001
From: Kavin Gnanapandithan <kavin.balag at gmail.com>
Date: Sat, 23 May 2026 20:32:16 -0400
Subject: [PATCH 1/6] [SLP] Ensure TreeCost is scaled for ordered fadd
 reductions

Addresses an issue where `getScaleToLoopIterations()` returns 1 on
isolated SLP trees because `UserTreeIndex` is invalid. This prevents
`TreeCost` from scaling alongside `ReductionCost`, causing the cost
model to incorrectly treat an unprofitable vector reduction as
profitable.
---
 .../Transforms/Vectorize/SLPVectorizer.cpp    | 23 +++++++++++--------
 .../SLPVectorizer/RISCV/ordered-reduction.ll  | 16 ++++++-------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 54a4d6b68b2e5..5a0c1f6611363 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -2176,7 +2176,8 @@ class slpvectorizer::BoUpSLP {
   /// Calculates the cost of the subtrees, trims non-profitable ones and returns
   /// final cost.
   InstructionCost
-  calculateTreeCostAndTrimNonProfitable(ArrayRef<Value *> VectorizedVals = {});
+  calculateTreeCostAndTrimNonProfitable(ArrayRef<Value *> VectorizedVals = {},
+                                        Instruction *RdxRoot = nullptr);
 
   /// \returns the vectorization cost of the subtree that starts at \p VL.
   /// A negative number means that this is profitable.
@@ -3996,7 +3997,8 @@ class slpvectorizer::BoUpSLP {
   /// LICM hoisting that optimizeGatherSequence() performs after vectorization
   /// for inserts with loop-invariant operands. Falls back to the whole-entry
   /// scale when per-lane information is unavailable or the feature is off.
-  uint64_t getGatherNodeEffectiveScale(const TreeEntry &TE);
+  uint64_t getGatherNodeEffectiveScale(const TreeEntry &TE,
+                                       Instruction *U = nullptr);
 
   /// Get the loop nest for the given loop \p L.
   ArrayRef<const Loop *> getLoopNest(const Loop *L);
@@ -16455,14 +16457,15 @@ uint64_t BoUpSLP::getLoopNestScale(const Loop *L) {
   return std::max<uint64_t>(1, Scale);
 }
 
-uint64_t BoUpSLP::getGatherNodeEffectiveScale(const TreeEntry &TE) {
+uint64_t BoUpSLP::getGatherNodeEffectiveScale(const TreeEntry &TE,
+                                              Instruction *U) {
   // Only meaningful for gather/buildvector-like entries; the per-lane
   // insertelements that make up such an entry are LICM-hoistable by
   // optimizeGatherSequence() when their operand is loop-invariant.
   assert((TE.isGather() || TE.State == TreeEntry::SplitVectorize) &&
          "Expected gather/split tree entry.");
 
-  uint64_t BaseScale = getScaleToLoopIterations(TE);
+  uint64_t BaseScale = getScaleToLoopIterations(TE, nullptr, U);
   if (!PerLaneGatherScale || LoopAwareTripCount == 0 || BaseScale <= 1)
     return BaseScale;
 
@@ -16487,7 +16490,8 @@ uint64_t BoUpSLP::getGatherNodeEffectiveScale(const TreeEntry &TE) {
     if (isConstant(V))
       continue;
     ++N;
-    uint64_t LaneScale = std::min(getScaleToLoopIterations(TE, V), BaseScale);
+    uint64_t LaneScale =
+        std::min(getScaleToLoopIterations(TE, V, U), BaseScale);
     Sum = SaturatingAdd(Sum, LaneScale, &Overflow);
     if (Overflow)
       return BaseScale;
@@ -18840,7 +18844,7 @@ static T *performExtractsShuffleAction(
 }
 
 InstructionCost BoUpSLP::calculateTreeCostAndTrimNonProfitable(
-    ArrayRef<Value *> VectorizedVals) {
+    ArrayRef<Value *> VectorizedVals, Instruction *RdxRoot) {
   // FIXME: support buildvector of the gather nodes with struct types.
   if (any_of(VectorizableTree, [&](const std::unique_ptr<TreeEntry> &TE) {
         return TE->isGather() &&
@@ -18960,8 +18964,8 @@ InstructionCost BoUpSLP::calculateTreeCostAndTrimNonProfitable(
       }
     }
     if (!CostIsFree && !Scale) {
-      Scale = IsGatherLike ? getGatherNodeEffectiveScale(TE)
-                           : getScaleToLoopIterations(TE);
+      Scale = IsGatherLike ? getGatherNodeEffectiveScale(TE, RdxRoot)
+                           : getScaleToLoopIterations(TE, nullptr, RdxRoot);
       C *= Scale;
       EntryToScale.try_emplace(&TE, Scale);
       if (!TE.isGather() && TE.hasState()) {
@@ -29837,7 +29841,8 @@ class HorizontalReduction {
 
       V.transformNodes();
       V.computeMinimumValueSizes();
-      InstructionCost TreeCost = V.calculateTreeCostAndTrimNonProfitable(VL);
+      InstructionCost TreeCost =
+          V.calculateTreeCostAndTrimNonProfitable(VL, RdxRootInst);
       V.buildExternalUses(LocalExternallyUsedValues);
 
       InstructionCost ReductionCost =
diff --git a/llvm/test/Transforms/SLPVectorizer/RISCV/ordered-reduction.ll b/llvm/test/Transforms/SLPVectorizer/RISCV/ordered-reduction.ll
index bddb521998cc7..55abb29ddbdd4 100644
--- a/llvm/test/Transforms/SLPVectorizer/RISCV/ordered-reduction.ll
+++ b/llvm/test/Transforms/SLPVectorizer/RISCV/ordered-reduction.ll
@@ -20,20 +20,18 @@ define void @test1(ptr %arrayidx.i60, ptr %24, ptr %arrayidx44.i61, double %.pre
 ; CHECK-NEXT:    [[INDVARS_IV_NEXT_I74]] = add nuw nsw i64 [[INDVARS_IV_I72]], 1
 ; CHECK-NEXT:    [[ARRAYIDX23_I75:%.*]] = getelementptr inbounds nuw [8 x i8], ptr [[ARRAYIDX_I60]], i64 [[INDVARS_IV_NEXT_I74]]
 ; CHECK-NEXT:    [[TMP7]] = load double, ptr [[ARRAYIDX23_I75]], align 8
+; CHECK-NEXT:    [[ADD24_I76:%.*]] = fadd double [[ADD_I73]], [[TMP7]]
+; CHECK-NEXT:    [[ADD30_I77:%.*]] = fadd double [[TMP4]], [[ADD24_I76]]
 ; CHECK-NEXT:    [[ARRAYIDX34_I78:%.*]] = getelementptr inbounds nuw [8 x i8], ptr [[TMP0]], i64 [[INDVARS_IV_I72]]
+; CHECK-NEXT:    [[ADD35_I79:%.*]] = fadd double [[TMP3]], [[ADD30_I77]]
 ; CHECK-NEXT:    [[ARRAYIDX40_I80:%.*]] = getelementptr inbounds nuw [8 x i8], ptr [[TMP0]], i64 [[INDVARS_IV_NEXT_I74]]
 ; CHECK-NEXT:    [[TMP8]] = load double, ptr [[ARRAYIDX40_I80]], align 8
+; CHECK-NEXT:    [[ADD41_I81:%.*]] = fadd double [[TMP8]], [[ADD35_I79]]
+; CHECK-NEXT:    [[ADD48_I82:%.*]] = fadd double [[TMP2]], [[ADD41_I81]]
+; CHECK-NEXT:    [[ADD54_I83:%.*]] = fadd double [[TMP1]], [[ADD48_I82]]
 ; CHECK-NEXT:    [[ARRAYIDX60_I84:%.*]] = getelementptr inbounds nuw [8 x i8], ptr [[ARRAYIDX44_I61]], i64 [[INDVARS_IV_NEXT_I74]]
 ; CHECK-NEXT:    [[TMP9]] = load double, ptr [[ARRAYIDX60_I84]], align 8
-; CHECK-NEXT:    [[TMP10:%.*]] = insertelement <8 x double> poison, double [[TMP7]], i32 0
-; CHECK-NEXT:    [[TMP11:%.*]] = insertelement <8 x double> [[TMP10]], double [[ADD_I73]], i32 1
-; CHECK-NEXT:    [[TMP12:%.*]] = insertelement <8 x double> [[TMP11]], double [[TMP4]], i32 2
-; CHECK-NEXT:    [[TMP13:%.*]] = insertelement <8 x double> [[TMP12]], double [[TMP3]], i32 3
-; CHECK-NEXT:    [[TMP14:%.*]] = insertelement <8 x double> [[TMP13]], double [[TMP8]], i32 4
-; CHECK-NEXT:    [[TMP15:%.*]] = insertelement <8 x double> [[TMP14]], double [[TMP2]], i32 5
-; CHECK-NEXT:    [[TMP16:%.*]] = insertelement <8 x double> [[TMP15]], double [[TMP1]], i32 6
-; CHECK-NEXT:    [[TMP17:%.*]] = insertelement <8 x double> [[TMP16]], double [[TMP9]], i32 7
-; CHECK-NEXT:    [[TMP18:%.*]] = call double @llvm.vector.reduce.fadd.v8f64(double -0.000000e+00, <8 x double> [[TMP17]])
+; CHECK-NEXT:    [[TMP18:%.*]] = fadd double [[TMP9]], [[ADD54_I83]]
 ; CHECK-NEXT:    [[DIV_I86]] = fdiv double [[TMP18]], 9.000000e+00
 ; CHECK-NEXT:    store double [[DIV_I86]], ptr [[ARRAYIDX34_I78]], align 8
 ; CHECK-NEXT:    [[EXITCOND_NOT_I87:%.*]] = icmp eq i64 [[INDVARS_IV_NEXT_I74]], 1999

>From 60dbccfa5eb83d8795d15c61efc1cbfa18733fbf Mon Sep 17 00:00:00 2001
From: Kavin Gnanapandithan <kavin.balag at gmail.com>
Date: Sun, 24 May 2026 12:51:00 -0400
Subject: [PATCH 2/6] Ensure RdxRoot is only passed to getGathersNodeEffective
 in calculateTreeCostAndTrimNonProfitable

---
 llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 5a0c1f6611363..507dc18159005 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -18965,7 +18965,7 @@ InstructionCost BoUpSLP::calculateTreeCostAndTrimNonProfitable(
     }
     if (!CostIsFree && !Scale) {
       Scale = IsGatherLike ? getGatherNodeEffectiveScale(TE, RdxRoot)
-                           : getScaleToLoopIterations(TE, nullptr, RdxRoot);
+                           : getScaleToLoopIterations(TE);
       C *= Scale;
       EntryToScale.try_emplace(&TE, Scale);
       if (!TE.isGather() && TE.hasState()) {

>From 54b26f31dce342cb3bfd02c2e5487ea0d6455007 Mon Sep 17 00:00:00 2001
From: Kavin Gnanapandithan <kavin.balag at gmail.com>
Date: Sun, 24 May 2026 13:17:02 -0400
Subject: [PATCH 3/6] Only pass in RdxRoot to getGatherNodeEffectScale for the
 root node

---
 llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 507dc18159005..7b6193a78df02 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -18964,7 +18964,8 @@ InstructionCost BoUpSLP::calculateTreeCostAndTrimNonProfitable(
       }
     }
     if (!CostIsFree && !Scale) {
-      Scale = IsGatherLike ? getGatherNodeEffectiveScale(TE, RdxRoot)
+      Scale = IsGatherLike ? getGatherNodeEffectiveScale(
+                                 TE, (TE.Idx == 0) ? RdxRoot : nullptr)
                            : getScaleToLoopIterations(TE);
       C *= Scale;
       EntryToScale.try_emplace(&TE, Scale);

>From 88a778fd163fe045b58e555acb5f8ea852cbc6cb Mon Sep 17 00:00:00 2001
From: Kavin Gnanapandithan <kavin.balag at gmail.com>
Date: Sun, 24 May 2026 13:21:53 -0400
Subject: [PATCH 4/6] Dropped parens

---
 llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 7b6193a78df02..1f5ddc096d276 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -18965,7 +18965,7 @@ InstructionCost BoUpSLP::calculateTreeCostAndTrimNonProfitable(
     }
     if (!CostIsFree && !Scale) {
       Scale = IsGatherLike ? getGatherNodeEffectiveScale(
-                                 TE, (TE.Idx == 0) ? RdxRoot : nullptr)
+                                 TE, TE.Idx == 0 ? RdxRoot : nullptr)
                            : getScaleToLoopIterations(TE);
       C *= Scale;
       EntryToScale.try_emplace(&TE, Scale);

>From 4a0e1c461b709af0658a75df21d3cb9cfa8aed64 Mon Sep 17 00:00:00 2001
From: Kavin Gnanapandithan <kavin.balag at gmail.com>
Date: Sun, 24 May 2026 13:43:18 -0400
Subject: [PATCH 5/6] clang format

---
 llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 1f5ddc096d276..c7617b7695531 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -18964,9 +18964,10 @@ InstructionCost BoUpSLP::calculateTreeCostAndTrimNonProfitable(
       }
     }
     if (!CostIsFree && !Scale) {
-      Scale = IsGatherLike ? getGatherNodeEffectiveScale(
-                                 TE, TE.Idx == 0 ? RdxRoot : nullptr)
-                           : getScaleToLoopIterations(TE);
+      Scale =
+          IsGatherLike
+              ? getGatherNodeEffectiveScale(TE, TE.Idx == 0 ? RdxRoot : nullptr)
+              : getScaleToLoopIterations(TE);
       C *= Scale;
       EntryToScale.try_emplace(&TE, Scale);
       if (!TE.isGather() && TE.hasState()) {

>From 1f3afcecffc643ffc3e50b80fa5458d3e7d13af1 Mon Sep 17 00:00:00 2001
From: Kavin Gnanapandithan <kavin.balag at gmail.com>
Date: Sun, 24 May 2026 13:56:27 -0400
Subject: [PATCH 6/6] clang format

---
 llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index c7617b7695531..d90eba7118b37 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -18843,8 +18843,9 @@ static T *performExtractsShuffleAction(
   return Prev;
 }
 
-InstructionCost BoUpSLP::calculateTreeCostAndTrimNonProfitable(
-    ArrayRef<Value *> VectorizedVals, Instruction *RdxRoot) {
+InstructionCost
+BoUpSLP::calculateTreeCostAndTrimNonProfitable(ArrayRef<Value *> VectorizedVals,
+                                               Instruction *RdxRoot) {
   // FIXME: support buildvector of the gather nodes with struct types.
   if (any_of(VectorizableTree, [&](const std::unique_ptr<TreeEntry> &TE) {
         return TE->isGather() &&



More information about the llvm-commits mailing list