[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 Alan Modra changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #20 from Alan Modra --- Fixed
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #19 from Alan Modra --- Author: amodra Date: Mon Apr 11 13:47:40 2016 New Revision: 234881 URL: https://gcc.gnu.org/viewcvs?rev=234881=gcc=rev Log: PR70117, ppc long double isinf gcc/ PR target/70117 * builtins.c (fold_builtin_classify): For IBM extended precision, look at just the high-order double to test for NaN. (fold_builtin_interclass_mathfn): Similarly for Inf. For isnormal test just the high double for Inf but both doubles for subnormal limit. gcc/testsuite/ * gcc.target/powerpc/pr70117.c: New. Added: branches/gcc-4_9-branch/gcc/testsuite/gcc.target/powerpc/pr70117.c Modified: branches/gcc-4_9-branch/gcc/ChangeLog branches/gcc-4_9-branch/gcc/builtins.c branches/gcc-4_9-branch/gcc/testsuite/ChangeLog
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #18 from Alan Modra --- Author: amodra Date: Mon Apr 11 13:46:51 2016 New Revision: 234880 URL: https://gcc.gnu.org/viewcvs?rev=234880=gcc=rev Log: PR70117, ppc long double isinf gcc/ PR target/70117 * builtins.c (fold_builtin_classify): For IBM extended precision, look at just the high-order double to test for NaN. (fold_builtin_interclass_mathfn): Similarly for Inf. For isnormal test just the high double for Inf but both doubles for subnormal limit. gcc/testsuite/ * gcc.target/powerpc/pr70117.c: New. Added: branches/gcc-5-branch/gcc/testsuite/gcc.target/powerpc/pr70117.c Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/builtins.c branches/gcc-5-branch/gcc/testsuite/ChangeLog
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #17 from Alan Modra --- Author: amodra Date: Fri Apr 8 02:11:52 2016 New Revision: 234821 URL: https://gcc.gnu.org/viewcvs?rev=234821=gcc=rev Log: PR70117, ppc long double isinf gcc/ PR target/70117 * builtins.c (fold_builtin_classify): For IBM extended precision, look at just the high-order double to test for NaN. (fold_builtin_interclass_mathfn): Similarly for Inf. For isnormal test just the high double for Inf but both doubles for subnormal limit. gcc/testsuite/ * gcc.target/powerpc/pr70117.c: New. Added: trunk/gcc/testsuite/gcc.target/powerpc/pr70117.c Modified: trunk/gcc/ChangeLog trunk/gcc/builtins.c trunk/gcc/testsuite/ChangeLog
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #16 from joseph at codesourcery dot com --- On Tue, 5 Apr 2016, amodra at gmail dot com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 > > --- Comment #14 from Alan Modra --- > > if (fmt == _extended_double) > > No, there is mips_extended_format too. I think that's now unused after the removal of IRIX support, and so should be removed from GCC. (The support for that format in fp-bit.[ch] is also now unused.)
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #15 from Richard Biener --- (In reply to Alan Modra from comment #14) > > if (fmt == _extended_double) > > No, there is mips_extended_format too. As said above the best is to provide optabs for all three fns and optimal implementations in the backend - that will disable the folding as well. Emitting a comparison as we do now is stupid and inefficient given that only the value of one of the double matters anyway.
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #14 from Alan Modra --- > if (fmt == _extended_double) No, there is mips_extended_format too.
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117
--- Comment #13 from Michael Meissner ---
In gcc/builtins.c it is probably better to use:
const struct real_format *fmt = FLOAT_MODE_FORMAT (mode);
if (fmt == _extended_double)
{
// ...
}
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 Alan Modra changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |amodra at gmail dot com
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #12 from rguenther at suse dot de --- On Mon, 14 Mar 2016, joseph at codesourcery dot com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 > > --- Comment #11 from joseph at codesourcery dot com dot com> --- > On Mon, 14 Mar 2016, rguenth at gcc dot gnu.org wrote: > > > but it will end up with libcalls for isinf, isfinite and isnormal for > > IBM extended double. I'm told glibc does the right thing for the gnulib > > And corresponding libcalls don't exist (or at least GCC doesn't know of > them; glibc has some functions such as __isinfl internally). Hmm, so the target(s) would need to import softfp variants into their libgcc then. Not a very good solution IMHO. This leaves the option to implement optabs and target patterns for all three functions (only the isinf optab is present at the moment). Such change could be safely backported to release branches (the softfp hackery likely not). > For the question of correctness of LDBL_MAX - and the related question of > whether LDBL_EPSILON, although correct by the current standard definition, > should change - see DR#467 (open). (But the reference there to DBL_MAX*2 > as a finite number does not apply to the IBM long double format > implemented by GCC, because of the rule about the round-to-nearest sum of > the two parts equalling the top part.) > (libgcc/config/rs6000/ibm-ldouble-format defines values with more than 106 > mantissa bits not to be normal - denormal but not subnormal.) So denormals are isinf()? I still believe the folding we apply is at least fishy for these kinds of float formats. Richard.
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #11 from joseph at codesourcery dot com --- On Mon, 14 Mar 2016, rguenth at gcc dot gnu.org wrote: > but it will end up with libcalls for isinf, isfinite and isnormal for > IBM extended double. I'm told glibc does the right thing for the gnulib And corresponding libcalls don't exist (or at least GCC doesn't know of them; glibc has some functions such as __isinfl internally). For the question of correctness of LDBL_MAX - and the related question of whether LDBL_EPSILON, although correct by the current standard definition, should change - see DR#467 (open). (But the reference there to DBL_MAX*2 as a finite number does not apply to the IBM long double format implemented by GCC, because of the rule about the round-to-nearest sum of the two parts equalling the top part.) (libgcc/config/rs6000/ibm-ldouble-format defines values with more than 106 mantissa bits not to be normal - denormal but not subnormal.)
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117
--- Comment #10 from Richard Biener ---
So, "simplest" patch:
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 234180)
+++ gcc/builtins.c (working copy)
@@ -7529,6 +7529,13 @@ fold_builtin_interclass_mathfn (location
mode = TYPE_MODE (TREE_TYPE (arg));
+ /* Do not perform this simplification for IBM extended double
+ which is made up of two IEEE doubles where only the first
+ one is relevant to the result of isinf, isfinite and isnormal. */
+ const struct real_format *fmt = FLOAT_MODE_FORMAT (mode);
+ if (fmt->pnan < fmt->p)
+return NULL_TREE;
+
/* If there is no optab, try generic code. */
switch (DECL_FUNCTION_CODE (fndecl))
{
but it will end up with libcalls for isinf, isfinite and isnormal for
IBM extended double. I'm told glibc does the right thing for the gnulib
DBL_MAX.
PPC people should evaluate if that's eventually even more efficient than
the inlining with the comparison against the large constant immediate.
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 Richard Biener changed: What|Removed |Added Target||powerpc* Status|UNCONFIRMED |NEW Last reconfirmed||2016-03-14 Ever confirmed|0 |1 --- Comment #9 from Richard Biener --- So another way to "fix" this would be to stop the simplification of isinf to isgreater(fabs(x),DBL_MAX) by providing patterns for isinf and friends (isfinite, isnormal), see fold_builtin_interclass_mathfn. Though neither isfinite nor isnormal have associated optabs - breaking testcases for them could be easily constructed. For the long double case more optimal implementation should be possible in the backend. Given we can't easily expose long double implementation details on GENERIC/GIMPLE it needs to be done there (well, in theory we could use sth. like VIEW_CONVERT (ldbl_1).d[0] or so ... or REALPART (V_C_E <_Complex double> (ldbl_1)) whatever would work better. But then struct realformat needs to expose this fact (real.c checks fmt->pnan < fmt->p for this!?). Confirmed anyway. The question remains whether the long double value from gnulib is "valid" (it's a valid bit pattern that is _not_ inf after all). I still somewhat think that GCC is at fault here and not gnulib - even if it is technically not LDBL_MAX that is wrong but the folding to isgreater(fabs(x),DBL_MAX).
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 Michel Normand changed: What|Removed |Added CC||normand at linux dot vnet.ibm.com --- Comment #8 from Michel Normand --- How to make this problem to be solved between gcc and gnulib ? There is pending discussion in gnulib ML at http://lists.gnu.org/archive/html/bug-gnulib/2016-03/msg00032.html
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #7 from Ulrich Weigand --- Ah, OK. I did't realize this value didn't fit into a 106-bit mantissa. I agree that it probably doesn't make sense to change the internal representation to allow larger mantissas. First of all, there's nothing really special about 107 bits; there can be IBM long double values that would require a much larger mantissa in the internal representation, since we can have many implicit zero bits. But more problematical, if we change the internal representation to a mantissa larger than 106 bits, there will be values in that internal format that cannot be represented directly in the target IBM long double format. In any case, I certainly agree that the is* routines for IBM long double should simply operate on the high double of the pair. I still think that it would be better for gnulib to use the same LDBL_MAX as GCC, which means gnulib should probably be changed to use the 106-bit value.
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #6 from Alan Modra --- > Well, what I don't quite understand is that the gnulib value, which is > > 0x1.f7cp+1023 Sorry, I didn't look properly at the bug before commenting last night. For some reason I thought the gnulib value didn't have the needed zero bit in the mantissa.. > likewise should round to the same double value, shouldn't it? Yes, it should, but gcc's LDBL_MAX is the largest 106 bit precision IBM extended double. The gnulib value has 107 bits of precision, 53 bits from each of the component doubles plus an implicit zero bit. As Joseph says, the problem is that gcc evaluates IBM extended double expressions to 106 bit precision. Widening to 107 bits of precision would almost certainly cause other problems.
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #5 from joseph at codesourcery dot com --- The issue is that GCC internally handles IBM long double as having a 106-bit mantissa. There is one value that is larger than can be represented with a 106-bit mantissa, while still being finite and satisfying the rule that the two halves, added in round-to-nearest, produce the top half. In my view, isinf, isnan and isfinite for IBM long double should all be special-cased in GCC to call the corresponding operation on the top half of the number (for both hard and soft float). This is correct in all cases and more efficient than anything that needs to look at both halves. It should also be a lot easier than fixing LDBL_MAX by allowing numbers with mantissas wider than 106 bits.
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 --- Comment #4 from Ulrich Weigand --- (In reply to Alan Modra from comment #3) > > while with GCC, we get: > > > > high double: 7FEF > > low double: 7C8F FFFE > > Right. This is 0x1.f78p+1023 > > gnulib isn't correct here. As the comment says the high double must be the > value of the long double correctly rounded to double (to nearest since that > is the only mode supported for IBM extended double). Any long double value > higher than the above will round up the high double to inf. Well, what I don't quite understand is that the gnulib value, which is 0x1.f7cp+1023 likewise should round to the same double value, shouldn't it? I notice that if I actually attempt to use that value in C source code, the compiler does indeed round it to inf -- but I don't see why it actually should do so ...
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117 Alan Modra changed: What|Removed |Added CC||amodra at gmail dot com --- Comment #3 from Alan Modra --- > while with GCC, we get: > > high double: 7FEF > low double: 7C8F FFFE Right. This is 0x1.f78p+1023 gnulib isn't correct here. As the comment says the high double must be the value of the long double correctly rounded to double (to nearest since that is the only mode supported for IBM extended double). Any long double value higher than the above will round up the high double to inf.
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117
--- Comment #2 from rguenther at suse dot de ---
On Mon, 7 Mar 2016, uweigand at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117
>
> Ulrich Weigand changed:
>
>What|Removed |Added
>
> CC||amodra at gcc dot gnu.org,
>||dje at gcc dot gnu.org,
>||uweigand at gcc dot gnu.org
>
> --- Comment #1 from Ulrich Weigand ---
> Hmm. For some reason, the gnulib definition of LDBL_MAX differs from GCC's
> definition. With gnulib, we get:
>
> high double: 7FEF
> low double: 7C8F
>
> while with GCC, we get:
>
> high double: 7FEF
> low double: 7C8F FFFE
>
> In any case, someone is wrong here -- values of LDBL_MAX should certainly
> agree.
>
> Now I'm not completely sure why GCC choses the value it does, I notice that
> GCC's choice is certainly deliberate; there's extra code to flip the last bit:
>
> real.c:get_max_float
>
> if (fmt->pnan < fmt->p)
> {
> /* This is an IBM extended double format made up of two IEEE
> doubles. The value of the long double is the sum of the
> values of the two parts. The most significant part is
> required to be the value of the long double rounded to the
> nearest double. Rounding means we need a slightly smaller
> value for LDBL_MAX. */
> buf[4 + fmt->pnan / 4] = "7bde"[fmt->pnan % 4];
> }
>
> and similarly real.c:real_maxval
>
> if (fmt->pnan < fmt->p)
> /* This is an IBM extended double format made up of two IEEE
>doubles. The value of the long double is the sum of the
>values of the two parts. The most significant part is
>required to be the value of the long double rounded to the
>nearest double. Rounding means we need a slightly smaller
>value for LDBL_MAX. */
> clear_significand_bit (r, SIGNIFICAND_BITS - fmt->pnan - 1);
>
> This code was originally added by Alan back in 2004 ... adding Alan on CC if
> he
> recalls what this was all about.
Note that independent of that question whether the number is isinf or not
depends only on the first double (which is not inf). So eventually
just the folding is bogus - who knows.
[Bug target/70117] ppc long double isinf() is wrong?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70117
Ulrich Weigand changed:
What|Removed |Added
CC||amodra at gcc dot gnu.org,
||dje at gcc dot gnu.org,
||uweigand at gcc dot gnu.org
--- Comment #1 from Ulrich Weigand ---
Hmm. For some reason, the gnulib definition of LDBL_MAX differs from GCC's
definition. With gnulib, we get:
high double: 7FEF
low double: 7C8F
while with GCC, we get:
high double: 7FEF
low double: 7C8F FFFE
In any case, someone is wrong here -- values of LDBL_MAX should certainly
agree.
Now I'm not completely sure why GCC choses the value it does, I notice that
GCC's choice is certainly deliberate; there's extra code to flip the last bit:
real.c:get_max_float
if (fmt->pnan < fmt->p)
{
/* This is an IBM extended double format made up of two IEEE
doubles. The value of the long double is the sum of the
values of the two parts. The most significant part is
required to be the value of the long double rounded to the
nearest double. Rounding means we need a slightly smaller
value for LDBL_MAX. */
buf[4 + fmt->pnan / 4] = "7bde"[fmt->pnan % 4];
}
and similarly real.c:real_maxval
if (fmt->pnan < fmt->p)
/* This is an IBM extended double format made up of two IEEE
doubles. The value of the long double is the sum of the
values of the two parts. The most significant part is
required to be the value of the long double rounded to the
nearest double. Rounding means we need a slightly smaller
value for LDBL_MAX. */
clear_significand_bit (r, SIGNIFICAND_BITS - fmt->pnan - 1);
This code was originally added by Alan back in 2004 ... adding Alan on CC if he
recalls what this was all about.
