Skip to content

Conversation

@arsenm
Copy link
Contributor

@arsenm arsenm commented Dec 10, 2025

nsz can only change the behavior of the sign bit.
The sign bit for fmul can be implemented as xor,
which is associative. DAGCombiner already reassociates
the multiply by 2 constants without nsz.

Fixes #64967

@arsenm arsenm added floating-point Floating-point math llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes labels Dec 10, 2025 — with Graphite App
@arsenm arsenm marked this pull request as ready for review December 10, 2025 23:01
@llvmbot
Copy link
Member

llvmbot commented Dec 10, 2025

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: Matt Arsenault (arsenm)

Changes

nsz can only change the behavior of the sign bit.
The sign bit for fmul can be implemented as xor,
which is associative. DAGCombiner already reassociates
the multiply by 2 constants without nsz.

Fixes #64967


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

4 Files Affected:

  • (modified) llvm/lib/IR/Instruction.cpp (+1)
  • (modified) llvm/test/Transforms/InstCombine/2006-10-26-VectorReassoc.ll (+2-4)
  • (modified) llvm/test/Transforms/InstCombine/fdiv.ll (+1-2)
  • (modified) llvm/test/Transforms/InstCombine/issue64967-reassoc-fmul.ll (+6-12)
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 33ca46ca1c2c6..b95c1466871bc 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -1271,6 +1271,7 @@ bool Instruction::isAssociative() const {
 
   switch (Opcode) {
   case FMul:
+    return cast<FPMathOperator>(this)->hasAllowReassoc();
   case FAdd:
     return cast<FPMathOperator>(this)->hasAllowReassoc() &&
            cast<FPMathOperator>(this)->hasNoSignedZeros();
diff --git a/llvm/test/Transforms/InstCombine/2006-10-26-VectorReassoc.ll b/llvm/test/Transforms/InstCombine/2006-10-26-VectorReassoc.ll
index fb860a5e7bdf3..6509797e0d3dc 100644
--- a/llvm/test/Transforms/InstCombine/2006-10-26-VectorReassoc.ll
+++ b/llvm/test/Transforms/InstCombine/2006-10-26-VectorReassoc.ll
@@ -35,12 +35,10 @@ define <4 x float> @test_fmul_reassoc_nsz(<4 x float> %V) {
 }
 
 ; (V * C1) * C2 => V * (C1 * C2)
-; TODO: This doesn't require 'nsz'.  It should fold to V * { 1.0, 4.0e+05, -9.0, 16.0 }
 define <4 x float> @test_fmul_reassoc(<4 x float> %V) {
 ; CHECK-LABEL: @test_fmul_reassoc(
-; CHECK-NEXT:     [[TMP1:%.*]] = fmul reassoc <4 x float> [[V:%.*]], <float 1.000000e+00, float 2.000000e+00, float 3.000000e+00, float 4.000000e+00>
-; CHECK-NEXT:     [[TMP2:%.*]] = fmul reassoc <4 x float> [[TMP1]], <float 1.000000e+00, float 2.000000e+05, float -3.000000e+00, float 4.000000e+00>
-; CHECK-NEXT:     ret <4 x float> [[TMP2]]
+; CHECK: [[TMP1:%.*]] = fmul reassoc <4 x float> %V, <float 1.000000e+00, float 4.000000e+05, float -9.000000e+00, float 1.600000e+01>
+; CHECK-NEXT:     ret <4 x float> [[TMP1]]
         %Y = fmul reassoc <4 x float> %V, < float 1.000000e+00, float 2.000000e+00, float 3.000000e+00, float 4.000000e+00 >
         %Z = fmul reassoc <4 x float> %Y, < float 1.000000e+00, float 2.000000e+05, float -3.000000e+00, float 4.000000e+00 >
         ret <4 x float> %Z
diff --git a/llvm/test/Transforms/InstCombine/fdiv.ll b/llvm/test/Transforms/InstCombine/fdiv.ll
index 54b0bf8c50ac7..3465781e3af9d 100644
--- a/llvm/test/Transforms/InstCombine/fdiv.ll
+++ b/llvm/test/Transforms/InstCombine/fdiv.ll
@@ -525,8 +525,7 @@ define <2 x float> @div_constant_dividend2_reassoc_only(<2 x float> %x) {
 
 define <2 x float> @div_constant_dividend3(<2 x float> %x) {
 ; CHECK-LABEL: @div_constant_dividend3(
-; CHECK-NEXT:    [[TMP1:%.*]] = fmul reassoc arcp <2 x float> [[X:%.*]], <float 1.500000e+01, float -7.000000e+00>
-; CHECK-NEXT:    [[T2:%.*]] = fmul reassoc arcp <2 x float> [[TMP1]], <float 0x3FD5555560000000, float 0x3FC24924A0000000>
+; CHECK-NEXT:    [[T2:%.*]] = fmul reassoc arcp <2 x float> [[X:%.*]], <float 5.000000e+00, float -1.000000e+00>
 ; CHECK-NEXT:    ret <2 x float> [[T2]]
 ;
   %t1 = fdiv <2 x float> <float 3.0e0, float 7.0e0>, %x
diff --git a/llvm/test/Transforms/InstCombine/issue64967-reassoc-fmul.ll b/llvm/test/Transforms/InstCombine/issue64967-reassoc-fmul.ll
index 16f9cf2dd64c5..5d064234bf609 100644
--- a/llvm/test/Transforms/InstCombine/issue64967-reassoc-fmul.ll
+++ b/llvm/test/Transforms/InstCombine/issue64967-reassoc-fmul.ll
@@ -25,8 +25,7 @@ define float @fmul(float %x) {
 define float @fmul_reassoc(float %x) {
 ; CHECK-LABEL: define float @fmul_reassoc(
 ; CHECK-SAME: float [[X:%.*]]) {
-; CHECK-NEXT:    [[FMUL0:%.*]] = fmul reassoc float [[X]], 2.000000e+00
-; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc float [[FMUL0]], 4.000000e+00
+; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc float [[X]], 8.000000e+00
 ; CHECK-NEXT:    ret float [[FMUL1]]
 ;
   %fmul0 = fmul reassoc float %x, 2.0
@@ -37,8 +36,7 @@ define float @fmul_reassoc(float %x) {
 define <2 x float> @fmul_reassoc_v2(<2 x float> %x) {
 ; CHECK-LABEL: define <2 x float> @fmul_reassoc_v2(
 ; CHECK-SAME: <2 x float> [[X:%.*]]) {
-; CHECK-NEXT:    [[FMUL0:%.*]] = fmul reassoc <2 x float> [[X]], splat (float 2.000000e+00)
-; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc <2 x float> [[FMUL0]], splat (float 4.000000e+00)
+; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc <2 x float> [[X]], splat (float 8.000000e+00)
 ; CHECK-NEXT:    ret <2 x float> [[FMUL1]]
 ;
   %fmul0 = fmul reassoc <2 x float> %x, splat (float 2.0)
@@ -54,8 +52,7 @@ define <2 x float> @fmul_reassoc_v2(<2 x float> %x) {
 define float @fmul_reassoc_negative_0(float %x) {
 ; CHECK-LABEL: define float @fmul_reassoc_negative_0(
 ; CHECK-SAME: float [[X:%.*]]) {
-; CHECK-NEXT:    [[FMUL0:%.*]] = fmul reassoc float [[X]], 2.000000e+00
-; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc float [[FMUL0]], -4.000000e+00
+; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc float [[X]], -8.000000e+00
 ; CHECK-NEXT:    ret float [[FMUL1]]
 ;
   %fmul0 = fmul reassoc float %x, 2.0
@@ -71,8 +68,7 @@ define float @fmul_reassoc_negative_0(float %x) {
 define float @fmul_reassoc_negative_1(float %x) {
 ; CHECK-LABEL: define float @fmul_reassoc_negative_1(
 ; CHECK-SAME: float [[X:%.*]]) {
-; CHECK-NEXT:    [[FMUL0:%.*]] = fmul reassoc float [[X]], -2.000000e+00
-; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc float [[FMUL0]], 4.000000e+00
+; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc float [[X]], -8.000000e+00
 ; CHECK-NEXT:    ret float [[FMUL1]]
 ;
   %fmul0 = fmul reassoc float %x, -2.0
@@ -95,8 +91,7 @@ define float @fmul_reassoc_nsz(float %x) {
 define float @fmul_reassoc_posk_neg0(float %x) {
 ; CHECK-LABEL: define float @fmul_reassoc_posk_neg0(
 ; CHECK-SAME: float [[X:%.*]]) {
-; CHECK-NEXT:    [[FMUL0:%.*]] = fmul reassoc float [[X]], 4.000000e+00
-; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc float [[FMUL0]], -0.000000e+00
+; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc float [[X]], -0.000000e+00
 ; CHECK-NEXT:    ret float [[FMUL1]]
 ;
   %fmul0 = fmul reassoc float %x, 4.0
@@ -108,8 +103,7 @@ define float @fmul_reassoc_neg0_posk(float %x) {
 ; CHECK-LABEL: define float @fmul_reassoc_neg0_posk(
 ; CHECK-SAME: float [[X:%.*]]) {
 ; CHECK-NEXT:    [[FMUL0:%.*]] = fmul reassoc float [[X]], -0.000000e+00
-; CHECK-NEXT:    [[FMUL1:%.*]] = fmul reassoc float [[FMUL0]], 4.000000e+00
-; CHECK-NEXT:    ret float [[FMUL1]]
+; CHECK-NEXT:    ret float [[FMUL0]]
 ;
   %fmul0 = fmul reassoc float %x, -0.0
   %fmul1 = fmul reassoc float %fmul0, 4.0

Copy link
Contributor

@andykaylor andykaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good, but I have a concern about the prior state.

; CHECK-LABEL: @div_constant_dividend3(
; CHECK-NEXT: [[TMP1:%.*]] = fmul reassoc arcp <2 x float> [[X:%.*]], <float 1.500000e+01, float -7.000000e+00>
; CHECK-NEXT: [[T2:%.*]] = fmul reassoc arcp <2 x float> [[TMP1]], <float 0x3FD5555560000000, float 0x3FC24924A0000000>
; CHECK-NEXT: [[T2:%.*]] = fmul reassoc arcp <2 x float> [[X:%.*]], <float 5.000000e+00, float -1.000000e+00>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one doesn't look right, but not because of the change in this PR. Shouldn't we have required 1reassocandrcpon bothfdivinstructions before converting them into twofmulinstructions withressocandarcp` on both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Many places still treat reassoc as viral

Base automatically changed from users/arsenm/issue64697/instcombine-add-baseline-test-reassoc-fmul to main December 11, 2025 10:37
nsz can only change the behavior of the sign bit.
The sign bit for fmul can be implemented as xor,
which is associative. DAGCombiner already reassociates
the multiply by 2 constants without nsz.

Fixes #64967
@arsenm arsenm force-pushed the users/arsenm/issue64697/allow-reassoc-fmul-no-nsz branch from a7f8f32 to 966cb03 Compare December 11, 2025 10:41
@arsenm arsenm enabled auto-merge (squash) December 11, 2025 10:47
@arsenm arsenm merged commit 481ce81 into main Dec 11, 2025
9 of 10 checks passed
@arsenm arsenm deleted the users/arsenm/issue64697/allow-reassoc-fmul-no-nsz branch December 11, 2025 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

floating-point Floating-point math llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:ir llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reassociatable fmul by constants unnecessarily requires nsz

4 participants