On 8/10/21 2:29 AM, Tobias Burnus wrote:

[snip]

To conclude: I like the code changes (LGTM); the
'__float128' -> 'TFmode' comment change also matches the code.

However, I think both longer comments need to be updated.

OK. I used your wording verbatim for the first one. For the second one, I'm still pretty confused as I think it is at least theoretically possible on PowerPC to have a target with 64-bit long double (AIX?) that also supports the __ibm128 format, and it would be wrong to assume that *any* 128-bit mode that's not long double is IEEE. So I decided the best thing is just to replace the FIXME with a pointer to the issue I opened yesterday

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101835

and folks who actually understand this stuff can add some more discussion there.

Here's a new version of the patch with changes to the two comments only. OK to commit this one?

-Sandra
commit e496723ec6ee0fcf5b0c58920c74b1c9afa831de
Author: Sandra Loosemore <san...@codesourcery.com>
Date:   Tue Aug 3 16:21:16 2021 -0700

    Fix c_float128 and c_float128_complex definitions.
    
    gfc_float128_type_node is only non-NULL on targets that support a
    128-bit type that is not long double.  Use float128_type_node instead
    when computing the value of the kind constants c_float128 and
    c_float128_complex from the ISO_C_BINDING intrinsic module; this also
    ensures it actually corresponds to __float128 (the IEEE encoding) and
    not some other 128-bit floating-point type.
    
    2021-08-10  Sandra Loosemore  <san...@codesourcery.com>
    
    gcc/fortran/
    	* iso-c-binding.def (c_float128, c_float128_complex): Check
    	float128_type_node instead of gfc_float128_type_node.
    	* trans-types.c (gfc_init_kinds, gfc_build_real_type):
    	Update comments re supported 128-bit floating-point types.

diff --git a/gcc/fortran/iso-c-binding.def b/gcc/fortran/iso-c-binding.def
index 8bf69ef..e65c750 100644
--- a/gcc/fortran/iso-c-binding.def
+++ b/gcc/fortran/iso-c-binding.def
@@ -114,9 +114,14 @@ NAMED_REALCST (ISOCBINDING_DOUBLE, "c_double", \
                get_real_kind_from_node (double_type_node), GFC_STD_F2003)
 NAMED_REALCST (ISOCBINDING_LONG_DOUBLE, "c_long_double", \
                get_real_kind_from_node (long_double_type_node), GFC_STD_F2003)
+
+/* GNU Extension.  Note that the equivalence here is specifically to
+   the IEEE 128-bit type __float128; if that does not map onto a type
+   otherwise supported by the Fortran front end, get_real_kind_from_node
+   will reject it as unsupported.  */
 NAMED_REALCST (ISOCBINDING_FLOAT128, "c_float128", \
-	       gfc_float128_type_node == NULL_TREE \
-		  ? -4 : get_real_kind_from_node (gfc_float128_type_node), \
+		(float128_type_node == NULL_TREE \
+		 ? -4 : get_real_kind_from_node (float128_type_node)), \
 	       GFC_STD_GNU)
 NAMED_CMPXCST (ISOCBINDING_FLOAT_COMPLEX, "c_float_complex", \
                get_real_kind_from_node (float_type_node), GFC_STD_F2003)
@@ -124,9 +129,11 @@ NAMED_CMPXCST (ISOCBINDING_DOUBLE_COMPLEX, "c_double_complex", \
                get_real_kind_from_node (double_type_node), GFC_STD_F2003)
 NAMED_CMPXCST (ISOCBINDING_LONG_DOUBLE_COMPLEX, "c_long_double_complex", \
                get_real_kind_from_node (long_double_type_node), GFC_STD_F2003)
+
+/* GNU Extension.  Similar issues to c_float128 above.  */
 NAMED_CMPXCST (ISOCBINDING_FLOAT128_COMPLEX, "c_float128_complex", \
-	       gfc_float128_type_node == NULL_TREE \
-		  ? -4 : get_real_kind_from_node (gfc_float128_type_node), \
+		(float128_type_node == NULL_TREE \
+		 ? -4 : get_real_kind_from_node (float128_type_node)), \
 	       GFC_STD_GNU)
 
 NAMED_LOGCST (ISOCBINDING_BOOL, "c_bool", \
diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c
index 50fda43..26dbbd8 100644
--- a/gcc/fortran/trans-types.c
+++ b/gcc/fortran/trans-types.c
@@ -446,7 +446,7 @@ gfc_init_kinds (void)
       if (!targetm.scalar_mode_supported_p (mode))
 	continue;
 
-      /* Only let float, double, long double and __float128 go through.
+      /* Only let float, double, long double and TFmode go through.
 	 Runtime support for others is not provided, so they would be
 	 useless.  */
       if (!targetm.libgcc_floating_mode_supported_p (mode))
@@ -471,7 +471,14 @@ gfc_init_kinds (void)
 	 We round up so as to handle IA-64 __floatreg (RFmode), which is an
 	 82 bit type.  Not to be confused with __float80 (XFmode), which is
 	 an 80 bit type also supported by IA-64.  So XFmode should come out
-	 to be kind=10, and RFmode should come out to be kind=11.  Egads.  */
+	 to be kind=10, and RFmode should come out to be kind=11.  Egads.
+
+	 TODO: The kind calculation has to be modified to support all
+	 three 128-bit floating-point modes on PowerPC as IFmode, KFmode,
+	 and TFmode since the following line would all map to kind=16.
+	 However, currently only float, double, long double, and TFmode
+	 reach this code.
+      */
 
       kind = (GET_MODE_PRECISION (mode) + 7) / 8;
 
@@ -851,6 +858,7 @@ gfc_build_real_type (gfc_real_info *info)
     info->c_long_double = 1;
   if (mode_precision != LONG_DOUBLE_TYPE_SIZE && mode_precision == 128)
     {
+      /* See PR101835.  */
       info->c_float128 = 1;
       gfc_real16_is_float128 = true;
     }

Reply via email to