[InstCombine] Enable FAdd simplifications when user can ignore sign bit #157757

VedantParanjape · 2025-09-09T22:03:45Z

When FAdd result is used by fabs, we can safely ignore the sign bit of fp zero. This patch enables an instruction simplification optimization that folds fadd x, 0 ==> x, which would otherwise not work as the compiler cannot prove that the zero isn't -0. But if the result of the fadd is used by fabs we can simply ignore this and still do the optimization.

Fixes #154238

llvmbot · 2025-09-09T22:04:20Z

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-llvm-analysis

Author: Vedant Paranjape (VedantParanjape)

Changes

When FAdd result is used by fabs, we can safely ignore the sign bit of fp zero. This patch enables an instruction simplification optimization that folds fadd x, 0 ==> x, which would otherwise not work as the compiler cannot prove that the zero isn't -0. But if the result of the fadd is used by fabs we can simply ignore this and still do the optimization.

Fixes #154238

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

2 Files Affected:

(modified) llvm/lib/Analysis/InstructionSimplify.cpp (+3-1)
(added) llvm/test/Transforms/InstSimplify/fold-fadd-with-zero-gh154238.ll (+12)

diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index ebe329aa1d5fe..7f555c24f71a8 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -5723,7 +5723,9 @@ simplifyFAddInst(Value *Op0, Value *Op1, FastMathFlags FMF,
   // fadd X, 0 ==> X, when we know X is not -0
   if (canIgnoreSNaN(ExBehavior, FMF))
     if (match(Op1, m_PosZeroFP()) &&
-        (FMF.noSignedZeros() || cannotBeNegativeZero(Op0, Q)))
+        (FMF.noSignedZeros() || cannotBeNegativeZero(Op0, Q) ||
+         (Q.CxtI && !Q.CxtI->use_empty() &&
+          canIgnoreSignBitOfZero(*(Q.CxtI->use_begin())))))
       return Op0;
 
   if (!isDefaultFPEnvironment(ExBehavior, Rounding))
