diff mbox series

[2/4] fortran: Teach get_real_kind_from_node for Power 128 fp modes [PR112993]

Message ID dbccfa2d-6794-e267-79dd-18860e2212a6@linux.ibm.com
State New
Headers show
Series [1/4] rs6000: Make all 128 bit scalar FP modes have 128 bit precision [PR112993] | expand

Commit Message

Kewen.Lin May 8, 2024, 5:27 a.m. UTC 
Hi,

Previously effective target fortran_real_c_float128 never
passes on Power regardless of the default 128 long double
is ibmlongdouble or ieeelongdouble.  It's due to that TF
mode is always used for kind 16 real, which has precision
127, while the node float128_type_node for c_float128 has
128 type precision, get_real_kind_from_node can't find a
matching as it only checks gfc_real_kinds[i].mode_precision
and type precision.

With changing TFmode/IFmode/KFmode to have the same mode
precision 128, now fortran_real_c_float12 can pass with
ieeelongdouble enabled by default and test cases guarded
with it get tested accordingly.  But with ibmlongdouble
enabled by default, since TFmode has precision 128 which
is the same as type precision 128 of float128_type_node,
get_real_kind_from_node considers kind for TFmode matches
float128_type_node, but it's wrong as at this time point
TFmode is with ibm extended format.  So this patch is to
teach get_real_kind_from_node to check one more field which
can be differentiable from the underlying real format, it
can avoid the unexpected matching when there more than one
modes have the same precision.

Bootstrapped and regress-tested on:
  - powerpc64-linux-gnu P8/P9 (with ibm128 by default)
  - powerpc64le-linux-gnu P9/P10 (with ibm128 by default)
  - powerpc64le-linux-gnu P9 (with ieee128 by default)

BR,
Kewen
-----
	PR target/112993

gcc/fortran/ChangeLog:

	* trans-types.cc (get_real_kind_from_node): Consider the case where
	more than one modes have the same precision.
---
 gcc/fortran/trans-types.cc | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

--
2.39.1

Comments

Mikael Morin May 8, 2024, 7:40 a.m. UTC | #1 
Hello,

Le 08/05/2024 à 07:27, Kewen.Lin a écrit :
> Hi,
> 
> Previously effective target fortran_real_c_float128 never
> passes on Power regardless of the default 128 long double
> is ibmlongdouble or ieeelongdouble.  It's due to that TF
> mode is always used for kind 16 real, which has precision
> 127, while the node float128_type_node for c_float128 has
> 128 type precision, get_real_kind_from_node can't find a
> matching as it only checks gfc_real_kinds[i].mode_precision
> and type precision.
> 
> With changing TFmode/IFmode/KFmode to have the same mode
> precision 128, now fortran_real_c_float12 can pass with
> ieeelongdouble enabled by default and test cases guarded
> with it get tested accordingly.  But with ibmlongdouble
> enabled by default, since TFmode has precision 128 which
> is the same as type precision 128 of float128_type_node,
> get_real_kind_from_node considers kind for TFmode matches
> float128_type_node, but it's wrong as at this time point
> TFmode is with ibm extended format.  So this patch is to
> teach get_real_kind_from_node to check one more field which
> can be differentiable from the underlying real format, it
> can avoid the unexpected matching when there more than one
> modes have the same precision.
> 
> Bootstrapped and regress-tested on:
>    - powerpc64-linux-gnu P8/P9 (with ibm128 by default)
>    - powerpc64le-linux-gnu P9/P10 (with ibm128 by default)
>    - powerpc64le-linux-gnu P9 (with ieee128 by default)
> 
OK from the fortran point of view.
Thanks.

> BR,
> Kewen
Steve Kargl May 8, 2024, 10:01 p.m. UTC | #2 
On Wed, May 08, 2024 at 01:27:53PM +0800, Kewen.Lin wrote:
> 
> Previously effective target fortran_real_c_float128 never
> passes on Power regardless of the default 128 long double
> is ibmlongdouble or ieeelongdouble.  It's due to that TF
> mode is always used for kind 16 real, which has precision
> 127, while the node float128_type_node for c_float128 has
> 128 type precision, get_real_kind_from_node can't find a
> matching as it only checks gfc_real_kinds[i].mode_precision
> and type precision.
> 
> With changing TFmode/IFmode/KFmode to have the same mode
> precision 128, now fortran_real_c_float12 can pass with
> ieeelongdouble enabled by default and test cases guarded
> with it get tested accordingly.  But with ibmlongdouble
> enabled by default, since TFmode has precision 128 which
> is the same as type precision 128 of float128_type_node,
> get_real_kind_from_node considers kind for TFmode matches
> float128_type_node, but it's wrong as at this time point
> TFmode is with ibm extended format.  So this patch is to
> teach get_real_kind_from_node to check one more field which
> can be differentiable from the underlying real format, it
> can avoid the unexpected matching when there more than one
> modes have the same precision.
> 
> Bootstrapped and regress-tested on:
>   - powerpc64-linux-gnu P8/P9 (with ibm128 by default)
>   - powerpc64le-linux-gnu P9/P10 (with ibm128 by default)
>   - powerpc64le-linux-gnu P9 (with ieee128 by default)
> 
> BR,
> Kewen
> -----
> 	PR target/112993
> 

