Skip to content

Conversation

VedantParanjape
Copy link
Contributor

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.

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.
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Sep 12, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 12, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Vedant Paranjape (VedantParanjape)

Changes

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.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp (-10)
  • (modified) llvm/test/Transforms/InstCombine/fold-fadd-with-zero-gh154238.ll (-11)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 726d09aa26941..00951fde0cf8a 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -3155,16 +3155,6 @@ Instruction *InstCombinerImpl::visitFSub(BinaryOperator &I) {
   Value *X, *Y;
   Constant *C;
 
-  // B = fsub A, 0.0
-  // Z = Op B
-  // can be transformed into
-  // Z = Op A
-  // Where Op is such that we can ignore sign of 0 in fsub
-  Value *A;
-  if (match(&I, m_OneUse(m_FSub(m_Value(A), m_AnyZeroFP()))) &&
-      canIgnoreSignBitOfZero(*I.use_begin()))
-    return replaceInstUsesWith(I, A);
-
   Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
   // If Op0 is not -0.0 or we can ignore -0.0: Z - (X - Y) --> Z + (Y - X)
   // Canonicalize to fadd to make analysis easier.
diff --git a/llvm/test/Transforms/InstCombine/fold-fadd-with-zero-gh154238.ll b/llvm/test/Transforms/InstCombine/fold-fadd-with-zero-gh154238.ll
index b9d951dc2945a..f9f0ca8a08bcb 100644
--- a/llvm/test/Transforms/InstCombine/fold-fadd-with-zero-gh154238.ll
+++ b/llvm/test/Transforms/InstCombine/fold-fadd-with-zero-gh154238.ll
@@ -24,14 +24,3 @@ define float @src2(float %arg1) {
   %v4 = fsub float %v2, %v3
   ret float %v4
 }
-
-define float @src_sub(float %arg1) {
-; CHECK-LABEL: define float @src_sub(
-; CHECK-SAME: float [[ARG1:%.*]]) {
-; CHECK-NEXT:    [[V3:%.*]] = call float @llvm.fabs.f32(float [[ARG1]])
-; CHECK-NEXT:    ret float [[V3]]
-;
-  %v2 = fsub float %arg1, 0.000000e+00
-  %v3 = call float @llvm.fabs.f32(float %v2)
-  ret float %v3
-}

@VedantParanjape VedantParanjape removed the request for review from dtcxzyw September 12, 2025 16:09
@VedantParanjape VedantParanjape merged commit c45aa5c into llvm:main Sep 12, 2025
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants