[llvm] [SCEV] Fix scMulExpr cost for multiply by -1 and power-of-2 (PR #191033)
Adel Ejjeh via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 8 12:47:18 PDT 2026
https://github.com/adelejjeh updated https://github.com/llvm/llvm-project/pull/191033
>From 2edc5e05b26b4fe69ea11d4ab200e824b8ffc4e2 Mon Sep 17 00:00:00 2001
From: Adel Ejjeh <adel.ejjeh at amd.com>
Date: Wed, 8 Apr 2026 14:06:11 -0500
Subject: [PATCH 1/2] [SCEV] Fix scMulExpr cost for multiply by -1 and
power-of-2 Assisted-by: Cursor (Claude)
---
.../Utils/ScalarEvolutionExpander.cpp | 24 ++++++++---
.../LoopUnroll/scev-mul-expansion-cost.ll | 40 +++++++++++++++++++
2 files changed, 59 insertions(+), 5 deletions(-)
create mode 100644 llvm/test/Transforms/LoopUnroll/scev-mul-expansion-cost.ll
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index a560c324b4f1e..5f2468f54e2fd 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -2023,12 +2023,26 @@ template<typename T> static InstructionCost costAndCollectOperands(
case scAddExpr:
Cost = ArithCost(Instruction::Add, S->getNumOperands() - 1);
break;
- case scMulExpr:
- // TODO: this is a very pessimistic cost modelling for Mul,
- // because of Bin Pow algorithm actually used by the expander,
- // see SCEVExpander::visitMulExpr(), ExpandOpBinPowN().
- Cost = ArithCost(Instruction::Mul, S->getNumOperands() - 1);
+ case scMulExpr: {
+ // Match the actual expansion in visitMulExpr: multiply by -1 is
+ // expanded as a negate (sub 0, x), and multiply by a power of 2 is
+ // expanded as a shift. Only handle the common two-operand case with a
+ // constant LHS; for everything else fall back to the pessimistic
+ // all-multiplies estimate.
+ // TODO: this is still pessimistic for the general case because of the
+ // Bin Pow algorithm actually used by the expander, see
+ // SCEVExpander::visitMulExpr(), ExpandOpBinPowN().
+ unsigned MulOpc = Instruction::Mul;
+ if (S->getNumOperands() == 2)
+ if (auto *SC = dyn_cast<SCEVConstant>(S->getOperand(0))) {
+ if (SC->getAPInt().isAllOnes()) // -1
+ MulOpc = Instruction::Sub;
+ else if (SC->getAPInt().isPowerOf2())
+ MulOpc = Instruction::Shl;
+ }
+ Cost = ArithCost(MulOpc, S->getNumOperands() - 1);
break;
+ }
case scSMaxExpr:
case scUMaxExpr:
case scSMinExpr:
diff --git a/llvm/test/Transforms/LoopUnroll/scev-mul-expansion-cost.ll b/llvm/test/Transforms/LoopUnroll/scev-mul-expansion-cost.ll
new file mode 100644
index 0000000000000..8e188f280fdf3
--- /dev/null
+++ b/llvm/test/Transforms/LoopUnroll/scev-mul-expansion-cost.ll
@@ -0,0 +1,40 @@
+; RUN: opt -S -passes=loop-unroll -unroll-runtime < %s | FileCheck %s
+
+; The trip count SCEV for "for (i = start; i < end; i++)" is
+; (-1 * start) + end. The SCEV expansion cost model should recognize that
+; (-1 * X) is expanded as a negate (sub 0, X), not a real multiply. On
+; targets where i32 mul is expensive (e.g. AMDGPU quarter-rate), the old
+; cost model would over-count this as a mul (cost 4) exceeding the default
+; budget of 4, and reject runtime unrolling. With the fix, it's costed as
+; a sub (cost 1), well within budget.
+
+; AMDGPU is used because its i32 mul has a cost of 4,
+; making the over-counting observable against the default budget of 4.
+target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-p9:192:256:256:32-ni:7:8:9"
+target triple = "amdgcn-amd-amdhsa"
+
+; CHECK-LABEL: @offset_start_loop
+; The loop should be runtime-unrolled 8x (prologue + main unrolled body).
+; CHECK: %xtraiter = and i32 %{{.*}}, 7
+; CHECK: loop.prol:
+; CHECK: loop:
+; CHECK: %iv.next.7 = add nsw i32 %iv, 8
+define void @offset_start_loop(ptr addrspace(1) %out, ptr addrspace(1) %in, i32 %start, i32 %end) {
+entry:
+ %cmp = icmp slt i32 %start, %end
+ br i1 %cmp, label %loop, label %exit
+
+loop:
+ %iv = phi i32 [ %start, %entry ], [ %iv.next, %loop ]
+ %idx = sext i32 %iv to i64
+ %gep.in = getelementptr inbounds float, ptr addrspace(1) %in, i64 %idx
+ %val = load float, ptr addrspace(1) %gep.in, align 4
+ %gep.out = getelementptr inbounds float, ptr addrspace(1) %out, i64 %idx
+ store float %val, ptr addrspace(1) %gep.out, align 4
+ %iv.next = add nsw i32 %iv, 1
+ %cond = icmp slt i32 %iv.next, %end
+ br i1 %cond, label %loop, label %exit
+
+exit:
+ ret void
+}
>From ac18a83a8ce2ab48e849eaa4b8c25531c08c1b28 Mon Sep 17 00:00:00 2001
From: Adel Ejjeh <adel.ejjeh at amd.com>
Date: Wed, 8 Apr 2026 14:46:43 -0500
Subject: [PATCH 2/2] Update variable name and move lit test
---
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp | 8 ++++----
.../LoopUnroll/{ => AMDGPU}/scev-mul-expansion-cost.ll | 0
2 files changed, 4 insertions(+), 4 deletions(-)
rename llvm/test/Transforms/LoopUnroll/{ => AMDGPU}/scev-mul-expansion-cost.ll (100%)
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index 5f2468f54e2fd..d54e4de81d995 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -2032,15 +2032,15 @@ template<typename T> static InstructionCost costAndCollectOperands(
// TODO: this is still pessimistic for the general case because of the
// Bin Pow algorithm actually used by the expander, see
// SCEVExpander::visitMulExpr(), ExpandOpBinPowN().
- unsigned MulOpc = Instruction::Mul;
+ unsigned OpCode = Instruction::Mul;
if (S->getNumOperands() == 2)
if (auto *SC = dyn_cast<SCEVConstant>(S->getOperand(0))) {
if (SC->getAPInt().isAllOnes()) // -1
- MulOpc = Instruction::Sub;
+ OpCode = Instruction::Sub;
else if (SC->getAPInt().isPowerOf2())
- MulOpc = Instruction::Shl;
+ OpCode = Instruction::Shl;
}
- Cost = ArithCost(MulOpc, S->getNumOperands() - 1);
+ Cost = ArithCost(OpCode, S->getNumOperands() - 1);
break;
}
case scSMaxExpr:
diff --git a/llvm/test/Transforms/LoopUnroll/scev-mul-expansion-cost.ll b/llvm/test/Transforms/LoopUnroll/AMDGPU/scev-mul-expansion-cost.ll
similarity index 100%
rename from llvm/test/Transforms/LoopUnroll/scev-mul-expansion-cost.ll
rename to llvm/test/Transforms/LoopUnroll/AMDGPU/scev-mul-expansion-cost.ll
More information about the llvm-commits
mailing list