On Tue, 1 Apr 2025, Jakub Jelinek wrote:
> On Tue, Apr 01, 2025 at 10:59:36AM +0200, Jakub Jelinek wrote:
> > On Tue, Apr 01, 2025 at 10:46:15AM +0200, Richard Biener wrote:
> > > This looks OK, but I wonder if ...
> > > > - /* The parameter should be a real operand, so that phi
> > > > node
> > > > - created for it at the start of the function has the
> > > > meaning
> > > > - of copying the value. This test implies
> > > > is_gimple_reg_type
> > > > - from the previous condition, however this one could be
> > > > - relaxed by being more careful with copying the new
> > > > value
> > > > - of the parameter (emitting appropriate GIMPLE_ASSIGN
> > > > and
> > > > - updating the virtual operands). */
> > > > - if (!is_gimple_reg (param))
> > > > + if (is_gimple_reg_type (TREE_TYPE (param))
> > > > + ? !is_gimple_reg (param)
> > >
> > > ... we want to restrict this to musttail calls at this point and
> > > relax for stage1 only?
> >
> > I can do that, sure.
>
> Here it is, ok if it passes bootstrap/regtest? I'll queue the interdiff
> between this patch and the previous one for GCC 16.
OK.
Thanks,
Richard.
> 2025-04-01 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/119493
> * tree-tailcall.cc (find_tail_calls): Don't punt on tail recusion
> if some arguments don't have is_gimple_reg_type, only punt if they
> have non-POD types, or volatile, or addressable or (for now) it is
> not a musttail call. Set tailr_arg_needs_copy in those cases too.
> (eliminate_tail_call): Copy call arguments to params if they don't
> have is_gimple_reg_type, use temporaries if the argument is used
> later.
> (tree_optimize_tail_calls_1): Skip !is_gimple_reg_type
> tailr_arg_needs_copy parameters. Formatting fix.
>
> * gcc.dg/pr119493-1.c: New test.
>
> --- gcc/tree-tailcall.cc.jj 2025-03-28 10:49:30.000000000 +0100
> +++ gcc/tree-tailcall.cc 2025-04-01 11:47:31.417637335 +0200
> @@ -676,19 +676,17 @@ find_tail_calls (basic_block bb, struct
> have a copyable type and the two arguments must have reasonably
> equivalent types. The latter requirement could be relaxed if
> we emitted a suitable type conversion statement. */
> - if (!is_gimple_reg_type (TREE_TYPE (param))
> + if (TREE_ADDRESSABLE (TREE_TYPE (param))
> || !useless_type_conversion_p (TREE_TYPE (param),
> TREE_TYPE (arg)))
> break;
>
> - /* The parameter should be a real operand, so that phi node
> - created for it at the start of the function has the meaning
> - of copying the value. This test implies is_gimple_reg_type
> - from the previous condition, however this one could be
> - relaxed by being more careful with copying the new value
> - of the parameter (emitting appropriate GIMPLE_ASSIGN and
> - updating the virtual operands). */
> - if (!is_gimple_reg (param))
> + if (is_gimple_reg_type (TREE_TYPE (param))
> + ? !is_gimple_reg (param)
> + : (!is_gimple_variable (param)
> + || TREE_THIS_VOLATILE (param)
> + || may_be_aliased (param)
> + || !gimple_call_must_tail_p (call)))
> break;
> }
> }
> @@ -938,9 +936,9 @@ find_tail_calls (basic_block bb, struct
> param = DECL_CHAIN (param), idx++)
> {
> tree ddef, arg = gimple_call_arg (call, idx);
> - if (is_gimple_reg (param)
> - && (ddef = ssa_default_def (cfun, param))
> - && (arg != ddef))
> + if (!is_gimple_reg (param)
> + || ((ddef = ssa_default_def (cfun, param))
> + && arg != ddef))
> bitmap_set_bit (tailr_arg_needs_copy, idx);
> }
> }
> @@ -1216,6 +1214,7 @@ eliminate_tail_call (struct tailcall *t,
>
> /* Add phi node entries for arguments. The ordering of the phi nodes
> should
> be the same as the ordering of the arguments. */
> + auto_vec<tree> copies;
> for (param = DECL_ARGUMENTS (current_function_decl),
> idx = 0, gpi = gsi_start_phis (first);
> param;
> @@ -1224,6 +1223,35 @@ eliminate_tail_call (struct tailcall *t,
> if (!bitmap_bit_p (tailr_arg_needs_copy, idx))
> continue;
>
> + if (!is_gimple_reg_type (TREE_TYPE (param)))
> + {
> + if (param == gimple_call_arg (stmt, idx))
> + continue;
> + /* First check if param isn't used by any of the following
> + call arguments. If it is, we need to copy first to
> + a temporary and only after doing all the assignments copy it
> + to param. */
> + size_t idx2 = idx + 1;
> + tree param2 = DECL_CHAIN (param);
> + for (; param2; param2 = DECL_CHAIN (param2), idx2++)
> + if (!is_gimple_reg_type (TREE_TYPE (param)))
> + {
> + tree base = get_base_address (gimple_call_arg (stmt, idx2));
> + if (base == param)
> + break;
> + }
> + tree tmp = param;
> + if (param2)
> + {
> + tmp = create_tmp_var (TREE_TYPE (param));
> + copies.safe_push (param);
> + copies.safe_push (tmp);
> + }
> + gimple *g = gimple_build_assign (tmp, gimple_call_arg (stmt, idx));
> + gsi_insert_before (&t->call_gsi, g, GSI_SAME_STMT);
> + continue;
> + }
> +
> arg = gimple_call_arg (stmt, idx);
> phi = gpi.phi ();
> gcc_assert (param == SSA_NAME_VAR (PHI_RESULT (phi)));
> @@ -1231,6 +1259,11 @@ eliminate_tail_call (struct tailcall *t,
> add_phi_arg (phi, arg, e, gimple_location (stmt));
> gsi_next (&gpi);
> }
> + for (unsigned i = 0; i < copies.length (); i += 2)
> + {
> + gimple *g = gimple_build_assign (copies[i], copies[i + 1]);
> + gsi_insert_before (&t->call_gsi, g, GSI_SAME_STMT);
> + }
>
> /* Update the values of accumulators. */
> adjust_accumulator_values (t->call_gsi, t->mult, t->add, e);
> @@ -1390,8 +1423,8 @@ tree_optimize_tail_calls_1 (bool opt_tai
> or if there are existing degenerate PHI nodes. */
> if (!single_pred_p (first)
> || !gimple_seq_empty_p (phi_nodes (first)))
> - first =
> - split_edge (single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
> + first
> + = split_edge (single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
>
> /* Copy the args if needed. */
> unsigned idx;
> @@ -1400,6 +1433,8 @@ tree_optimize_tail_calls_1 (bool opt_tai
> param = DECL_CHAIN (param), idx++)
> if (bitmap_bit_p (tailr_arg_needs_copy, idx))
> {
> + if (!is_gimple_reg_type (TREE_TYPE (param)))
> + continue;
> tree name = ssa_default_def (cfun, param);
> tree new_name = make_ssa_name (param, SSA_NAME_DEF_STMT (name));
> gphi *phi;
> --- gcc/testsuite/gcc.dg/pr119493-1.c.jj 2025-03-31 12:37:08.194529478
> +0200
> +++ gcc/testsuite/gcc.dg/pr119493-1.c 2025-03-31 12:45:10.583848794 +0200
> @@ -0,0 +1,55 @@
> +/* PR tree-optimization/119493 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fdump-tree-tailr1" } */
> +/* { dg-final { scan-tree-dump-times " = foo \\\(\[^\n\r;]*\\\);" 4 "tailr1"
> } } */
> +/* { dg-final { scan-tree-dump-times " = bar \\\(\[^\n\r;]*\\\);" 4 "tailr1"
> } } */
> +/* { dg-final { scan-tree-dump-not " = foo \\\(\[^\n\r;]*\\\); \\\[must tail
> call\\\]" "tailr1" } } */
> +/* { dg-final { scan-tree-dump-not " = bar \\\(\[^\n\r;]*\\\); \\\[must tail
> call\\\]" "tailr1" } } */
> +
> +struct S { unsigned s; };
> +struct T { struct S t[2]; };
> +
> +[[gnu::noinline, gnu::noclone]] struct S
> +foo (struct S m)
> +{
> + if (m.s == 0 || m.s == 42)
> + return m;
> + [[gnu::musttail]] return foo ((struct S) { m.s - 1 });
> +}
> +
> +[[gnu::noinline, gnu::noclone]] struct S
> +bar (struct T m, struct S n, int o, int p, int q)
> +{
> + struct T r;
> + if (m.t[1].s != o || n.s != o)
> + __builtin_abort ();
> + if (o == 0 || o == 42)
> + return n;
> + r = m;
> + m.t[1].s -= p;
> + r.t[1].s -= q;
> + [[gnu::musttail]] return bar (r, m.t[1], o - 1, p, q);
> +}
> +
> +int
> +main ()
> +{
> + if (foo ((struct S) { 0 }).s != 0)
> + __builtin_abort ();
> + if (foo ((struct S) { 4 }).s != 0)
> + __builtin_abort ();
> + if (foo ((struct S) { 42 }).s != 42)
> + __builtin_abort ();
> + if (foo ((struct S) { 51 }).s != 42)
> + __builtin_abort ();
> + if (bar ((struct T) { { { 0 }, { 0 } } }, (struct S) { 0 }, 0, 1, 1).s !=
> 0)
> + __builtin_abort ();
> + if (bar ((struct T) { { { 7 }, { 7 } } }, (struct S) { 7 }, 7, 1, 1).s !=
> 0)
> + __builtin_abort ();
> + if (bar ((struct T) { { { 42 }, { 42 } } },
> + (struct S) { 42 }, 42, 1, 1).s != 42)
> + __builtin_abort ();
> + if (bar ((struct T) { { { 48 }, { 48 } } },
> + (struct S) { 48 }, 48, 1, 1).s != 42)
> + __builtin_abort ();
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
