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

[Richard Biener via Gcc-patches]

> 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  <ja...@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
>