Skip to content
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

[InstCombine] Resolve TODO: nnan nsz X / -0.0 -> copysign(inf, X) #79766

Merged
merged 2 commits into from
Feb 7, 2024

Conversation

AtariDreams
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 28, 2024

@llvm/pr-subscribers-llvm-transforms

Author: AtariDreams (AtariDreams)

Changes

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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp (+11-1)
  • (modified) llvm/test/Transforms/InstCombine/fdiv.ll (+19)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
index 6c3adf00c189a8e..3f2ec4eb6e94bec 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -1600,7 +1600,17 @@ Instruction *InstCombinerImpl::foldFDivConstantDivisor(BinaryOperator &I) {
   // nnan X / +0.0 -> copysign(inf, X)
   if (I.hasNoNaNs() && match(I.getOperand(1), m_Zero())) {
     IRBuilder<> B(&I);
-    // TODO: nnan nsz X / -0.0 -> copysign(inf, X)
+    CallInst *CopySign = B.CreateIntrinsic(
+        Intrinsic::copysign, {C->getType()},
+        {ConstantFP::getInfinity(I.getType()), I.getOperand(0)}, &I);
+    CopySign->takeName(&I);
+    return replaceInstUsesWith(I, CopySign);
+  } else if (I.hasNoNaNs() && I.hasNoSignedZeros() &&
+             match(I.getOperand(1),
+                   m_Specific(ConstantFP::get(Type::getDoubleTy(I.getContext()),
+                                              -0.0)))) {
+    // Handle nnan nsz X / -0.0 -> copysign(inf, X)
+    IRBuilder<> B(&I);
     CallInst *CopySign = B.CreateIntrinsic(
         Intrinsic::copysign, {C->getType()},
         {ConstantFP::getInfinity(I.getType()), I.getOperand(0)}, &I);
diff --git a/llvm/test/Transforms/InstCombine/fdiv.ll b/llvm/test/Transforms/InstCombine/fdiv.ll
index c81a9ba6d4215d9..e0aeb6840223337 100644
--- a/llvm/test/Transforms/InstCombine/fdiv.ll
+++ b/llvm/test/Transforms/InstCombine/fdiv.ll
@@ -992,3 +992,22 @@ define float @fdiv_nnan_neg_zero_f32(float %x) {
   %fdiv = fdiv nnan float %x, -0.0
   ret float %fdiv
 }
+
+define double @test_positive_zero(double %X) {
+; CHECK-LABEL: @test_positive_zero(
+; CHECK-NEXT:    [[TMP1:%.*]] = call nnan nsz double @llvm.copysign.f64(double 0x7FF0000000000000, double [[X:%.*]])
+; CHECK-NEXT:    ret double [[TMP1]]
+;
+  %1 = fdiv nnan nsz double %X, 0.0
+  ret double %1
+}
+
+define double @test_negative_zero(double %X) {
+; CHECK-LABEL: @test_negative_zero(
+; CHECK-NEXT:    [[TMP1:%.*]] = call nnan nsz double @llvm.copysign.f64(double 0x7FF0000000000000, double [[X:%.*]])
+; CHECK-NEXT:    ret double [[TMP1]]
+;
+  %1 = fdiv nnan nsz double %X, -0.0
+  ret double %1
+}
+

@nikic nikic requested review from arsenm and jcranmer-intel and removed request for nikic January 28, 2024 20:19
@AtariDreams
Copy link
Contributor Author

@arsenm All issues addressed!

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

lgtm with some additional edge case handling

llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp Outdated Show resolved Hide resolved
llvm/test/Transforms/InstCombine/fdiv.ll Show resolved Hide resolved
@AtariDreams
Copy link
Contributor Author

@dtcxzyw Fixed!

@arsenm arsenm merged commit 966f78b into llvm:main Feb 7, 2024
4 checks passed
@AtariDreams AtariDreams deleted the float branch February 7, 2024 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants