Skip to content

Simplify sinh(x) / cosh(x) -> tanh(x) with fast-math #78871

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
k-arrows opened this issue Jan 21, 2024 · 6 comments
Open

Simplify sinh(x) / cosh(x) -> tanh(x) with fast-math #78871

k-arrows opened this issue Jan 21, 2024 · 6 comments

Comments

@k-arrows
Copy link

k-arrows commented Jan 21, 2024

It seems that the simplification sin(x) / cos(x) -> tan(x) has been already implemented, but the same applies for hyperbolic function(s).

https://godbolt.org/z/WqYfKxr7K

For those who are interested in floating-point arithmetic optimization, I think this can be added to #34959.

@felixkellenbenz
Copy link
Contributor

Hey is anyone working on this ? If not could someone assign this to me ?

@dc03
Copy link
Member

dc03 commented Jan 25, 2024

No one's replied till now (5 days since issue filing), so I'll assume no one has picked this up.

@arsenm
Copy link
Contributor

arsenm commented Jan 25, 2024

#77799 also related

@felixkellenbenz
Copy link
Contributor

felixkellenbenz commented Jan 25, 2024

Thanks for assigning me, I will look into this as soon as possible

@felixkellenbenz
Copy link
Contributor

I am almost done with this but there is one thing I can't quite figure out. I used replaceInstUsesWith to replace the div Instruction with a call Instruction to tanh. This does not replace the calls to sinh and cosh but when looking at the optimization of sin/cos it seems to replace the calls to sin and cos, why is this and how can I delete the calls to sinh and cosh ?
Any help would be appreciated.

@arsenm
Copy link
Contributor

arsenm commented Feb 5, 2024

it seems to replace the calls tosinandcos, why is this and how can I delete the calls to sinhandcosh` ? Any help would be appreciated.

The replaceInstUsesWith only replaces the uses, leaving behind the dead call instructions. You need to separately erase them

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

No branches or pull requests

6 participants