From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 434073857353 for ; Mon, 16 May 2022 06:31:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 434073857353 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 12FDE21F2B; Mon, 16 May 2022 06:31:11 +0000 (UTC) Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id EFA532C141; Mon, 16 May 2022 06:31:10 +0000 (UTC) Date: Mon, 16 May 2022 06:31:10 +0000 (UTC) From: Richard Biener To: Tamar Christina cc: gcc-patches@gcc.gnu.org, nd@arm.com, jeffreyalaw@gmail.com, richard.sandiford@arm.com Subject: Re: [PATCH 1/3]middle-end: Add the ability to let the target decide the method of argument promotions. In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_LOTSOFHASH, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 May 2022 06:31:14 -0000 On Fri, 13 May 2022, Tamar Christina wrote: > Hi All, > > Some targets require function parameters to be promoted to a different > type on expand time because the target may not have native instructions > to work on such types. As an example the AArch64 port does not have native > instructions working on integer 8- or 16-bit values. As such it promotes > every parameter of these types to 32-bits. > > This promotion could be done by a target for two reasons: > > 1. For correctness. This may be an APCS requirement for instance. > 2. For efficiency. By promoting the argument at expansion time we don't have > to keep promoting the type back and forth after each operation on it. > i.e. the promotion simplies the RTL. > > This patch adds the ability for a target to decide whether during the expansion > to use an extend to handle promotion or to use a paradoxical subreg. > > A pradoxical subreg can be used when there's no correctness issues and when you > still want the RTL efficiency of not doing the promotion constantly. > > This also allows the target to not need to generate any code when the top bits > are not significant. > > An example is in AArch64 the following extend is unneeded: > > uint8_t fd2 (uint8_t xr){ > return xr + 1; > } > > currently generates: > > fd2: > and w0, w0, 255 > add w0, w0, 1 > ret > > instead of > > fd2: > add w0, w0, #1 > ret > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > Bootstrapped on x86_64-pc-linux-gnu and no issues > > Ok for master? Why do we need a target hook for this? Why doesn't the targets function_arg(?) hook just return (subreg:SI (reg:QI 0)) here instead when no zero-extension is required and (reg:QI 0) when it is? That said, an extra hook looks like a bad design to me, the existing cummulative args way of querying the target ABI shoud be enough, and if not, should be extended in a less hackish way. But of course I am not familiar at all with the current state, but since you specifially CCed me ... Richard. > Thanks, > Tamar > > gcc/ChangeLog: > > * cfgexpand.cc (set_rtl): Check for function promotion. > * tree-outof-ssa.cc (insert_value_copy_on_edge): Likewise. > * function.cc (assign_parm_setup_reg): Likewise. > * hooks.cc (hook_bool_mode_mode_int_tree_false, > hook_bool_mode_mode_int_tree_true): New. > * hooks.h (hook_bool_mode_mode_int_tree_false, > hook_bool_mode_mode_int_tree_true): New. > * target.def (promote_function_args_subreg_p): New. > * doc/tm.texi: Document it. > * doc/tm.texi.in: Likewise. > > --- inline copy of patch -- > diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc > index d3cc77d2ca98f620b29623fc5696410bad9bc184..df95184cfa185312c2a46cb92daa051718d9f4f3 100644 > --- a/gcc/cfgexpand.cc > +++ b/gcc/cfgexpand.cc > @@ -206,14 +206,20 @@ set_rtl (tree t, rtx x) > have to compute it ourselves. For RESULT_DECLs, we accept mode > mismatches too, as long as we have BLKmode or are not coalescing > across variables, so that we don't reject BLKmode PARALLELs or > - unpromoted REGs. */ > + unpromoted REGs. For any promoted types that result in a > + paradoxical subreg also accept the argument. */ > gcc_checking_assert (!x || x == pc_rtx || TREE_CODE (t) != SSA_NAME > || (SSAVAR (t) > && TREE_CODE (SSAVAR (t)) == RESULT_DECL > && (promote_ssa_mode (t, NULL) == BLKmode > || !flag_tree_coalesce_vars)) > || !use_register_for_decl (t) > - || GET_MODE (x) == promote_ssa_mode (t, NULL)); > + || GET_MODE (x) == promote_ssa_mode (t, NULL) > + || targetm.calls.promote_function_args_subreg_p ( > + GET_MODE (x), > + promote_ssa_mode (t, NULL), > + TYPE_UNSIGNED (TREE_TYPE (t)), > + SSAVAR (t))); > > if (x) > { > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi > index 2f92d37da8c0091e9879a493cfe8a361eb1d9299..6314cd83a2488dc225d4a1a15599e8e51e639f7f 100644 > --- a/gcc/doc/tm.texi > +++ b/gcc/doc/tm.texi > @@ -3906,6 +3906,15 @@ cases of mismatch, it also makes for better code on certain machines. > The default is to not promote prototypes. > @end deftypefn > > +@deftypefn {Target Hook} bool TARGET_PROMOTE_FUNCTION_ARGS_SUBREG_P (machine_mode @var{mode}, machine_mode @var{promoted_mode}, int @var{unsignedp}, tree @var{v}) > +When a function argument is promoted with @code{PROMOTE_MODE} then this > +hook is used to determine whether the bits of the promoted type are all > +significant in the expression pointed to by V. If they are an extend is > +generated, if they are not a paradoxical subreg is created for the argument > +from @code{mode} to @code{promoted_mode}. > +The default is to promote using an extend. > +@end deftypefn > + > @deftypefn {Target Hook} bool TARGET_PUSH_ARGUMENT (unsigned int @var{npush}) > This target hook returns @code{true} if push instructions will be > used to pass outgoing arguments. When the push instruction usage is > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in > index f869ddd5e5b8b7acbd8e9765fb103af24a1085b6..35f955803ec0a5a93be18a028fa1043f90858982 100644 > --- a/gcc/doc/tm.texi.in > +++ b/gcc/doc/tm.texi.in > @@ -3103,6 +3103,8 @@ control passing certain arguments in registers. > > @hook TARGET_PROMOTE_PROTOTYPES > > +@hook TARGET_PROMOTE_FUNCTION_ARGS_SUBREG_P > + > @hook TARGET_PUSH_ARGUMENT > > @defmac PUSH_ARGS_REVERSED > diff --git a/gcc/function.cc b/gcc/function.cc > index d5ed51a6a663a1ef472f5b1c090543f359c18f42..92f469bfd5d1ebfb09cc94d9be854715cd2f90f8 100644 > --- a/gcc/function.cc > +++ b/gcc/function.cc > @@ -3161,7 +3161,7 @@ assign_parm_setup_reg (struct assign_parm_data_all *all, tree parm, > machine_mode promoted_nominal_mode; > int unsignedp = TYPE_UNSIGNED (TREE_TYPE (parm)); > bool did_conversion = false; > - bool need_conversion, moved; > + bool need_conversion, moved, use_subregs; > enum insn_code icode; > rtx rtl; > > @@ -3172,7 +3172,20 @@ assign_parm_setup_reg (struct assign_parm_data_all *all, tree parm, > = promote_function_mode (data->nominal_type, data->nominal_mode, &unsignedp, > TREE_TYPE (current_function_decl), 2); > > - parmreg = gen_reg_rtx (promoted_nominal_mode); > + /* Check to see how the target wants the promotion of function arguments to > + be handled. */ > + use_subregs > + = targetm.calls.promote_function_args_subreg_p (data->nominal_mode, > + promoted_nominal_mode, > + unsignedp, parm); > + > + /* If we're promoting using a paradoxical subreg then we need to keep using > + the unpromoted type because that's the only fully defined value. */ > + if (use_subregs) > + parmreg = gen_reg_rtx (data->nominal_mode); > + else > + parmreg = gen_reg_rtx (promoted_nominal_mode); > + > if (!DECL_ARTIFICIAL (parm)) > mark_user_reg (parmreg); > > @@ -3256,9 +3269,19 @@ assign_parm_setup_reg (struct assign_parm_data_all *all, tree parm, > } > else > t = op1; > - rtx_insn *pat = gen_extend_insn (op0, t, promoted_nominal_mode, > - data->passed_mode, unsignedp); > - emit_insn (pat); > + > + /* Promote the argument itself now if a target wants it. This > + prevents unneeded back and forth convertions in RTL between > + the original and promoted type. */ > + if (use_subregs) > + emit_move_insn (op0, lowpart_subreg (promoted_nominal_mode, t, > + data->nominal_mode)); > + else > + { > + rtx_insn *pat = gen_extend_insn (op0, t, promoted_nominal_mode, > + data->passed_mode, unsignedp); > + emit_insn (pat); > + } > insns = get_insns (); > > moved = true; > diff --git a/gcc/hooks.h b/gcc/hooks.h > index 1056e1e9e4dc3e6ce298557351047caa2f84227f..8d68de5cdb9adaea0a79ebf6de599f66b40aa67a 100644 > --- a/gcc/hooks.h > +++ b/gcc/hooks.h > @@ -31,6 +31,8 @@ extern bool hook_bool_const_int_const_int_true (const int, const int); > extern bool hook_bool_mode_false (machine_mode); > extern bool hook_bool_mode_true (machine_mode); > extern bool hook_bool_mode_mode_true (machine_mode, machine_mode); > +extern bool hook_bool_mode_mode_int_tree_false (machine_mode, machine_mode, int, tree); > +extern bool hook_bool_mode_mode_int_tree_true (machine_mode, machine_mode, int, tree); > extern bool hook_bool_mode_const_rtx_false (machine_mode, const_rtx); > extern bool hook_bool_mode_const_rtx_true (machine_mode, const_rtx); > extern bool hook_bool_mode_rtx_false (machine_mode, rtx); > diff --git a/gcc/hooks.cc b/gcc/hooks.cc > index b29233f4f852fb81ede75a5065d743cd16cc9219..7647774f9e8efbbe13d5607e4a4b2f1c9d22f045 100644 > --- a/gcc/hooks.cc > +++ b/gcc/hooks.cc > @@ -89,6 +89,22 @@ hook_bool_mode_mode_true (machine_mode, machine_mode) > return true; > } > > +/* Generic hook that takes (machine_mode, machine_mode, int, tree) and > + returns false. */ > +bool > +hook_bool_mode_mode_int_tree_false (machine_mode, machine_mode, int, tree) > +{ > + return false; > +} > + > +/* Generic hook that takes (machine_mode, machine_mode, int, tree) and > + returns true. */ > +bool > +hook_bool_mode_mode_int_tree_true (machine_mode, machine_mode, int, tree) > +{ > + return true; > +} > + > /* Generic hook that takes (machine_mode, const_rtx) and returns false. */ > bool > hook_bool_mode_const_rtx_false (machine_mode, const_rtx) > diff --git a/gcc/target.def b/gcc/target.def > index 72c2e1ef756cf70a1c92abe81f8a6577eaaa2501..bdbacf8c5fd7b0626a37951f6f6ec649f3194977 100644 > --- a/gcc/target.def > +++ b/gcc/target.def > @@ -4561,6 +4561,17 @@ The default is to not promote prototypes.", > bool, (const_tree fntype), > hook_bool_const_tree_false) > > +DEFHOOK > +(promote_function_args_subreg_p, > + "When a function argument is promoted with @code{PROMOTE_MODE} then this\n\ > +hook is used to determine whether the bits of the promoted type are all\n\ > +significant in the expression pointed to by V. If they are an extend is\n\ > +generated, if they are not a paradoxical subreg is created for the argument\n\ > +from @code{mode} to @code{promoted_mode}.\n\ > +The default is to promote using an extend.", > + bool, (machine_mode mode, machine_mode promoted_mode, int unsignedp, tree v), > + hook_bool_mode_mode_int_tree_false) > + > DEFHOOK > (struct_value_rtx, > "This target hook should return the location of the structure value\n\ > diff --git a/gcc/tree-outof-ssa.cc b/gcc/tree-outof-ssa.cc > index ec883126ad86d86a2c2dafee4592b8d83e2ed917..0f437023983baa0f23da25221f7bce8fc559a8b8 100644 > --- a/gcc/tree-outof-ssa.cc > +++ b/gcc/tree-outof-ssa.cc > @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3. If not see > #include "tree-ssa-coalesce.h" > #include "tree-outof-ssa.h" > #include "dojump.h" > +#include "target.h" > > /* FIXME: A lot of code here deals with expanding to RTL. All that code > should be in cfgexpand.cc. */ > @@ -333,7 +334,10 @@ insert_value_copy_on_edge (edge e, int dest, tree src, location_t locus) > dest_mode = GET_MODE (dest_rtx); > gcc_assert (src_mode == TYPE_MODE (TREE_TYPE (name))); > gcc_assert (!REG_P (dest_rtx) > - || dest_mode == promote_ssa_mode (name, &unsignedp)); > + || dest_mode == promote_ssa_mode (name, &unsignedp) > + || targetm.calls.promote_function_args_subreg_p ( > + dest_mode, promote_ssa_mode (name, NULL), unsignedp, > + name)); > > if (src_mode != dest_mode) > { > > > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)