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 <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
