[PATCH] builtins: Fix up expand_builtin_int_roundingfn_2 [PR105211]

[PATCH] builtins: Fix up expand_builtin_int_roundingfn_2 [PR105211]

[Jakub Jelinek]

Hi!

The expansion of __builtin_iround{,f,l} etc. builtins in some cases
emits calls to a different fallback builtin.  To locate the right builtin
it uses mathfn_built_in_1 with the type of the first argument.
If its TYPE_MAIN_VARIANT is {float,double,long_double}_type_node, all is
fine, but on the following testcase, because GIMPLE considers scalar
float conversions between types with the same mode as useless,
TYPE_MAIN_VARIANT of the arg's type is float32_type_node and because there
isn't __builtin_lroundf32 returns NULL and we ICE.

This patch will first try the type of the argument and as fallback the
type of the first argument of the builtin (which can't be 100% trusted
either if user incorrectly prototypes it), and if neither works, doesn't
fallback.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Though, perhaps it would be better to prefer the TREE_VALUE (TYPE_ARG_TYPES)
type and only use TREE_TYPE (arg) as fallback, so that say on
a TYPE_MODE (double_type_mode) == TYPE_MODE (long_double_type_mode)
target we decide based on what builtin the user actually called rather than
whether the argument has been converted from the other type earlier.

2022-04-11  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/105211
	* builtins.cc (expand_builtin_int_roundingfn_2): If mathfn_built_in_1
	fails for TREE_TYPE (arg), retry it with
	TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl))) and if even that
	fails, emit call normally.

	* gcc.dg/pr105211.c: New test.

--- gcc/builtins.cc.jj	2022-03-14 10:34:34.122924399 +0100
+++ gcc/builtins.cc	2022-04-11 11:57:31.178251743 +0200
@@ -2968,15 +2968,25 @@ expand_builtin_int_roundingfn_2 (tree ex
 	 a call to lround in the hope that the target provides at least some
 	 C99 functions.  This should result in the best user experience for
 	 not full C99 targets.  */
-      tree fallback_fndecl = mathfn_built_in_1
-	(TREE_TYPE (arg), as_combined_fn (fallback_fn), 0);
+      tree fallback_fndecl
+	= mathfn_built_in_1 (TREE_TYPE (arg), as_combined_fn (fallback_fn), 0);
+      /* As scalar float conversions with same mode are useless in GIMPLE,
+	 we can end up e.g. with _Float32 argument passed to float builtin,
+	 try to get the type from the builtin prototype instead.  */
+      if (fallback_fndecl == NULL_TREE)
+	if (tree argtypes = TYPE_ARG_TYPES (TREE_TYPE (fndecl)))
+	  fallback_fndecl
+	    = mathfn_built_in_1 (TREE_VALUE (argtypes),
+				 as_combined_fn (fallback_fn), 0);
+      if (fallback_fndecl)
+	{
+	  exp = build_call_nofold_loc (EXPR_LOCATION (exp),
+				       fallback_fndecl, 1, arg);
 
-      exp = build_call_nofold_loc (EXPR_LOCATION (exp),
-				   fallback_fndecl, 1, arg);
-
-      target = expand_call (exp, NULL_RTX, target == const0_rtx);
-      target = maybe_emit_group_store (target, TREE_TYPE (exp));
-      return convert_to_mode (mode, target, 0);
+	  target = expand_call (exp, NULL_RTX, target == const0_rtx);
+	  target = maybe_emit_group_store (target, TREE_TYPE (exp));
+	  return convert_to_mode (mode, target, 0);
+	}
     }
 
   return expand_call (exp, target, target == const0_rtx);
--- gcc/testsuite/gcc.dg/pr105211.c.jj	2022-04-11 11:48:17.369946248 +0200
+++ gcc/testsuite/gcc.dg/pr105211.c	2022-04-11 11:48:09.924049700 +0200
@@ -0,0 +1,11 @@
+/* PR rtl-optimization/105211 */
+/* { dg-do compile } */
+/* { dg-options "-Os -ffast-math" } */
+/* { dg-add-options float32 } */
+/* { dg-require-effective-target float32 } */
+
+short
+foo (_Float32 f)
+{
+  return __builtin_roundf (f);
+}

	Jakub

Re: [PATCH] builtins: Fix up expand_builtin_int_roundingfn_2 [PR105211]

