-
-
Notifications
You must be signed in to change notification settings - Fork 129
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
feat: implement acosd intrinsic function #3804
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment to be addressed before this PR becomes ready to merge. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left comments above, everything but test looks good.
@Pranavchiku I applied one of your suggestions. It was about the utilization of M_PI from math.h library. I checked its value and found that it was less accurate. Its value was 3.14159. Previously, I was using the value of PI as 3.14159265358979323846. So what do you think which one should I use? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is missing compile time tests, left comments above.
I am sorry I resolved conflicts via github UI which resulted into Merge commit and hence will have to squash and merge this PR, I hope this is fine. |
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good, thanks!
No description provided.