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

Incorrect value of sin(13.11250158914107) on x87 #243

Closed
eduardosm opened this issue May 20, 2020 · 6 comments
Closed

Incorrect value of sin(13.11250158914107) on x87 #243

eduardosm opened this issue May 20, 2020 · 6 comments

Comments

@eduardosm
Copy link

On x87, sin(13.11250158914107) returns 0.00003381429460243744, when the correct value is 0.51938488614424543.

The following command can be used to build for x87:

RUSTFLAGS="-C target-cpu=pentium" cargo build --target i686-unknown-linux-gnu
@eduardosm
Copy link
Author

It looks like the issue is in the floor function.

@plugwash
Copy link

plugwash commented Jan 2, 2021

I'm also seeing issues with the floor and ceiling functions in the Debian package of this library on i386 (Debian i386 uses x87).

Taking a read of the source, I suspect that the issue is that calculations are being performed with excess precision and this is breaking some of the tricks used in the floor/ceiling implementations. Specifically I suspect that the following construction is not producing the correct result.

/* y = int(x) - x, where int(x) is an integer neighbor of x */
let y = if (ui >> 63) != 0 {
    x - TOINT + TOINT - x
} else {
    x + TOINT - TOINT - x
};

Doing some reading I can't find a clear statement either way on whether such excess precision should be considered a bug in rustc or not.

I suspect the pragmatic way to fix this would be to reimplement this function with a different approach, i,e, converting to an integer and back, then adjusing as nessacery as proposed in #219

@plugwash
Copy link

plugwash commented Jan 2, 2021

I just whipped up a patch which made cargo test pass on x87, it's not particularly pretty though.

floor, ceil, round, roundf and rem_pio2f (but not floorf or ceilf) use an add/sub trick to round stuff to the nearest integer. Unfortunately this seems to break due to excess precision on x87.

For floor and ceil I made the functions use alternative implementations based on the proposal at #219

For the other functions I added a conversion to/from bits, this seems to force rounding, though I don't know for sure how robust that soloution is.

Finally I loosened the test slightly for j1f

I've pushed my changes at https://github.com/plugwash/libm if anyone wants to comment on them or clean them up.

@Lokathor
Copy link
Contributor

Lokathor commented Jan 2, 2021

opening it as a pr would allow for an easier review process, but i won't have time to look until Sunday or Monday

@plugwash
Copy link

plugwash commented Jan 3, 2021

PR filed at #249

@tgross35
Copy link
Contributor

I believe this was fixed by #249.

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

No branches or pull requests

4 participants