[llvm] [Reassociate] Distribute multiply over add to enable factorization (PR #178201)
via llvm-commits
llvm-commits at lists.llvm.org
Wed May 6 08:11:41 PDT 2026
https://github.com/hazarathayya updated https://github.com/llvm/llvm-project/pull/178201
>From dd763a23f606eb8c760a88bd013b704da68415e8 Mon Sep 17 00:00:00 2001
From: hazarathayya <hazarathayyayallanki at gmail.com>
Date: Tue, 28 Apr 2026 10:25:24 -0700
Subject: [PATCH 1/4] [Reassociate] Refine (X+Y)*C distribution with
PatternMatch and profitability checks
This patch adds distribution of (A+B)*C -> A*C + B*C to enable further factorization.
Key changes:
- Adopted PatternMatch for cleaner matching of (A +/- B) * C.
- Added hasOneUse() and Add/Sub parent checks for profitability.
- Modified FactorOccurrences tie-breaker to prefer Variables over Constants, preventing factorization from undoing the distribution and causing an infinite loop.
- Queues the parent instruction in RedoInsts to ensure proper linearization order.
---
llvm/lib/Transforms/Scalar/Reassociate.cpp | 86 +++++++++-
.../Reassociate/reassociate-distribute.ll | 154 ++++++++++++++++++
2 files changed, 237 insertions(+), 3 deletions(-)
create mode 100644 llvm/test/Transforms/Reassociate/reassociate-distribute.ll
diff --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp
index ea6c394740f22..989387d704923 100644
--- a/llvm/lib/Transforms/Scalar/Reassociate.cpp
+++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp
@@ -963,6 +963,68 @@ static BinaryOperator *convertOrWithNoCommonBitsToAdd(Instruction *Or) {
return New;
}
+/// Return true if Mul is of the form (X+Y)*C or (X-Y)*C where C is a
+/// constant, and there exists a sibling instruction of the form X*C' or Y*C'
+/// in the same expression — indicating that distribution followed by
+/// factoring will reduce the instruction count.
+static bool ShouldBreakUpDistribution(Instruction *Mul) {
+ Value *A, *B;
+ if (!match(Mul, m_c_Mul(m_OneUse(m_CombineOr(m_Add(m_Value(A), m_Value(B)),
+ m_Sub(m_Value(A), m_Value(B)))),
+ m_ImmConstant())))
+ return false;
+
+ if (!Mul->hasOneUse())
+ return false;
+
+ auto *MulUser = dyn_cast<Instruction>(Mul->user_back());
+ // The parent MUST be an Add or Sub to ensure the tree is flattened
+ if (!MulUser || (MulUser->getOpcode() != Instruction::Add &&
+ MulUser->getOpcode() != Instruction::Sub))
+ return false;
+
+ if (!MulUser)
+ return false;
+
+ for (Value *Sibling : MulUser->operands()) {
+ if (Sibling == Mul)
+ continue;
+
+ // Sibling must be NonConst * C'.
+ Value *SibNC;
+ if (match(Sibling, m_Mul(m_Value(SibNC), m_Constant()))) {
+ if ((SibNC == A || SibNC == B) && !isa<Constant>(SibNC))
+ return true;
+ }
+ }
+ return false;
+}
+
+/// Distribute Mul of the form (X+Y)*C into X*C + Y*C.
+/// For the sub case (X-Y)*C, the second term uses -C to avoid
+/// introducing a negation instruction.
+static BinaryOperator *BreakUpDistribute(Instruction *Mul,
+ ReassociatePass::OrderedSet &ToRedo) {
+
+ Instruction *AddSub = cast<Instruction>(Mul->getOperand(0));
+ Constant *C = cast<Constant>(Mul->getOperand(1));
+ Constant *C2 = (AddSub->getOpcode() == Instruction::Sub)
+ ? cast<Constant>(ConstantExpr::getNeg(C))
+ : C;
+
+ BinaryOperator *M1 =
+ CreateMul(AddSub->getOperand(0), C, "Mul1", Mul->getIterator(), Mul);
+ BinaryOperator *M2 =
+ CreateMul(AddSub->getOperand(1), C2, "Mul2", Mul->getIterator(), Mul);
+ BinaryOperator *Result;
+ Result = CreateAdd(M1, M2, "DistAdd", Mul->getIterator(), Mul);
+
+ Mul->replaceAllUsesWith(Result);
+ Result->setDebugLoc(Mul->getDebugLoc());
+
+ return Result;
+}
+
/// Return true if we should break up this subtract of X-Y into (X + -Y).
static bool ShouldBreakUpSubtract(Instruction *Sub) {
// If this is a negation, we can't split it up!
@@ -1603,7 +1665,10 @@ Value *ReassociatePass::OptimizeAdd(Instruction *I,
continue;
unsigned Occ = ++FactorOccurrences[Factor];
- if (Occ > MaxOcc) {
+ if (Occ > MaxOcc ||
+ (Occ == MaxOcc &&
+ (isa<Instruction>(Factor) || isa<Argument>(Factor)) &&
+ isa<Constant>(MaxOccVal) && !isa<UndefValue>(MaxOccVal))) {
MaxOcc = Occ;
MaxOccVal = Factor;
}
@@ -1617,7 +1682,10 @@ Value *ReassociatePass::OptimizeAdd(Instruction *I,
if (!Duplicates.insert(Factor).second)
continue;
unsigned Occ = ++FactorOccurrences[Factor];
- if (Occ > MaxOcc) {
+ if (Occ > MaxOcc ||
+ (Occ == MaxOcc &&
+ (isa<Instruction>(Factor) || isa<Argument>(Factor)) &&
+ isa<Constant>(MaxOccVal) && !isa<UndefValue>(MaxOccVal))) {
MaxOcc = Occ;
MaxOccVal = Factor;
}
@@ -1630,7 +1698,10 @@ Value *ReassociatePass::OptimizeAdd(Instruction *I,
if (!Duplicates.insert(Factor).second)
continue;
unsigned Occ = ++FactorOccurrences[Factor];
- if (Occ > MaxOcc) {
+ if (Occ > MaxOcc ||
+ (Occ == MaxOcc &&
+ (isa<Instruction>(Factor) || isa<Argument>(Factor)) &&
+ isa<Constant>(MaxOccVal) && !isa<UndefValue>(MaxOccVal))) {
MaxOcc = Occ;
MaxOccVal = Factor;
}
@@ -2197,6 +2268,15 @@ void ReassociatePass::OptimizeInst(Instruction *I) {
I = NI;
}
+ if (I->getOpcode() == Instruction::Mul && ShouldBreakUpDistribution(I)) {
+ Instruction *MulUser = cast<Instruction>(I->user_back());
+ BreakUpDistribute(I, RedoInsts);
+ RedoInsts.insert(I);
+ RedoInsts.insert(MulUser);
+ MadeChange = true;
+ return;
+ }
+
// If this is a subtract instruction which is not already in negate form,
// see if we can convert it to X+-Y.
if (I->getOpcode() == Instruction::Sub) {
diff --git a/llvm/test/Transforms/Reassociate/reassociate-distribute.ll b/llvm/test/Transforms/Reassociate/reassociate-distribute.ll
new file mode 100644
index 0000000000000..3ad8bc13e8364
--- /dev/null
+++ b/llvm/test/Transforms/Reassociate/reassociate-distribute.ll
@@ -0,0 +1,154 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=reassociate -S %s | FileCheck %s
+
+; ---- SHOULD transform ----
+
+; Basic case: a*8697 - (a+b)*6436 → a*2261 + b*(-6436)
+define i32 @test_basic(i32 %a, i32 %b) {
+; CHECK-LABEL: @test_basic(
+; CHECK-NEXT: [[MUL1_NEG:%.*]] = mul i32 [[B:%.*]], -6436
+; CHECK-NEXT: [[REASS_MUL:%.*]] = mul i32 [[A:%.*]], 2261
+; CHECK-NEXT: [[SUB:%.*]] = add i32 [[REASS_MUL]], [[MUL1_NEG]]
+; CHECK-NEXT: ret i32 [[SUB]]
+;
+ %mul = mul nsw i32 %a, 8697
+ %add = add nsw i32 %a, %b
+ %mul1 = mul nsw i32 %add, 6436
+ %sub = sub nsw i32 %mul, %mul1
+ ret i32 %sub
+}
+
+; Constant on left side of mul: a*100 + 3*(a+b)
+define i32 @test_const_on_left(i32 %a, i32 %b) {
+; CHECK-LABEL: @test_const_on_left(
+; CHECK-NEXT: [[MUL1:%.*]] = mul i32 [[B:%.*]], 7
+; CHECK-NEXT: [[REASS_MUL:%.*]] = mul i32 [[A:%.*]], 107
+; CHECK-NEXT: [[ADD2:%.*]] = add i32 [[MUL1]], [[REASS_MUL]]
+; CHECK-NEXT: ret i32 [[ADD2]]
+;
+ %mul = mul nsw i32 %a, 100
+ %add = add nsw i32 %a, %b
+ %mul1 = mul nsw i32 7, %add
+ %add2 = add nsw i32 %mul, %mul1
+ ret i32 %add2
+}
+
+; Sub instead of add inside: a*500 - (a-b)*300
+define i32 @test_with_sub(i32 %a, i32 %b) {
+; CHECK-LABEL: @test_with_sub(
+; CHECK-NEXT: [[MUL2_NEG_NEG:%.*]] = mul i32 [[B:%.*]], 300
+; CHECK-NEXT: [[REASS_MUL:%.*]] = mul i32 [[A:%.*]], 200
+; CHECK-NEXT: [[SUB:%.*]] = add i32 [[REASS_MUL]], [[MUL2_NEG_NEG]]
+; CHECK-NEXT: ret i32 [[SUB]]
+;
+ %mul = mul nsw i32 %a, 500
+ %sub1 = sub nsw i32 %a, %b
+ %mul1 = mul nsw i32 %sub1, 300
+ %sub = sub nsw i32 %mul, %mul1
+ ret i32 %sub
+}
+
+; b operand matches: b*11 + (a+b)*4
+define i32 @test_b_matches(i32 %a, i32 %b) {
+; CHECK-LABEL: @test_b_matches(
+; CHECK-NEXT: [[MUL2:%.*]] = mul i32 [[A:%.*]], 4
+; CHECK-NEXT: [[REASS_MUL:%.*]] = mul i32 [[B:%.*]], 15
+; CHECK-NEXT: [[ADD2:%.*]] = add i32 [[REASS_MUL]], [[MUL2]]
+; CHECK-NEXT: ret i32 [[ADD2]]
+;
+ %mul = mul nsw i32 %b, 11
+ %add = add nsw i32 %a, %b
+ %mul1 = mul nsw i32 %add, 4
+ %add2 = add nsw i32 %mul, %mul1
+ ret i32 %add2
+}
+
+; i16 type: x*10 + (x+y)*20
+define i16 @test_i16(i16 %x, i16 %y) {
+; CHECK-LABEL: @test_i16(
+; CHECK-NEXT: [[MUL1:%.*]] = mul i16 [[Y:%.*]], 20
+; CHECK-NEXT: [[REASS_MUL:%.*]] = mul i16 [[X:%.*]], 30
+; CHECK-NEXT: [[ADD2:%.*]] = add i16 [[MUL1]], [[REASS_MUL]]
+; CHECK-NEXT: ret i16 [[ADD2]]
+;
+ %mul1 = mul nsw i16 %x, 10
+ %add = add nsw i16 %x, %y
+ %mul2 = mul nsw i16 %add, 20
+ %add2 = add i16 %mul1, %mul2
+ ret i16 %add2
+}
+
+; NSW/NUW must be dropped on new instructions after distribution
+; (x+y) *nsw 20 does NOT imply (x *nsw 20) + (y *nsw 20)
+define i16 @test_nsw_dropped(i16 %x, i16 %y) {
+; CHECK-LABEL: @test_nsw_dropped(
+; CHECK-NEXT: [[MUL1:%.*]] = mul i16 [[Y:%.*]], 25
+; CHECK-NEXT: [[REASS_MUL:%.*]] = mul i16 [[X:%.*]], 40
+; CHECK-NEXT: [[ADD2:%.*]] = add i16 [[MUL1]], [[REASS_MUL]]
+; CHECK-NEXT: ret i16 [[ADD2]]
+;
+ %mul1 = mul nsw i16 %x, 15
+ %add = add nsw i16 %x, %y
+ %mul2 = mul nsw i16 %add, 25
+ %add2 = add i16 %mul1, %mul2
+ ret i16 %add2
+}
+
+; ---- Should NOT transform ----
+
+; No sibling constant mul — nothing to combine with
+define i32 @test_no_match(i32 %a, i32 %b) {
+; CHECK-LABEL: @test_no_match(
+; CHECK-NEXT: [[ADD:%.*]] = add nsw i32 [[B:%.*]], [[A:%.*]]
+; CHECK-NEXT: [[MUL:%.*]] = mul nsw i32 [[ADD]], 999
+; CHECK-NEXT: ret i32 [[MUL]]
+; CHECK-NOT: Mul1
+; CHECK-NOT: Mul2
+; CHECK-NOT: DistAdd
+; CHECK-NOT: DistSub
+;
+ %add = add nsw i32 %a, %b
+ %mul = mul nsw i32 %add, 999
+ ret i32 %mul
+}
+
+; AddSub has multiple uses — oneUse required for distribution
+define i32 @test_multi_use_add(i32 %a, i32 %b) {
+; CHECK-LABEL: @test_multi_use_add(
+; CHECK-NEXT: [[MUL1:%.*]] = mul i32 [[B:%.*]], -442
+; CHECK-NEXT: [[REASS_MUL:%.*]] = mul i32 [[A:%.*]], 335
+; CHECK-NEXT: [[RET:%.*]] = add i32 [[REASS_MUL]], [[MUL1]]
+; CHECK-NEXT: ret i32 [[RET]]
+; CHECK-NOT: Mul1
+; CHECK-NOT: Mul2
+; CHECK-NOT: DistAdd
+; CHECK-NOT: DistSub
+;
+ %mul = mul nsw i32 %a, 777
+ %add = add nsw i32 %a, %b
+ %mul1 = mul nsw i32 %add, 444
+ %mul2 = mul nsw i32 %add, 2
+ %sub = sub nsw i32 %mul, %mul1
+ %ret = add nsw i32 %sub, %mul2
+ ret i32 %ret
+}
+
+; Sibling mul has no constant operand — should not trigger
+define i32 @test_no_const_on_other_mul(i32 %a, i32 %b, i32 %c) {
+; CHECK-LABEL: @test_no_const_on_other_mul(
+; CHECK-NEXT: [[MUL:%.*]] = mul nsw i32 [[C:%.*]], [[A:%.*]]
+; CHECK-NEXT: [[ADD:%.*]] = add nsw i32 [[B:%.*]], [[A]]
+; CHECK-NEXT: [[MUL1:%.*]] = mul nsw i32 [[ADD]], 555
+; CHECK-NEXT: [[SUB:%.*]] = sub nsw i32 [[MUL]], [[MUL1]]
+; CHECK-NEXT: ret i32 [[SUB]]
+; CHECK-NOT: Mul1
+; CHECK-NOT: Mul2
+; CHECK-NOT: DistAdd
+; CHECK-NOT: DistSub
+;
+ %mul = mul nsw i32 %a, %c
+ %add = add nsw i32 %a, %b
+ %mul1 = mul nsw i32 %add, 555
+ %sub = sub nsw i32 %mul, %mul1
+ ret i32 %sub
+}
>From 4ef404f710946e613b93bca2ebe241a78f51e113 Mon Sep 17 00:00:00 2001
From: hazarathayya <hazarathayyayallanki at gmail.com>
Date: Wed, 29 Apr 2026 02:22:02 -0700
Subject: [PATCH 2/4] added sibling oneuse check, updated createmul, createadd
to remove check there itself, added to redo the new instructions
---
llvm/lib/Transforms/Scalar/Reassociate.cpp | 15 ++++++-----
.../Reassociate/reassociate-distribute.ll | 27 ++++++++++---------
2 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp
index 989387d704923..9f13bd41082fa 100644
--- a/llvm/lib/Transforms/Scalar/Reassociate.cpp
+++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp
@@ -983,11 +983,8 @@ static bool ShouldBreakUpDistribution(Instruction *Mul) {
MulUser->getOpcode() != Instruction::Sub))
return false;
- if (!MulUser)
- return false;
-
for (Value *Sibling : MulUser->operands()) {
- if (Sibling == Mul)
+ if (Sibling == Mul || !Sibling->hasOneUse())
continue;
// Sibling must be NonConst * C'.
@@ -1013,15 +1010,19 @@ static BinaryOperator *BreakUpDistribute(Instruction *Mul,
: C;
BinaryOperator *M1 =
- CreateMul(AddSub->getOperand(0), C, "Mul1", Mul->getIterator(), Mul);
+ CreateMul(AddSub->getOperand(0), C, "Mul1", Mul->getIterator(), nullptr);
BinaryOperator *M2 =
- CreateMul(AddSub->getOperand(1), C2, "Mul2", Mul->getIterator(), Mul);
+ CreateMul(AddSub->getOperand(1), C2, "Mul2", Mul->getIterator(), nullptr);
BinaryOperator *Result;
- Result = CreateAdd(M1, M2, "DistAdd", Mul->getIterator(), Mul);
+ Result = CreateAdd(M1, M2, "DistAdd", Mul->getIterator(), nullptr);
Mul->replaceAllUsesWith(Result);
Result->setDebugLoc(Mul->getDebugLoc());
+ ToRedo.insert(M1);
+ ToRedo.insert(M2);
+ ToRedo.insert(Result);
+
return Result;
}
diff --git a/llvm/test/Transforms/Reassociate/reassociate-distribute.ll b/llvm/test/Transforms/Reassociate/reassociate-distribute.ll
index 3ad8bc13e8364..b2057d168796c 100644
--- a/llvm/test/Transforms/Reassociate/reassociate-distribute.ll
+++ b/llvm/test/Transforms/Reassociate/reassociate-distribute.ll
@@ -94,6 +94,21 @@ define i16 @test_nsw_dropped(i16 %x, i16 %y) {
ret i16 %add2
}
+define i16 @test_surviving_poison(i16 %x, i16 %y) {
+; CHECK-LABEL: @test_surviving_poison(
+; CHECK-NEXT: [[MUL1:%.*]] = mul i16 [[Y:%.*]], 20
+; CHECK-NEXT: [[REASS_MUL:%.*]] = mul i16 [[X:%.*]], 30
+; CHECK-NEXT: [[ADD2:%.*]] = add i16 [[MUL1]], [[REASS_MUL]]
+; CHECK-NEXT: ret i16 [[ADD2]]
+;
+ %add = add nsw i16 %x, %y
+ %mul1 = mul nsw i16 %x, 10
+ ; This mul has 'nsw'. We distribute it to x*20 and y*20.
+ %mul2 = mul nsw i16 %add, 20
+ %add2 = add i16 %mul1, %mul2
+ ret i16 %add2
+}
+
; ---- Should NOT transform ----
; No sibling constant mul — nothing to combine with
@@ -102,10 +117,6 @@ define i32 @test_no_match(i32 %a, i32 %b) {
; CHECK-NEXT: [[ADD:%.*]] = add nsw i32 [[B:%.*]], [[A:%.*]]
; CHECK-NEXT: [[MUL:%.*]] = mul nsw i32 [[ADD]], 999
; CHECK-NEXT: ret i32 [[MUL]]
-; CHECK-NOT: Mul1
-; CHECK-NOT: Mul2
-; CHECK-NOT: DistAdd
-; CHECK-NOT: DistSub
;
%add = add nsw i32 %a, %b
%mul = mul nsw i32 %add, 999
@@ -119,10 +130,6 @@ define i32 @test_multi_use_add(i32 %a, i32 %b) {
; CHECK-NEXT: [[REASS_MUL:%.*]] = mul i32 [[A:%.*]], 335
; CHECK-NEXT: [[RET:%.*]] = add i32 [[REASS_MUL]], [[MUL1]]
; CHECK-NEXT: ret i32 [[RET]]
-; CHECK-NOT: Mul1
-; CHECK-NOT: Mul2
-; CHECK-NOT: DistAdd
-; CHECK-NOT: DistSub
;
%mul = mul nsw i32 %a, 777
%add = add nsw i32 %a, %b
@@ -141,10 +148,6 @@ define i32 @test_no_const_on_other_mul(i32 %a, i32 %b, i32 %c) {
; CHECK-NEXT: [[MUL1:%.*]] = mul nsw i32 [[ADD]], 555
; CHECK-NEXT: [[SUB:%.*]] = sub nsw i32 [[MUL]], [[MUL1]]
; CHECK-NEXT: ret i32 [[SUB]]
-; CHECK-NOT: Mul1
-; CHECK-NOT: Mul2
-; CHECK-NOT: DistAdd
-; CHECK-NOT: DistSub
;
%mul = mul nsw i32 %a, %c
%add = add nsw i32 %a, %b
>From 5ce52320966b94485ca38d3d533ed851b74489f9 Mon Sep 17 00:00:00 2001
From: hazarathayya <hazarathayyayallanki at gmail.com>
Date: Wed, 6 May 2026 07:44:52 -0700
Subject: [PATCH 3/4] added lambda helpers, used binary operator's creatmul and
createadd
---
llvm/lib/Transforms/Scalar/Reassociate.cpp | 35 +++++++++++-----------
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp
index 9f13bd41082fa..c881811922519 100644
--- a/llvm/lib/Transforms/Scalar/Reassociate.cpp
+++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp
@@ -974,9 +974,6 @@ static bool ShouldBreakUpDistribution(Instruction *Mul) {
m_ImmConstant())))
return false;
- if (!Mul->hasOneUse())
- return false;
-
auto *MulUser = dyn_cast<Instruction>(Mul->user_back());
// The parent MUST be an Add or Sub to ensure the tree is flattened
if (!MulUser || (MulUser->getOpcode() != Instruction::Add &&
@@ -1010,11 +1007,11 @@ static BinaryOperator *BreakUpDistribute(Instruction *Mul,
: C;
BinaryOperator *M1 =
- CreateMul(AddSub->getOperand(0), C, "Mul1", Mul->getIterator(), nullptr);
+ BinaryOperator::CreateMul(AddSub->getOperand(0), C, "Mul1", Mul->getIterator());
BinaryOperator *M2 =
- CreateMul(AddSub->getOperand(1), C2, "Mul2", Mul->getIterator(), nullptr);
+ BinaryOperator::CreateMul(AddSub->getOperand(1), C2, "Mul2", Mul->getIterator());
BinaryOperator *Result;
- Result = CreateAdd(M1, M2, "DistAdd", Mul->getIterator(), nullptr);
+ Result = BinaryOperator::CreateAdd(M1, M2, "DistAdd", Mul->getIterator());
Mul->replaceAllUsesWith(Result);
Result->setDebugLoc(Mul->getDebugLoc());
@@ -1648,6 +1645,17 @@ Value *ReassociatePass::OptimizeAdd(Instruction *I,
// where they are actually the same multiply.
unsigned MaxOcc = 0;
Value *MaxOccVal = nullptr;
+
+ // Prefer a non-constant factor over a constant when occurrence counts
+ // tie. Factoring out a variable (e.g., X from X*C1 + X*C2) exposes
+ // downstream constant folding; factoring out a constant does not.
+ auto IsBetterFactor = [](Value *Factor, Value *MaxOccVal, unsigned Occ,
+ unsigned MaxOcc) {
+ return Occ > MaxOcc ||
+ (Occ == MaxOcc &&
+ (isa<Instruction>(Factor) || isa<Argument>(Factor)) &&
+ isa<Constant>(MaxOccVal) && !isa<UndefValue>(MaxOccVal));
+ };
for (const ValueEntry &Op : Ops) {
BinaryOperator *BOp =
isReassociableOp(Op.Op, Instruction::Mul, Instruction::FMul);
@@ -1666,10 +1674,7 @@ Value *ReassociatePass::OptimizeAdd(Instruction *I,
continue;
unsigned Occ = ++FactorOccurrences[Factor];
- if (Occ > MaxOcc ||
- (Occ == MaxOcc &&
- (isa<Instruction>(Factor) || isa<Argument>(Factor)) &&
- isa<Constant>(MaxOccVal) && !isa<UndefValue>(MaxOccVal))) {
+ if (IsBetterFactor(Factor, MaxOccVal, Occ, MaxOcc)) {
MaxOcc = Occ;
MaxOccVal = Factor;
}
@@ -1683,10 +1688,7 @@ Value *ReassociatePass::OptimizeAdd(Instruction *I,
if (!Duplicates.insert(Factor).second)
continue;
unsigned Occ = ++FactorOccurrences[Factor];
- if (Occ > MaxOcc ||
- (Occ == MaxOcc &&
- (isa<Instruction>(Factor) || isa<Argument>(Factor)) &&
- isa<Constant>(MaxOccVal) && !isa<UndefValue>(MaxOccVal))) {
+ if (IsBetterFactor(Factor, MaxOccVal, Occ, MaxOcc)) {
MaxOcc = Occ;
MaxOccVal = Factor;
}
@@ -1699,10 +1701,7 @@ Value *ReassociatePass::OptimizeAdd(Instruction *I,
if (!Duplicates.insert(Factor).second)
continue;
unsigned Occ = ++FactorOccurrences[Factor];
- if (Occ > MaxOcc ||
- (Occ == MaxOcc &&
- (isa<Instruction>(Factor) || isa<Argument>(Factor)) &&
- isa<Constant>(MaxOccVal) && !isa<UndefValue>(MaxOccVal))) {
+ if (IsBetterFactor(Factor, MaxOccVal, Occ, MaxOcc)) {
MaxOcc = Occ;
MaxOccVal = Factor;
}
>From 9be43d151a805ab8383fb310d9a9b8bdc8837395 Mon Sep 17 00:00:00 2001
From: Hazarath <92698778+hazarathayya at users.noreply.github.com>
Date: Wed, 6 May 2026 20:41:28 +0530
Subject: [PATCH 4/4] Apply suggestions from code review
Co-authored-by: Yingwei Zheng <dtcxzyw at qq.com>
---
llvm/lib/Transforms/Scalar/Reassociate.cpp | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp
index c881811922519..e23d1f73c06b5 100644
--- a/llvm/lib/Transforms/Scalar/Reassociate.cpp
+++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp
@@ -986,10 +986,9 @@ static bool ShouldBreakUpDistribution(Instruction *Mul) {
// Sibling must be NonConst * C'.
Value *SibNC;
- if (match(Sibling, m_Mul(m_Value(SibNC), m_Constant()))) {
- if ((SibNC == A || SibNC == B) && !isa<Constant>(SibNC))
- return true;
- }
+ if (match(Sibling, m_Mul(m_Value(SibNC), m_Constant())) && (SibNC == A || SibNC == B) && !isa<Constant>(SibNC))
+ return true;
+
}
return false;
}
@@ -1002,7 +1001,7 @@ static BinaryOperator *BreakUpDistribute(Instruction *Mul,
Instruction *AddSub = cast<Instruction>(Mul->getOperand(0));
Constant *C = cast<Constant>(Mul->getOperand(1));
- Constant *C2 = (AddSub->getOpcode() == Instruction::Sub)
+ Constant *C2 = AddSub->getOpcode() == Instruction::Sub
? cast<Constant>(ConstantExpr::getNeg(C))
: C;
More information about the llvm-commits
mailing list