[Richard Biener]

> Am 11.04.2022 um 18:45 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>:
> 
> Hi!
> 
> The expansion of __builtin_iround{,f,l} etc. builtins in some cases
> emits calls to a different fallback builtin.  To locate the right builtin
> it uses mathfn_built_in_1 with the type of the first argument.
> If its TYPE_MAIN_VARIANT is {float,double,long_double}_type_node, all is
> fine, but on the following testcase, because GIMPLE considers scalar
> float conversions between types with the same mode as useless,
> TYPE_MAIN_VARIANT of the arg's type is float32_type_node and because there
> isn't __builtin_lroundf32 returns NULL and we ICE.
> 
> This patch will first try the type of the argument and as fallback the
> type of the first argument of the builtin (which can't be 100% trusted
> either if user incorrectly prototypes it), and if neither works, doesn't
> fallback.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Richard 

> Though, perhaps it would be better to prefer the TREE_VALUE (TYPE_ARG_TYPES)
> type and only use TREE_TYPE (arg) as fallback, so that say on
> a TYPE_MODE (double_type_mode) == TYPE_MODE (long_double_type_mode)
> target we decide based on what builtin the user actually called rather than
> whether the argument has been converted from the other type earlier.

Possibly yes

> 2022-04-11  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR rtl-optimization/105211
>    * builtins.cc (expand_builtin_int_roundingfn_2): If mathfn_built_in_1
>    fails for TREE_TYPE (arg), retry it with
>    TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl))) and if even that
>    fails, emit call normally.
> 
>    * gcc.dg/pr105211.c: New test.
> 
> --- gcc/builtins.cc.jj    2022-03-14 10:34:34.122924399 +0100
> +++ gcc/builtins.cc    2022-04-11 11:57:31.178251743 +0200
> @@ -2968,15 +2968,25 @@ expand_builtin_int_roundingfn_2 (tree ex
>     a call to lround in the hope that the target provides at least some
>     C99 functions.  This should result in the best user experience for
>     not full C99 targets.  */
> -      tree fallback_fndecl = mathfn_built_in_1
> -    (TREE_TYPE (arg), as_combined_fn (fallback_fn), 0);
> +      tree fallback_fndecl
> +    = mathfn_built_in_1 (TREE_TYPE (arg), as_combined_fn (fallback_fn), 0);
> +      /* As scalar float conversions with same mode are useless in GIMPLE,
> +     we can end up e.g. with _Float32 argument passed to float builtin,
> +     try to get the type from the builtin prototype instead.  */
> +      if (fallback_fndecl == NULL_TREE)
> +    if (tree argtypes = TYPE_ARG_TYPES (TREE_TYPE (fndecl)))
> +      fallback_fndecl
> +        = mathfn_built_in_1 (TREE_VALUE (argtypes),
> +                 as_combined_fn (fallback_fn), 0);
> +      if (fallback_fndecl)
> +    {
> +      exp = build_call_nofold_loc (EXPR_LOCATION (exp),
> +                       fallback_fndecl, 1, arg);
> 
> -      exp = build_call_nofold_loc (EXPR_LOCATION (exp),
> -                   fallback_fndecl, 1, arg);
> -
> -      target = expand_call (exp, NULL_RTX, target == const0_rtx);
> -      target = maybe_emit_group_store (target, TREE_TYPE (exp));
> -      return convert_to_mode (mode, target, 0);
> +      target = expand_call (exp, NULL_RTX, target == const0_rtx);
> +      target = maybe_emit_group_store (target, TREE_TYPE (exp));
> +      return convert_to_mode (mode, target, 0);
> +    }
>     }
> 
>   return expand_call (exp, target, target == const0_rtx);
> --- gcc/testsuite/gcc.dg/pr105211.c.jj    2022-04-11 11:48:17.369946248 +0200
> +++ gcc/testsuite/gcc.dg/pr105211.c    2022-04-11 11:48:09.924049700 +0200
> @@ -0,0 +1,11 @@
> +/* PR rtl-optimization/105211 */
> +/* { dg-do compile } */
> +/* { dg-options "-Os -ffast-math" } */
> +/* { dg-add-options float32 } */
> +/* { dg-require-effective-target float32 } */
> +
> +short
> +foo (_Float32 f)
> +{
> +  return __builtin_roundf (f);
> +}
> 
>    Jakub
>