> -----Original Message-----
> From: Richard Biener <rguent...@suse.de>
> Sent: Monday, May 16, 2022 7:31 AM
> To: Tamar Christina <tamar.christ...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; jeffreya...@gmail.com;
> Richard Sandiford <richard.sandif...@arm.com>
> Subject: Re: [PATCH 1/3]middle-end: Add the ability to let the target decide
> the method of argument promotions.
>
> 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?
>
Because I'm not sure it's allowed to. From what I can tell function_arg expects
return registers to be hardregs. i.e. the actual APCS determined register for
the
argument. And since it's a hardreg can't make it a paradoxical subreg, it'll
just
resize the register to the new mode.
assign_parm_setup_reg gets around this by creating a new pseudo and then
assigning the resulting rtl to the param set_parm_rtl (parm, rtl); which as far
as I can tell changes what expand will expand the parm to without actually
changing the actual APCS register. Which is why I think the current code does
the promotions there.
> 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.
I could perhaps modify function_arg_info to have a field for the subreg
extension
which I can then use in function_arg for the promotion. However I'll have a
problem
with the asserts in insert_value_copy_on_edge and set_rtl both of which check
that
either the in/out types match, or the out type is a promoted in type.
set_rtl I can extend with enough information to determine whether the subreg was
intentional but insert_value_copy_on_edge doesn't give me access to enough
information..
Which is probably why the current code re-queries the target.. Can I get to
the parm information
Here?
I also initially tried to extend PROMOTE_MODE itself, however this is used by
promote_mode, which
is used in many other places and I am not sure some of these usages are safe to
change, such as
promote_ssa_mode.
The new hook I know only interacts with parms. Any thoughts appreciated 😊
Cheers,
Tamar
>
> 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..df95184cfa185312c2a46cb92da
> a
> > 051718d9f4f3 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..6314cd83a2488dc225d4a1a15
> 599
> > e8e51e639f7f 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..35f955803ec0a5a93be18a028
> fa1
> > 043f90858982 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..92f469bfd5d1ebfb09cc94d9be
> 85
> > 4715cd2f90f8 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..8d68de5cdb9adaea0a79ebf6d
> e59
> > 9f66b40aa67a 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..7647774f9e8efbbe13d5607e4
> a4b
> > 2f1c9d22f045 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..bdbacf8c5fd7b0626a37951f6f6
> e
> > c649f3194977 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..0f437023983baa0f23da25221
> f7b
> > ce8fc559a8b8 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)
