Skip to content
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

Fix sqrt. #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix sqrt. #16

wants to merge 1 commit into from

Conversation

sodero
Copy link

@sodero sodero commented Feb 2, 2021

When testing libstdc++ sqrt gave incorrect results. Negative arguments should give a NaN return value. Refer to this.

@sba1
Copy link
Contributor

sba1 commented Feb 3, 2021

This is unfortuntaelly not unique, e.g., see

But as I read it out, according to IEEE 754 it should behave like the proposed patch.

More importantly, if the change will be accepted I guess all the other math function should be checked, e.g., against setting the errno value. I didn't know that math functions could have such side effects (which also prevents optimization BTW).

@sodero
Copy link
Author

sodero commented Feb 3, 2021

This is unfortuntaelly not unique, e.g., see

But as I read it out, according to IEEE 754 it should behave like the proposed patch.

More importantly, if the change will be accepted I guess all the other math function should be checked, e.g., against setting the errno value. I didn't know that math functions could have such side effects (which also prevents optimization BTW).

Yes, I wrote the test cases using glibc and whatever c lib MacOS is using (I assume that it's some BSD derivative) and they both return NaN.

As far as errno is concerned we could look into what other libs do. I don't think musl for example sets errno but I need to double check that. I would be nice to not having to implement any side effects.

Update: glibc sets EDOM just like clib2 does right now so I updated the patch to just change the return value to NAN when the input is < 0 and leave the rest as it is.

@sodero
Copy link
Author

sodero commented Feb 7, 2021

More importantly, if the change will be accepted I guess all the other math function should be checked, e.g., against setting the errno value. I didn't know that math functions could have such side effects (which also prevents optimization BTW).

Did some more digging and there are quite a few functions like sqrt, with return values / errno codes that can be discussed.

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

Successfully merging this pull request may close these issues.

2 participants