[ConstantFold] Special case atan +/-0.0

[lei137]

C's Annex F specifies that atan +/-0.0 returns the input value;
however, this behavior is optional and host C libraries may behave
differently. This change applies the Annex F behavior to constant
folding by LLVM.

Ref: https://pubs.opengroup.org/onlinepubs/9799919799/functions/atan.html

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: Lei Huang (lei137)

Changes

C's Annex F specifies that atan +/-0.0 returns the input value;
however, this behavior is optional and host C libraries may behave
differently. This change applies the Annex F behavior to constant
folding by LLVM.

Ref: https://pubs.opengroup.org/onlinepubs/9799919799/functions/atan.html

---

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

2 Files Affected:

• (modified) llvm/lib/Analysis/ConstantFolding.cpp (+3)
• (modified) llvm/test/Transforms/InstSimplify/ConstProp/atan-intrinsic.ll (-1)

diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp
index 139a0b81e299b..6a54c7355d70a 100644
--- a/llvm/lib/Analysis/ConstantFolding.cpp
+++ b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -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.
+        if (U.isZero())
+          return ConstantFP::get(Ty->getContext(), U);
         return ConstantFoldFP(atan, APF, Ty);
       case Intrinsic::sqrt:
         return ConstantFoldFP(sqrt, APF, Ty);
diff --git a/llvm/test/Transforms/InstSimplify/ConstProp/atan-intrinsic.ll b/llvm/test/Transforms/InstSimplify/ConstProp/atan-intrinsic.ll
index c5c17d65524c2..d824d6d35643d 100644
--- a/llvm/test/Transforms/InstSimplify/ConstProp/atan-intrinsic.ll
+++ b/llvm/test/Transforms/InstSimplify/ConstProp/atan-intrinsic.ll
@@ -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() {

[hubert-reinterpretcast]

LGTM; thanks!

[topperc]

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.

[topperc]

Specifically this code in the same file

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

[lei137]

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.

[topperc]

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.

[lei137]

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

[topperc]

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
+}

[lei137]

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