You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is a copy of the bug report from the gcc bugtracker. See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63269 for the mentioned patch.
--
Dominik Vogt 2014-09-15 14:10:37 UTC
------------------------------------
There are several flaws in the math library test code. On s390x, the test TestLog2
fails for three reasons:
1) This part of the test is too strict for i == 0:
for i := -1074; i <= 1023; i++ {
f := Ldexp(1, i)
l := Log2(f)
if l != float64(i) {
t.Errorf("Log2(2**%d) = %g, want %d", i, l, i)
}
}
It should really use the veryclose function() as there is no guarantee that the result
of this calculation from log10.go is exact:
return Log(frac)*(1/Ln2) + float64(exp)
With frac == 0.5 and exp == 1, the result may be just close to 0, not exactly zero:
* If the compiler generated two instructions, one to mulitply and one to add, the result
is (with a small error delta):
Product: Log(0.5)*(1/Ln2) = -1 + delta -> rounded to -1
Sum: -1 + 1 = 0 -> rounded to 0
Result: 0
* If the compiler generates a single multiply and add instruction, only the final result
is rouced, i.e.
Product: Log(0.5)*(1/Ln2) = -1 + delta (not rounded)
Sum: -1 + delta + 1 = delta -> rounded to delta'
Result: delta' != 0
So, the fix is to use the "veryclose" error tolerance.
2) There's a bug in the tolerance() function:
if a != 0 {
e = e * a
if e < 0 {
e = -e
}
}
This snippet uses the defective value a but should use the expected value b instead:
if b != 0 {
e = e * b
if e < 0 {
e = -e
}
}
Otherwise, bad things happen for an expected value 0 when the defective value is very
close, i.e. they never match.
3) The alike() function does allow an error tolerance. This kicks in on s390x in the
case that the expected value is 0 and the defective value is not identical.
switch {
case IsNaN(a) && IsNaN(b):
return true
case a == b:
return Signbit(a) == Signbit(b)
}
A possible fix would be to detect this one situation and return true, and rely on more
specific tests to chack that the error is small ebough.
switch {
case IsNaN(a) && IsNaN(b):
return true
case a == 0 && !IsNaN(b) && !IsInf(b, 0):
// allow deviations when the expected value is zero
return true
case a == b:
return Signbit(a) == Signbit(b)
}
Comment 3 Ian Lance Taylor 2014-11-05 03:54:14 UTC
--------------------------------------------------
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.
Comment 4 Dominik Vogt 2014-11-05 14:34:38 UTC
----------------------------------------------
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.
Comment 5 Dominik Vogt 2014-11-05 16:47:03 UTC
----------------------------------------------
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.
The text was updated successfully, but these errors were encountered:
regarding 3)
I think, the alike() function is broken by design for the same reason as (1). It cannot
reasonably be expected that the result of the Log... functions is bit exact for all
input values (unless the math library would use a higher precision internally, which it
doesn't). The test for equality in the alike() function is too strict. Assuming thet
library code has been handmade to be bit exact given specific compiler, platform and
test cases it's not surprising that it fails with other platforms or compiler, e.g.
depending on optimisation choices.
(1) It seems reasonable to expect Log2 on powers of two to get exactly the right answer. Will fix the implementation.
(2) Thanks, will fix tolerance.
(3) alike is for exact matching. It's a stricter form of ==, for testing the outputs on special cases. We definitely do not want a tolerance there. I believe fixing Log2 on powers of two (in particular, Log2(1)) will make alike a non-issue.
rsc
changed the title
math: all_test.go: test failures in TestLog2 on s390
math: all_test.go: test failures in TestLog2 on ppc64 (and s390)
Jul 15, 2015
by vogt@linux.vnet.ibm.com:
The text was updated successfully, but these errors were encountered: