Skip to content

[ConstantFold] Special case atan +/-0.0 #143962

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.

Sign up for GitHub

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 3 commits into from
Jun 25, 2025
Merged

[ConstantFold] Special case atan +/-0.0 #143962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1bb408a
Revert "[PowerPC][AIX] xfail atan-intrinsic to unblock bot (#143723)"
lei137 Jun 11, 2025
139bf5b
[ConstantFold] Special case atan +/-0.0
lei137 Jun 12, 2025
0a64bc5
add test and handling for atan lib calls
lei137 Jun 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions llvm/lib/Analysis/ConstantFolding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2549,6 +2549,9 @@ static Constant *ConstantFoldScalarCall1(StringRef Name,
case Intrinsic::cosh:
return ConstantFoldFP(cosh, APF, Ty);
case Intrinsic::atan:
// Implement optional behavior from C's Annex F for +/-0.0.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume it's still wrong on the scalar libcall that's used by clang without -ffast-math, but we just don't have test for it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Specifically this code in the same file

    case LibFunc_atan:                                                           
    case LibFunc_atanf:                                                          
      if (TLI->has(Func))                                                        
        return ConstantFoldFP(atan, APF, Ty);                                    
      break;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It think there are multiple places this will need to be added and we need to look into a better way to fix then just add for individual cases as it shows up. I can add it there if you like, but there are no tests for it currently. This PR is just to provide a fix to #143416 which broke the aix bot.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what we should do. If we aren't going to fix LibFunc_atan then an equally valid fix for the current issue is to change the test case in #143416 to use a different constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fix is more valid then changing the test case IMHO.
Would you like to provide a separate test case for LibFunc_atan?

Copy link
Collaborator

Choose a reason for hiding this comment

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

diff --git a/llvm/test/Transforms/InstSimplify/ConstProp/calls.ll b/llvm/test/Transforms/InstSimplify/ConstProp/calls.ll
index 61a30c781c0f..f4320dc518aa 100644
--- a/llvm/test/Transforms/InstSimplify/ConstProp/calls.ll
+++ b/llvm/test/Transforms/InstSimplify/ConstProp/calls.ll
@@ -204,3 +204,15 @@ entry:
 
 declare double @llvm.pow.f64(double, double) nounwind readonly
 declare float @llvm.pow.f32(float, float) nounwind readonly
+
+define float @test_atan_negzero() {
+; CHECK-LABEL: define float @test_atan_negzero() {
+; CHECK-NEXT:    ret float -0.000000e+00
+;
+; FNOBUILTIN-LABEL: define float @test_atan_negzero() {
+; FNOBUILTIN-NEXT:    [[TMP1:%.*]] = call float @atanf(float -0.000000e+00)
+; FNOBUILTIN-NEXT:    ret float [[TMP1]]
+;
+  %1 = call float @atanf(float -0.0)
+  ret float %1
+}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @topperc! I've updated the PR.

if (U.isZero())
return ConstantFP::get(Ty->getContext(), U);
return ConstantFoldFP(atan, APF, Ty);
case Intrinsic::sqrt:
return ConstantFoldFP(sqrt, APF, Ty);
Expand Down Expand Up @@ -2602,6 +2605,9 @@ static Constant *ConstantFoldScalarCall1(StringRef Name,
break;
case LibFunc_atan:
case LibFunc_atanf:
// Implement optional behavior from C's Annex F for +/-0.0.
if (U.isZero())
return ConstantFP::get(Ty->getContext(), U);
if (TLI->has(Func))
return ConstantFoldFP(atan, APF, Ty);
break;
Expand Down
1 change: 0 additions & 1 deletion llvm/test/Transforms/InstSimplify/ConstProp/atan-intrinsic.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt -S -passes=instsimplify < %s | FileCheck %s
; XFAIL: target={{.*}}-aix{{.*}}

define double @test_atan_0() {
; CHECK-LABEL: define double @test_atan_0() {
Expand Down
12 changes: 12 additions & 0 deletions llvm/test/Transforms/InstSimplify/ConstProp/calls.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,5 +202,17 @@ entry:
ret float %0
}

define float @test_atan_negzero() nounwind uwtable ssp {
entry:
; CHECK-LABEL: @test_atan_negzero(
; CHECK: ret float -0.000000e+00
;
; FNOBUILTIN-LABEL: @test_atan_negzero(
; FNOBUILTIN: ret float -0.000000e+00
;
%1 = call float @atanf(float -0.0)
ret float %1
}

declare double @llvm.pow.f64(double, double) nounwind readonly
declare float @llvm.pow.f32(float, float) nounwind readonly
Loading