[patch] Fix PR fortran/96983

[Eric Botcazou]

Hi,

AFAICS the code in build_round_expr implicitly assumes that __float128 exists, 
which is *not* the common case among 64-bit architectures since "long double" 
is generally already 128-bit for them.

Tested on x86-64/Linux and SPARC64/Linux, OK for the mainline?


2021-03-08  Eric Botcazou  

PR fortran/96983
* trans-intrinsic.c (build_round_expr): Do not implicitly assume
that __float128 is the 128-bit floating-point type.

-- 
Eric Botcazoudiff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 5c9258c65c3..ae359a07973 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -407,7 +407,10 @@ build_round_expr (tree arg, tree restype)
   if (kind < 0)
 	gfc_internal_error ("Could not find real kind with at least %d bits",
 			resprec);
-  arg = fold_convert (gfc_float128_type_node, arg);
+  if (gfc_real16_is_float128)
+	arg = fold_convert (gfc_float128_type_node, arg);
+  else
+	arg = fold_convert (long_double_type_node, arg);
   fn = gfc_builtin_decl_for_float_kind (BUILT_IN_ROUND, kind);
 }
   else

Re: [patch] Fix PR fortran/96983

[Eric Botcazou]

> I think it is a bad idea to hard-code the real type.
> Does the following work for you? If so, I think that
> patch is obvious and you can go ahead and commit it.
> 
> --- a/gcc/fortran/trans-intrinsic.c
> +++ b/gcc/fortran/trans-intrinsic.c
> @@ -407,7 +407,7 @@ build_round_expr (tree arg, tree restype)
> if (kind < 0)
>  gfc_internal_error ("Could not find real kind with at least %d
> bits", resprec);
> -  arg = fold_convert (gfc_float128_type_node, arg);
> +  arg = fold_convert (gfc_get_real_type (kind), arg);
> fn = gfc_builtin_decl_for_float_kind (BUILT_IN_ROUND, kind);
>   }
> else

Yes, it works fine on x86-64/Linux and SPARC64/Linux, applied, thanks.

-- 
Eric Botcazou

Re: [PATCH][v2] Adjust volatile handling of the operand scanner

[Eric Botcazou]

> The question is whether we instead want to amend build3 to
> set TREE_THIS_VOLATILE automatically when the FIELD_DECL has
> it set.  At least for the Fortran FE cases the gimplifier
> fails to see some volatile references and thus can generate
> wrong code which is a latent issue.

What do we do for other similar flags, e.g. TREE_READONLY?

-- 
Eric Botcazou

Re: [PATCH][v2] Adjust volatile handling of the operand scanner

[Eric Botcazou]

> build3 currently does no special processing for the FIELD_DECL operand,
> it just sets TREE_THIS_VOLATILE from operand zero for tcc_references.
> 
> The C and C++ frontends have repeated patterns like
> 
>   ref = build3 (COMPONENT_REF, subtype, datum, subdatum,
> NULL_TREE);
>   SET_EXPR_LOCATION (ref, loc);
>   if (TREE_READONLY (subdatum)
> 
>   || (use_datum_quals && TREE_READONLY (datum)))
> 
> TREE_READONLY (ref) = 1;
>   if (TREE_THIS_VOLATILE (subdatum)
> 
>   || (use_datum_quals && TREE_THIS_VOLATILE (datum)))
> 
> TREE_THIS_VOLATILE (ref) = 1;

Likewise in the Ada front-end (gigi).

> Now - I wonder if there's a reason a frontend might _not_ want to
> set TREE_THIS_VOLATILE on a COMPONENT_REF when the FIELD_DECL has
> TREE_THIS_VOLATILE set.

This would be weird semantics in my opinion.

> I guess I'll do one more experiment and add verification that
> TREE_THIS_VOLATILE on COMPONENT_REFs and FIELD_DECLs is consistent
> and see where that trips.

Sounds good to me.

-- 
Eric Botcazou

Re: [PATCH][v2] Adjust volatile handling of the operand scanner

[Eric Botcazou]

> So I'm leaning towards leaving build3 alone and fixing up frontends
> as issues pop up.

FWIW fine with me.

-- 
Eric Botcazou

Re: Adding a new thread model to GCC

[Eric Botcazou via Fortran]

> fixed now.
> bootstrapped successfully!

Thanks for fixing it.  Another way out is to hide the Win32 API by defining  
__GTHREAD_HIDE_WIN32API like libstdc++ does in its header files.

-- 
Eric Botcazou