[Bug go/63269] libgo/math test failures in TestLog2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63269 Eric Gallager changed: What|Removed |Added Status|WAITING |RESOLVED URL||https://github.com/golang/g ||o/issues/9066 CC||egallager at gcc dot gnu.org Resolution|--- |MOVED --- Comment #7 from Eric Gallager --- (In reply to Dominik Vogt from comment #6) > (In reply to Ian Lance Taylor from comment #3) > > First, let me say that this code is in the Go master library and must be > > fixed there. It might be more effective to discuss it on the Go issue > > tracker at http://golang.org/issue. > > --> https://code.google.com/p/go/issues/detail?id=9066 Markings as MOVED upstream then
[Bug go/63269] libgo/math test failures in TestLog2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63269 --- Comment #6 from Dominik Vogt --- (In reply to Ian Lance Taylor from comment #3) > First, let me say that this code is in the Go master library and must be > fixed there. It might be more effective to discuss it on the Go issue > tracker at http://golang.org/issue. --> https://code.google.com/p/go/issues/detail?id=9066
[Bug go/63269] libgo/math test failures in TestLog2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63269 --- Comment #5 from Dominik Vogt --- regarding 1) My earlier explanation of the problem was wrong. Multiply and add is not generated; it probably only was in the artificial test case that I made and certainly did not compile with -ffp-contract=off. In this calculation in log2(), Log(frac)*(1/Ln2) + float64(exp) Gcc does constant folding for (1/Ln2) and generates a multiply instruction and then adds the second term. Same result if you write "*Log2E" instead of "*(1/Ln2)"). But with Log(frac)/Ln2 + float64(exp) it generates a divide instruction. The multiplication and the division yield results that differ in the least significant bit, and I don't see how this could be prevented in general; it's just an artifact of the floating point format. I've verified that the constants Ln2, 1/Ln2 and Log2E are bit correct. The "easy" way to fix this is increasing the allowed tolerance as my patch does (note that the arguments of the veryclose() call need to be swapped, see previous comment). The "right" way to fix this is to calculate platform specific ULPs for all the algorithms from the math library and use these. That's what glibc does.
[Bug go/63269] libgo/math test failures in TestLog2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63269
--- Comment #4 from Dominik Vogt ---
regarding 2)
> I'm not entirely persuaded by your argument for item 2. ...
Hm, good that you doubted it, because the actual mistake is somehwere else:
The unpatched code has
if l != float64(i)
but if you want to use a tolerance here this must become
if !veryclose(float64(i), l) {
With the argument reversed. This could/should be cleaned up by renaming the
arguments of the tolerance() function, e.g. a -> expected, b -> result, e ->
maxerr.
> Zero is a special
> value. When we expect a zero, we should get a zero, not something close to
> zero. I don't think this change is correct in general. It may be correct for
> some specific cases, but then we need to investigate those.
Actually, this has nothing to do with 0 being special here, abut with scaling
of the allowed error: Multiplying it by 0 yields zero error tolerance, so the
tolerance() function does not do that.
--> This chunk is not necessary, but a (separate) cleanup patch might help to
avoid future confusion.
[Bug go/63269] libgo/math test failures in TestLog2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63269 Ian Lance Taylor changed: What|Removed |Added Status|UNCONFIRMED |WAITING Last reconfirmed||2014-11-05 Ever confirmed|0 |1 --- Comment #3 from Ian Lance Taylor --- First, let me say that this code is in the Go master library and must be fixed there. It might be more effective to discuss it on the Go issue tracker at http://golang.org/issue. I don't agree with your argument for item 1. You say that the wrong result happens when using a fused multiply-add instruction. The math library is compiled with -ffp-contract=off (see MATH_FLAG in configure.ac and Makefile.am), so the compiler should not be generating a fused multiply-add instruction. I'm not entirely persuaded by your argument for item 2. Zero is a special value. When we expect a zero, we should get a zero, not something close to zero. I don't think this change is correct in general. It may be correct for some specific cases, but then we need to investigate those. Item 3 is the same sort of thing: when we expect zero, we should, in general, get exactly zero.
[Bug go/63269] libgo/math test failures in TestLog2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63269 --- Comment #2 from Dominik Vogt --- Created attachment 33506 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33506&action=edit updated patch Managed to get a stray character into the test between testing and committing it. Fixed.
[Bug go/63269] libgo/math test failures in TestLog2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63269 --- Comment #1 from Dominik Vogt --- Created attachment 33493 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33493&action=edit Proposed fixes
