Skip to content

release/20.x: [InstCombine] Preserve signbit semantics of NaN with fold to fabs (#136648) #137608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 29, 2025

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Apr 28, 2025

Backport 3e1e406

Requested by: @dtcxzyw

@llvmbot
Copy link
Member Author

llvmbot commented Apr 28, 2025

@arsenm What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Apr 28, 2025

@llvm/pr-subscribers-llvm-transforms

Author: None (llvmbot)

Changes

Backport 3e1e406

Requested by: @dtcxzyw


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp (+8-1)
  • (modified) llvm/test/Transforms/InstCombine/fabs.ll (+46-11)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 29c5cef84ccdb..9cd234dd3babf 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -2838,7 +2838,14 @@ static Instruction *foldSelectWithFCmpToFabs(SelectInst &SI,
 
     // fold (X <= +/-0.0) ? (0.0 - X) : X to fabs(X), when 'Swap' is false
     // fold (X >  +/-0.0) ? X : (0.0 - X) to fabs(X), when 'Swap' is true
-    if (match(TrueVal, m_FSub(m_PosZeroFP(), m_Specific(X)))) {
+    // Note: We require "nnan" for this fold because fcmp ignores the signbit
+    //       of NAN, but IEEE-754 specifies the signbit of NAN values with
+    //       fneg/fabs operations.
+    if (match(TrueVal, m_FSub(m_PosZeroFP(), m_Specific(X))) &&
+        (cast<FPMathOperator>(CondVal)->hasNoNaNs() || SI.hasNoNaNs() ||
+         isKnownNeverNaN(X, /*Depth=*/0,
+                         IC.getSimplifyQuery().getWithInstruction(
+                             cast<Instruction>(CondVal))))) {
       if (!Swap && (Pred == FCmpInst::FCMP_OLE || Pred == FCmpInst::FCMP_ULE)) {
         Value *Fabs = IC.Builder.CreateUnaryIntrinsic(Intrinsic::fabs, X, &SI);
         return IC.replaceInstUsesWith(SI, Fabs);
diff --git a/llvm/test/Transforms/InstCombine/fabs.ll b/llvm/test/Transforms/InstCombine/fabs.ll
index 7b9a672f188ca..f449d4b8e6b37 100644
--- a/llvm/test/Transforms/InstCombine/fabs.ll
+++ b/llvm/test/Transforms/InstCombine/fabs.ll
@@ -256,6 +256,19 @@ define double @select_fcmp_ole_zero(double %x) {
 ; CHECK-LABEL: @select_fcmp_ole_zero(
 ; CHECK-NEXT:    [[FABS:%.*]] = call double @llvm.fabs.f64(double [[X:%.*]])
 ; CHECK-NEXT:    ret double [[FABS]]
+;
+  %lezero = fcmp nnan ole double %x, 0.0
+  %negx = fsub double 0.0, %x
+  %fabs = select i1 %lezero, double %negx, double %x
+  ret double %fabs
+}
+
+define double @select_fcmp_ole_zero_no_nnan(double %x) {
+; CHECK-LABEL: @select_fcmp_ole_zero_no_nnan(
+; CHECK-NEXT:    [[LEZERO:%.*]] = fcmp ole double [[X:%.*]], 0.000000e+00
+; CHECK-NEXT:    [[NEGX:%.*]] = fsub double 0.000000e+00, [[X]]
+; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[LEZERO]], double [[NEGX]], double [[X]]
+; CHECK-NEXT:    ret double [[FABS]]
 ;
   %lezero = fcmp ole double %x, 0.0
   %negx = fsub double 0.0, %x
@@ -263,12 +276,34 @@ define double @select_fcmp_ole_zero(double %x) {
   ret double %fabs
 }
 
+define double @select_fcmp_ole_zero_no_nnan_input_nofpclass_nan(double nofpclass(nan) %x) {
+; CHECK-LABEL: @select_fcmp_ole_zero_no_nnan_input_nofpclass_nan(
+; CHECK-NEXT:    [[FABS:%.*]] = call double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    ret double [[FABS]]
+;
+  %lezero = fcmp ole double %x, 0.0
+  %negx = fsub double 0.0, %x
+  %fabs = select i1 %lezero, double %negx, double %x
+  ret double %fabs
+}
+
+define double @select_fcmp_ole_zero_select_nnan(double %x) {
+; CHECK-LABEL: @select_fcmp_ole_zero_select_nnan(
+; CHECK-NEXT:    [[FABS:%.*]] = call nnan double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    ret double [[FABS]]
+;
+  %lezero = fcmp ole double %x, 0.0
+  %negx = fsub double 0.0, %x
+  %fabs = select nnan i1 %lezero, double %negx, double %x
+  ret double %fabs
+}
+
 define double @select_fcmp_nnan_ole_zero(double %x) {
 ; CHECK-LABEL: @select_fcmp_nnan_ole_zero(
 ; CHECK-NEXT:    [[FABS:%.*]] = call double @llvm.fabs.f64(double [[X:%.*]])
 ; CHECK-NEXT:    ret double [[FABS]]
 ;
-  %lezero = fcmp ole double %x, 0.0
+  %lezero = fcmp nnan ole double %x, 0.0
   %negx = fsub nnan double 0.0, %x
   %fabs = select i1 %lezero, double %negx, double %x
   ret double %fabs
@@ -279,7 +314,7 @@ define double @select_nnan_fcmp_nnan_ole_zero(double %x) {
 ; CHECK-NEXT:    [[FABS:%.*]] = call nnan double @llvm.fabs.f64(double [[X:%.*]])
 ; CHECK-NEXT:    ret double [[FABS]]
 ;
-  %lezero = fcmp ole double %x, 0.0
+  %lezero = fcmp nnan ole double %x, 0.0
   %negx = fsub nnan double 0.0, %x
   %fabs = select nnan i1 %lezero, double %negx, double %x
   ret double %fabs
@@ -292,7 +327,7 @@ define double @select_fcmp_nnan_ule_zero(double %x) {
 ; CHECK-NEXT:    [[FABS:%.*]] = call double @llvm.fabs.f64(double [[X:%.*]])
 ; CHECK-NEXT:    ret double [[FABS]]
 ;
-  %lezero = fcmp ule double %x, 0.0
+  %lezero = fcmp nnan ule double %x, 0.0
   %negx = fsub nnan double 0.0, %x
   %fabs = select i1 %lezero, double %negx, double %x
   ret double %fabs
@@ -320,7 +355,7 @@ define <2 x float> @select_fcmp_nnan_ole_negzero(<2 x float> %x) {
 ; CHECK-NEXT:    [[FABS:%.*]] = call <2 x float> @llvm.fabs.v2f32(<2 x float> [[X:%.*]])
 ; CHECK-NEXT:    ret <2 x float> [[FABS]]
 ;
-  %lezero = fcmp ole <2 x float> %x, <float -0.0, float -0.0>
+  %lezero = fcmp nnan ole <2 x float> %x, <float -0.0, float -0.0>
   %negx = fsub nnan <2 x float> <float 0.0, float poison>, %x
   %fabs = select <2 x i1> %lezero, <2 x float> %negx, <2 x float> %x
   ret <2 x float> %fabs
@@ -331,7 +366,7 @@ define <2 x float> @select_nnan_fcmp_nnan_ole_negzero(<2 x float> %x) {
 ; CHECK-NEXT:    [[FABS:%.*]] = call nnan <2 x float> @llvm.fabs.v2f32(<2 x float> [[X:%.*]])
 ; CHECK-NEXT:    ret <2 x float> [[FABS]]
 ;
-  %lezero = fcmp ole <2 x float> %x, <float -0.0, float -0.0>
+  %lezero = fcmp nnan ole <2 x float> %x, <float -0.0, float -0.0>
   %negx = fsub nnan <2 x float> <float 0.0, float poison>, %x
   %fabs = select nnan <2 x i1> %lezero, <2 x float> %negx, <2 x float> %x
   ret <2 x float> %fabs
@@ -344,7 +379,7 @@ define fp128 @select_fcmp_ogt_zero(fp128 %x) {
 ; CHECK-NEXT:    [[FABS:%.*]] = call fp128 @llvm.fabs.f128(fp128 [[X:%.*]])
 ; CHECK-NEXT:    ret fp128 [[FABS]]
 ;
-  %gtzero = fcmp ogt fp128 %x, zeroinitializer
+  %gtzero = fcmp nnan ogt fp128 %x, zeroinitializer
   %negx = fsub fp128 zeroinitializer, %x
   %fabs = select i1 %gtzero, fp128 %x, fp128 %negx
   ret fp128 %fabs
@@ -382,7 +417,7 @@ define fp128 @select_fcmp_nnan_ogt_zero(fp128 %x) {
 ; CHECK-NEXT:    [[FABS:%.*]] = call fp128 @llvm.fabs.f128(fp128 [[X:%.*]])
 ; CHECK-NEXT:    ret fp128 [[FABS]]
 ;
-  %gtzero = fcmp ogt fp128 %x, zeroinitializer
+  %gtzero = fcmp nnan ogt fp128 %x, zeroinitializer
   %negx = fsub nnan fp128 zeroinitializer, %x
   %fabs = select i1 %gtzero, fp128 %x, fp128 %negx
   ret fp128 %fabs
@@ -393,7 +428,7 @@ define fp128 @select_nnan_fcmp_nnan_ogt_zero(fp128 %x) {
 ; CHECK-NEXT:    [[FABS:%.*]] = call nnan fp128 @llvm.fabs.f128(fp128 [[X:%.*]])
 ; CHECK-NEXT:    ret fp128 [[FABS]]
 ;
-  %gtzero = fcmp ogt fp128 %x, zeroinitializer
+  %gtzero = fcmp nnan ogt fp128 %x, zeroinitializer
   %negx = fsub nnan fp128 zeroinitializer, %x
   %fabs = select nnan i1 %gtzero, fp128 %x, fp128 %negx
   ret fp128 %fabs
@@ -406,7 +441,7 @@ define half @select_fcmp_nnan_ogt_negzero(half %x) {
 ; CHECK-NEXT:    [[FABS:%.*]] = call half @llvm.fabs.f16(half [[X:%.*]])
 ; CHECK-NEXT:    ret half [[FABS]]
 ;
-  %gtzero = fcmp ogt half %x, -0.0
+  %gtzero = fcmp nnan ogt half %x, -0.0
   %negx = fsub nnan half 0.0, %x
   %fabs = select i1 %gtzero, half %x, half %negx
   ret half %fabs
@@ -417,7 +452,7 @@ define half @select_nnan_fcmp_nnan_ogt_negzero(half %x) {
 ; CHECK-NEXT:    [[FABS:%.*]] = call nnan half @llvm.fabs.f16(half [[X:%.*]])
 ; CHECK-NEXT:    ret half [[FABS]]
 ;
-  %gtzero = fcmp ogt half %x, -0.0
+  %gtzero = fcmp nnan ogt half %x, -0.0
   %negx = fsub nnan half 0.0, %x
   %fabs = select nnan i1 %gtzero, half %x, half %negx
   ret half %fabs
@@ -430,7 +465,7 @@ define half @select_fcmp_nnan_ugt_negzero(half %x) {
 ; CHECK-NEXT:    [[FABS:%.*]] = call half @llvm.fabs.f16(half [[X:%.*]])
 ; CHECK-NEXT:    ret half [[FABS]]
 ;
-  %gtzero = fcmp ugt half %x, -0.0
+  %gtzero = fcmp nnan ugt half %x, -0.0
   %negx = fsub nnan half 0.0, %x
   %fabs = select i1 %gtzero, half %x, half %negx
   ret half %fabs

@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Merge in LLVM Release Status Apr 29, 2025
…vm#136648)

As per the LangRef and IEEE 754-2008 standard, the sign bit of NaN is
preserved if there is no floating-point operation being performed.
See also
llvm@862e35e
for reference.

Alive2: https://alive2.llvm.org/ce/z/QYtEGj
Closes llvm#136646

(cherry picked from commit 3e1e406)
@tstellar tstellar merged commit f4779c3 into llvm:release/20.x Apr 29, 2025
7 of 10 checks passed
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Apr 29, 2025
Copy link

@dtcxzyw (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants