From: Richard Biener <rguenther@suse.de>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Jeff Law <jeffreyalaw@gmail.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] builtins: Fix up expand_builtin_int_roundingfn_2 [PR105211]
Date: Mon, 11 Apr 2022 19:03:18 +0200 [thread overview]
Message-ID: <9C125FF2-129D-4003-88AB-E69ECBD41B25@suse.de> (raw)
In-Reply-To: <YlRa3pMUoXHUai6S@tucnak>
> 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
>
prev parent reply other threads:[~2022-04-11 17:03 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-11 16:44 Jakub Jelinek
2022-04-11 17:03 ` Richard Biener [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9C125FF2-129D-4003-88AB-E69ECBD41B25@suse.de \
--to=rguenther@suse.de \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=jeffreyalaw@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).