diff --git a/llvm/test/Transforms/InstSimplify/fold-fadd-with-zero-gh154238.ll b/llvm/test/Transforms/InstSimplify/fold-fadd-with-zero-gh154238.ll
new file mode 100644
index 0000000000000..bb12328574dda
--- /dev/null
+++ b/llvm/test/Transforms/InstSimplify/fold-fadd-with-zero-gh154238.ll
@@ -0,0 +1,12 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=instsimplify -S | FileCheck %s
+define float @src(float %arg1) {
+; CHECK-LABEL: define float @src(
+; CHECK-SAME: float [[ARG1:%.*]]) {
+; CHECK-NEXT:    [[V3:%.*]] = call float @llvm.fabs.f32(float [[ARG1]])
+; CHECK-NEXT:    ret float [[V3]]
+;
+  %v2 = fadd float %arg1, 0.000000e+00
+  %v3 = call float @llvm.fabs.f32(float %v2)
+  ret float %v3
+}

llvmbot · 2025-09-09T22:04:21Z

@llvm/pr-subscribers-llvm-transforms

Author: Vedant Paranjape (VedantParanjape)

Changes

When FAdd result is used by fabs, we can safely ignore the sign bit of fp zero. This patch enables an instruction simplification optimization that folds fadd x, 0 ==> x, which would otherwise not work as the compiler cannot prove that the zero isn't -0. But if the result of the fadd is used by fabs we can simply ignore this and still do the optimization.

Fixes #154238

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

2 Files Affected:

(modified) llvm/lib/Analysis/InstructionSimplify.cpp (+3-1)
(added) llvm/test/Transforms/InstSimplify/fold-fadd-with-zero-gh154238.ll (+12)

diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index ebe329aa1d5fe..7f555c24f71a8 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -5723,7 +5723,9 @@ simplifyFAddInst(Value *Op0, Value *Op1, FastMathFlags FMF,
   // fadd X, 0 ==> X, when we know X is not -0
   if (canIgnoreSNaN(ExBehavior, FMF))
     if (match(Op1, m_PosZeroFP()) &&
-        (FMF.noSignedZeros() || cannotBeNegativeZero(Op0, Q)))
+        (FMF.noSignedZeros() || cannotBeNegativeZero(Op0, Q) ||
+         (Q.CxtI && !Q.CxtI->use_empty() &&
+          canIgnoreSignBitOfZero(*(Q.CxtI->use_begin())))))
       return Op0;
 
   if (!isDefaultFPEnvironment(ExBehavior, Rounding))
diff --git a/llvm/test/Transforms/InstSimplify/fold-fadd-with-zero-gh154238.ll b/llvm/test/Transforms/InstSimplify/fold-fadd-with-zero-gh154238.ll
new file mode 100644
index 0000000000000..bb12328574dda
--- /dev/null
+++ b/llvm/test/Transforms/InstSimplify/fold-fadd-with-zero-gh154238.ll
@@ -0,0 +1,12 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=instsimplify -S | FileCheck %s
+define float @src(float %arg1) {
+; CHECK-LABEL: define float @src(
+; CHECK-SAME: float [[ARG1:%.*]]) {
+; CHECK-NEXT:    [[V3:%.*]] = call float @llvm.fabs.f32(float [[ARG1]])
+; CHECK-NEXT:    ret float [[V3]]
+;
+  %v2 = fadd float %arg1, 0.000000e+00
+  %v3 = call float @llvm.fabs.f32(float %v2)
+  ret float %v3
+}

VedantParanjape · 2025-09-09T23:16:07Z

I reviewed the failing test case (CodeGen/AMDGPU/fcanonicalize-elimination.ll) test_fold_canonicalize_fabs_value_f32. Since fadd X, 0 = X after the optimization, AMD backend generates v_mul_f32_e64 instead of v_and_b32_e32 for fabs. I can fix this in testcase, but doesn't this look like the ISel is wrong? mul is costlier than an and usually, is this not applicable for AMDGPU?

When FAdd result is used by fabs, we can safely ignore the sign bit of fp zero. This patch enables an instruction simplification optimization that folds fadd x, 0 ==> x, which would otherwise not work as the compiler cannot prove that the zero isn't -0. But if the result of the fadd is used by fabs we can simply ignore this and still do the optimization. Fixes llvm#154238

VedantParanjape · 2025-09-10T05:07:52Z

I reviewed the failing test case (CodeGen/AMDGPU/fcanonicalize-elimination.ll) test_fold_canonicalize_fabs_value_f32. Since fadd X, 0 = X after the optimization, AMD backend generates v_mul_f32_e64 instead of v_and_b32_e32 for fabs. I can fix this in testcase, but doesn't this look like the ISel is wrong? mul is costlier than an and usually, is this not applicable for AMDGPU?

It seems on older arch it emits a vmax, and vmul on the newer ones. It does so to make sure fmath flags are copied over correctly.

nikic · 2025-09-10T08:12:55Z

llvm/lib/Analysis/InstructionSimplify.cpp

-        (FMF.noSignedZeros() || cannotBeNegativeZero(Op0, Q)))
+        (FMF.noSignedZeros() || cannotBeNegativeZero(Op0, Q) ||
+         (Q.CxtI && !Q.CxtI->use_empty() &&
+          canIgnoreSignBitOfZero(*(Q.CxtI->use_begin())))))


You can't do use-based reasoning inside InstSimplify, this must happen in InstCombine.

VedantParanjape · 2025-09-10T16:25:31Z

Made the changes as proposed, also changed canIgnoreSignBitOfNaN for uniformity.

dtcxzyw · 2025-09-10T16:30:06Z

Made the changes as proposed, also changed canIgnoreSignBitOfNaN for uniformity.

This reverts the previous change asked by #141015 (comment) :)
You should put your code in InstCombine instead of InstSimplify, if it checks the users.

VedantParanjape · 2025-09-10T17:45:12Z

Made the changes as proposed, also changed canIgnoreSignBitOfNaN for uniformity.

This reverts the previous change asked by #141015 (comment) :) You should put your code in InstCombine instead of InstSimplify, if it checks the users.

Okay makes sense, as in move the complete optimization to InstCombine

VedantParanjape · 2025-09-10T18:34:29Z

@dtcxzyw made the changes.

llvm/test/Transforms/InstCombine/fold-fadd-with-zero-gh154238.ll

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

VedantParanjape · 2025-09-11T21:57:11Z

@arsenm tracking here for which call operators this can be implemented. Operators whose identity element is zero.

visitFAdd
visitFSub

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

dtcxzyw · 2025-09-12T02:29:37Z

@zyw-bot mfuzz

Co-authored-by: Yingwei Zheng <dtcxzyw@qq.com>

dtcxzyw

LG

nikic · 2025-09-12T08:35:31Z

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

+  Value *A;
+  if (match(&I, m_OneUse(m_FSub(m_Value(A), m_AnyZeroFP()))) &&
+      canIgnoreSignBitOfZero(*I.use_begin()))
+    return replaceInstUsesWith(I, A);


This fold doesn't make sense, as fsub x, C will be canonicalized to fadd x, -C. The test case you added makes even less sense because it uses fsub x, 0.0 aka fadd x, -0.0 which is always a no-op and does not actually depend on the use-based logic.