First, I have no issue with Mikael's OK for committing the
patch. 

That said, Fortran has the concept of model numbers, which
are set in arith.c.  Does this change give the expected 
value for ibm128?  For example, with "REAL(16) X", one
has "DIGITS(X) = 113", which is the precision on the 
of the underlying IEEE754 binary128 type.
Kewen.Lin May 9, 2024, 5:37 a.m. UTC | #3 
Hi,

on 2024/5/9 06:01, Steve Kargl wrote:
> On Wed, May 08, 2024 at 01:27:53PM +0800, Kewen.Lin wrote:
>>
>> Previously effective target fortran_real_c_float128 never
>> passes on Power regardless of the default 128 long double
>> is ibmlongdouble or ieeelongdouble.  It's due to that TF
>> mode is always used for kind 16 real, which has precision
>> 127, while the node float128_type_node for c_float128 has
>> 128 type precision, get_real_kind_from_node can't find a
>> matching as it only checks gfc_real_kinds[i].mode_precision
>> and type precision.
>>
>> With changing TFmode/IFmode/KFmode to have the same mode
>> precision 128, now fortran_real_c_float12 can pass with
>> ieeelongdouble enabled by default and test cases guarded
>> with it get tested accordingly.  But with ibmlongdouble
>> enabled by default, since TFmode has precision 128 which
>> is the same as type precision 128 of float128_type_node,
>> get_real_kind_from_node considers kind for TFmode matches
>> float128_type_node, but it's wrong as at this time point
>> TFmode is with ibm extended format.  So this patch is to
>> teach get_real_kind_from_node to check one more field which
>> can be differentiable from the underlying real format, it
>> can avoid the unexpected matching when there more than one
>> modes have the same precision.
>>
>> Bootstrapped and regress-tested on:
>>   - powerpc64-linux-gnu P8/P9 (with ibm128 by default)
>>   - powerpc64le-linux-gnu P9/P10 (with ibm128 by default)
>>   - powerpc64le-linux-gnu P9 (with ieee128 by default)
>>
>> BR,
>> Kewen
>> -----
>> 	PR target/112993
>>

> OK from the fortran point of view.
> Thanks.

> 
> First, I have no issue with Mikael's OK for committing the
> patch. 

Thanks to both!

> 
> That said, Fortran has the concept of model numbers, which
> are set in arith.c.  Does this change give the expected 
> value for ibm128?  For example, with "REAL(16) X", one
> has "DIGITS(X) = 113", which is the precision on the 
> of the underlying IEEE754 binary128 type.
> 

With some testings locally, I noticed that currently DIGITS has
been already correct even without this change.  For "REAL(16) X",
with -mabi=ibmlongdouble it's long double with ibm128 format and
its DIGITS(X) is 106, while with -mabi=ieeelongdouble it's long
double with ieee128 format and its DIGITS(X) is 113.

BR,
Kewen
Steve Kargl May 9, 2024, 4:47 p.m. UTC | #4 
On Thu, May 09, 2024 at 01:37:32PM +0800, Kewen.Lin wrote:
> 
> > 
> > That said, Fortran has the concept of model numbers, which
> > are set in arith.c.  Does this change give the expected 
> > value for ibm128?  For example, with "REAL(16) X", one
> > has "DIGITS(X) = 113", which is the precision on the 
> > of the underlying IEEE754 binary128 type.
> > 
> 
> With some testings locally, I noticed that currently DIGITS has
> been already correct even without this change.  For "REAL(16) X",
> with -mabi=ibmlongdouble it's long double with ibm128 format and
> its DIGITS(X) is 106, while with -mabi=ieeelongdouble it's long
> double with ieee128 format and its DIGITS(X) is 113.
> 

That's good.  I'll take a look later this weekend
at arith.c.  There are few others to consider:
precision(x), minexponent(x), maxexponent(x), huge(x),
and tiny(x).
diff mbox series

Patch

diff --git a/gcc/fortran/trans-types.cc b/gcc/fortran/trans-types.cc
index 676014e9b98..dd94ef77741 100644
--- a/gcc/fortran/trans-types.cc
+++ b/gcc/fortran/trans-types.cc
@@ -183,7 +183,21 @@  get_real_kind_from_node (tree type)

   for (i = 0; gfc_real_kinds[i].kind != 0; i++)
     if (gfc_real_kinds[i].mode_precision == TYPE_PRECISION (type))
-      return gfc_real_kinds[i].kind;
+      {
+	/* On Power, we have three 128-bit scalar floating-point modes
+	   and all of their types have 128 bit type precision, so we
+	   should check underlying real format details further.  */
+#if defined(HAVE_TFmode) && defined(HAVE_IFmode) && defined(HAVE_KFmode)
+	if (gfc_real_kinds[i].kind == 16)
+	  {
+	    machine_mode mode = TYPE_MODE (type);
+	    const struct real_format *fmt = REAL_MODE_FORMAT (mode);
+	    if (fmt->p != gfc_real_kinds[i].digits)
+	      continue;
+	  }
+#endif
+	return gfc_real_kinds[i].kind;
+      }

   return -4;
 }