On 8/5/21 2:09 PM, Michael Meissner wrote:

All PowerPC systems that I'm aware of that use 128-bit floating point use the
IBM format.  It is anticipated that one or more Linux distributions in the
future may move to IEEE 128-bit format, but right now, I'm not aware that any
have moved.

OK. I now think it's correct that the Fortran front end doesn't support c_float128 and c_float128_complex on this target, but that the code that initializes those constants is still buggy. The reason why it shouldn't support them is that all 3 128-bit floating-point modes on PowerPC would map onto kind=16, and we can only support one of them unless we make some exception to the formula for mapping precision -> kind. And the mode the Fortran front end already prefers is the one that corresponds to long double or TFmode.

So the reason why the current code is wrong is that gfc_float128_type_node only gets set if there is a type with 128-bit precision that is not long double, so c_float128 wouldn't be supported on a target where __float128 is equivalent to long double. Moreover, on targets where it does define gfc_float128_type_node, it's not doing anything to ensure that it's actually the same IEEE representation as __float128. Both problems can be fixed by just using float128_type_node to get the type corresponding to __float128. If that's not a mode the Fortran front end knows about, get_real_kind_from_node will detect that.

I suspect there are some similar lurking bugs in other uses of gfc_float128_type_node in the Fortran front end, but that's out of scope for my current project focusing on interoperability features. I'll file an issue about it, though.

Here's a revised patch.  Fortran maintainers, is this one OK to check in?

-Sandra
commit 073dd403d11553c199610b038285b203c130cee5
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-09  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..8250219 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,15 @@ 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.
+
+	 One consequence of this mapping is that all 3 128-bit
+	 floating-point modes on PowerPC would have kind 16 in spite of
+	 having incompatible representations.  But we have filtered out
+	 IFmode (the explicit IBM format) and KFmode (the explicit IEEE
+	 format) already in the code just above, leaving only whichever
+	 representation has been designated for TFmode as kind=16.
+      */
 
       kind = (GET_MODE_PRECISION (mode) + 7) / 8;
 
@@ -851,6 +859,9 @@ gfc_build_real_type (gfc_real_info *info)
     info->c_long_double = 1;
   if (mode_precision != LONG_DOUBLE_TYPE_SIZE && mode_precision == 128)
     {
+      /* FIXME: why are we assuming that this type has IEEE
+	 representation?  That's implied by associating it with the
+	 C type __float128.  */
       info->c_float128 = 1;
       gfc_real16_is_float128 = true;
     }

Reply via email to