-
Notifications
You must be signed in to change notification settings - Fork 140
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
rem_pio2 computed incorrectly on i486 #57
Comments
From that Julia issue, it sounds like you're on a rather old machine, On 32-bit Windows the output looks like: https://gist.github.com/tkelman/c208e27a1fda765a5fa7 |
Yeah, it's just an old machine we keep around in my lab for light tasks :) Here's the output: https://gist.github.com/waldyrious/73eb0ddca80a14737724 |
This might be useful. While trying out TextPlots.jl on the same machine, I came across this bug and realized more exactly where the error lies. Essentially, Here's a screenshot using the system's libm: And here's a screenshot using openlibm: (note: this is using a freshly updated master, but using 0.2 gives the same results) |
Very helpful, @Waldir. Nice find. |
@ViralBShah you need an old 486 |
I am guessing this is because of some of the code in the |
I updated the title to make it clear what are the reproducibility conditions. |
If true, it should be reproducible on newer platforms by setting the appropriate compiler flags |
Just throwing a possible idea out here. When I was last messing with assembly in relation to Visual Studio, I found it defaulted to compatibility 16-bit mode unless you added in a @ViralBShah can you compare the output of |
What I have done so far is build with make |
One immediate thing for now may be to disallow building on very old machines, which are not being tested. |
All x86 targets use the code in i387. So you might have to pass -march in CFLAGS to force it to emit different asm. |
I verified that the i387 code got used, but I did not get the erroneous result. |
I can reproduce this:
I'm using clang so that I don't have to install a gcc with i487 support. It seems that clang includes support for all platforms (even windows cross-compile!), whereas gcc only includes support for the current platform (i686 in my case). edit: increasing the resolution in the graph reveals that the result is a square wave (rounded from correct result), outside of the -2pi to 2pi range |
I was under the impression that we use s_sin.S and s_cos.S from |
I am relatively certain the issue lies in the rem_pio2 argument reduction |
You are probably right. |
Other |
|
Some old discussion on rem_pio2: http://lists.freebsd.org/pipermail/freebsd-i386/2005-February/002101.html |
Related to JuliaLang/julia#7185 ? |
No - that one seems like a different issue - perhaps just a tolerance thing. |
I wonder if this gets fixed by changing the optimization level, using |
Using clang-3.3, the bug is present at all optimization levels and without optimizations too. |
What are the conditions to reproduce this problem? In JuliaLang/julia#7363 (comment) I seem to have experienced this with a Fedora build VM under RHEL6, so the hardware is not old, though the compiler (gcc 4.4.7) is. |
It seems that this happens in any 32-bit environment - as I have verified with ubuntu 32-bit images on my laptop with recent hardware. |
Well, the RPM nightlies as of today pass all of the tests on 32-bit, including the So it seems to me that this bug only happens with old enough software (apparently Ubuntu 12.04 comes with gcc 4.6.3). |
This is interesting and helps narrow it down. I wonder if there is a gcc flag we can use for older versions of gcc. |
OK, the bug disappeared when building openlibm/openspecfun with gcc 4.8.2 on RHEL6 |
I wonder if we can close this by requiring gcc 4.8 as the minimum. |
Originally reported at JuliaLang/julia#4850
sin()
appears to be returning the wrong value:However, if I use the system libm, the tests pass:
The text was updated successfully, but these errors were encountered: