Skip to content

Missed optimization in math expression: sin(asin(a)) == a #34951

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

Open
zamazan4ik opened this issue Dec 10, 2017 · 6 comments
Open

Missed optimization in math expression: sin(asin(a)) == a #34951

zamazan4ik opened this issue Dec 10, 2017 · 6 comments
Labels
bugzilla Issues migrated from bugzilla floating-point Floating-point math llvm:codegen

Comments

@zamazan4ik
Copy link

zamazan4ik commented Dec 10, 2017

Bugzilla Link 35603
Version trunk
OS All
Blocks #34959
CC @hfinkel,@rotateright

Extended Description

clang(trunk) with '--std=c++17 -O3 -march=native -ffast-math' flags for this code:

#include <cmath>

double test(double a)
{
    return sin(asin(a));
}

generates this assembly:

test(double): # @test(double)
  push rax
  call __asin_finite
  pop rax
  jmp sin # TAILCALL

But sin(asin(a)) == a. So there is no reason to call anything.

@zamazan4ik
Copy link
Author

The same issue about cos(acos(x)).

@zamazan4ik
Copy link
Author

@hfinkel
Copy link
Collaborator

hfinkel commented Dec 10, 2017

Is this right? sin is periodic, so there's some range reduction here. Maybe this is true only if we know the value is between 0 and 2*pi?

@llvmbot
Copy link
Member

llvmbot commented Dec 18, 2017

sin(asin(a)) = a requires: -1 <= a <= 1. Implementing this requires introducing additional cmp instructions, so the question would it actually be better?

@hfinkel
Copy link
Collaborator

hfinkel commented Dec 19, 2017

sin(asin(a)) = a requires: -1 <= a <= 1. Implementing this requires
introducing additional cmp instructions, so the question would it actually
be better?

It could be. asin is likely non-trivial to compute. In any case, -ffast-math implies no NaNs, and asin returns NaN outside of [-1:1], so sin(asin(a)) = a seems like a fine fast-math optimization.

@RKSimon
Copy link
Collaborator

RKSimon commented Nov 27, 2021

mentioned in issue #34959

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@arsenm arsenm added the floating-point Floating-point math label Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla floating-point Floating-point math llvm:codegen
Projects
None yet
Development

No branches or pull requests

5 participants