Please remove this code again.

Since FSub X, 0 gets canoncialised to FAdd X, -0 the said optimization didn't make much sense for FSub. Remove it from IC and the adjoined testcase.

VedantParanjape requested a review from nikic as a code owner September 9, 2025 22:03

llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Sep 9, 2025

VedantParanjape requested a review from dtcxzyw September 9, 2025 22:04

VedantParanjape force-pushed the main branch from 0e7d005 to b230ed7 Compare September 10, 2025 04:51

llvmbot added the backend:AMDGPU label Sep 10, 2025

nikic reviewed Sep 10, 2025

View reviewed changes

nikic added the floating-point Floating-point math label Sep 10, 2025

moved opt to IC from InstSimplify

3aaf155

VedantParanjape force-pushed the main branch from f6e3f21 to 3aaf155 Compare September 10, 2025 18:20

revert fcanonicalize testcase

717c2c8

arsenm reviewed Sep 11, 2025

View reviewed changes

llvm/test/Transforms/InstCombine/fold-fadd-with-zero-gh154238.ll Show resolved Hide resolved

VedantParanjape changed the title ~~[InstSimplify] Enable FAdd simplifications when user can ignore sign bit~~ [InstCombine] Enable FAdd simplifications when user can ignore sign bit Sep 11, 2025

nikic reviewed Sep 11, 2025

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp Outdated Show resolved Hide resolved

rework to check for a single use, and update the testcase

486bbe7

Add support for FSub as well

9ff60ee

dtcxzyw reviewed Sep 12, 2025

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp Outdated Show resolved Hide resolved

zyw-bot mentioned this pull request Sep 12, 2025

Fuzz PR157757 dtcxzyw/llvm-mutation-based-fuzz-service#100

Closed

dtcxzyw mentioned this pull request Sep 12, 2025

Task submission dtcxzyw/llvm-opt-benchmark#1312

Open

zyw-bot mentioned this pull request Sep 12, 2025

pre-commit: PR157757 dtcxzyw/llvm-opt-benchmark#2805

Closed

Update llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

1dd1c85

Co-authored-by: Yingwei Zheng <dtcxzyw@qq.com>

dtcxzyw approved these changes Sep 12, 2025

View reviewed changes

VedantParanjape merged commit 092de9b into llvm:main Sep 12, 2025
9 checks passed

nikic reviewed Sep 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[InstCombine] Enable FAdd simplifications when user can ignore sign bit #157757

[InstCombine] Enable FAdd simplifications when user can ignore sign bit #157757

Uh oh!

VedantParanjape commented Sep 9, 2025

Uh oh!

llvmbot commented Sep 9, 2025 •

edited

Loading

Uh oh!

llvmbot commented Sep 9, 2025

Uh oh!

VedantParanjape commented Sep 9, 2025

Uh oh!

VedantParanjape commented Sep 10, 2025

Uh oh!

nikic Sep 10, 2025

Uh oh!

VedantParanjape commented Sep 10, 2025

Uh oh!

dtcxzyw commented Sep 10, 2025

Uh oh!

VedantParanjape commented Sep 10, 2025

Uh oh!

VedantParanjape commented Sep 10, 2025

Uh oh!

Uh oh!

Uh oh!

VedantParanjape commented Sep 11, 2025 •

edited

Loading

Uh oh!

Uh oh!

dtcxzyw commented Sep 12, 2025

Uh oh!

dtcxzyw left a comment

Uh oh!

Uh oh!

nikic Sep 12, 2025

Uh oh!

Uh oh!

[InstCombine] Enable FAdd simplifications when user can ignore sign bit #157757

[InstCombine] Enable FAdd simplifications when user can ignore sign bit #157757

Uh oh!

Conversation

VedantParanjape commented Sep 9, 2025

Uh oh!

llvmbot commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 9, 2025

Uh oh!

VedantParanjape commented Sep 9, 2025

Uh oh!

VedantParanjape commented Sep 10, 2025

Uh oh!

nikic Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

VedantParanjape commented Sep 10, 2025

Uh oh!

dtcxzyw commented Sep 10, 2025

Uh oh!

VedantParanjape commented Sep 10, 2025

Uh oh!

VedantParanjape commented Sep 10, 2025

Uh oh!

Uh oh!

Uh oh!

VedantParanjape commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dtcxzyw commented Sep 12, 2025

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nikic Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvmbot commented Sep 9, 2025 •

edited

Loading

VedantParanjape commented Sep 11, 2025 •

edited

Loading