Message ID | 54E6FDA2.5030408@mentor.com |
---|---|
State | New |
Headers | show |
On Fri, Feb 20, 2015 at 10:25:54AM +0100, Tom de Vries wrote: > this patch reverses the abort logic in pr30957-1.c, such that it aborts on > failure rather than on success. That sounds really weird. From the description it looks like it is a known bug that we don't return -0.0. If 0.0 is the right return value instead, I'd the test should be written as if (__builtin_copysignf (1.0, foo (0.0 / -5.0, 10)) != 1.0) abort (); to make it clear you are expecting positive 0. Jakub
On Feb 20, 2015, at 1:42 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Fri, Feb 20, 2015 at 10:25:54AM +0100, Tom de Vries wrote: >> this patch reverses the abort logic in pr30957-1.c, such that it aborts on >> failure rather than on success. > > That sounds really weird. From the description it looks like it is a known bug > that we don't return -0.0. > If 0.0 is the right return value instead, I'd the test should be written as > if (__builtin_copysignf (1.0, foo (0.0 / -5.0, 10)) != 1.0) > abort (); > to make it clear you are expecting positive 0. So, did you read the bug report? They expect the value -1.0, so, I think the above is wrong?
On Feb 20, 2015, at 10:12 AM, Mike Stump <mikestump@comcast.net> wrote: > On Feb 20, 2015, at 1:42 AM, Jakub Jelinek <jakub@redhat.com> wrote: >> On Fri, Feb 20, 2015 at 10:25:54AM +0100, Tom de Vries wrote: >>> this patch reverses the abort logic in pr30957-1.c, such that it aborts on >>> failure rather than on success. >> >> That sounds really weird. From the description it looks like it is a known bug >> that we don't return -0.0. >> If 0.0 is the right return value instead, I'd the test should be written as >> if (__builtin_copysignf (1.0, foo (0.0 / -5.0, 10)) != 1.0) >> abort (); >> to make it clear you are expecting positive 0. > > So, did you read the bug report? They expect the value -1.0, so, I think the above is wrong? Ignore that… I’ve read though more of the history and the whole think is just messier than I’d like. I now see why Tom proposed the patch he did.
diff --git a/gcc/testsuite/gcc.dg/pr30957-1.c b/gcc/testsuite/gcc.dg/pr30957-1.c index 65b98fa..45fb6d6 100644 --- a/gcc/testsuite/gcc.dg/pr30957-1.c +++ b/gcc/testsuite/gcc.dg/pr30957-1.c @@ -1,4 +1,4 @@ -/* { dg-do run { xfail *-*-* } } */ +/* { dg-do run } */ /* We don't (and don't want to) perform this optimisation on soft-float targets, where each addition is a library call. This test requires -fassociative-math for enabling the variable-expansion as well as @@ -27,8 +27,8 @@ int main () { if (__builtin_copysignf (1.0, foo (0.0 / -5.0, 10)) != -1.0) - abort (); - exit (0); + exit (0); + abort (); } /* { dg-final { scan-rtl-dump "Expanding Accumulator" "loop2_unroll" } } */