On Mon, 7 Apr 2025, Jakub Jelinek wrote:
> Hi!
>
> The IPA-VRP workaround in the tailc/musttail passes was just comparing
> the singleton constant from a tail call candidate return with the ret_val.
> This unfortunately doesn't work in the following testcase, where we have
> <bb 5> [local count: 152205050]:
> baz (); [must tail call]
> goto <bb 4>; [100.00%]
>
> <bb 6> [local count: 762356696]:
> _8 = foo ();
>
> <bb 7> [local count: 1073741824]:
> # _3 = PHI <0B(4), _8(6)>
> return _3;
> and in the unreduced testcase even more PHIs before we reach the return
> stmt.
>
> Normally when the call has lhs, whenever we follow a (non-EH) successor
> edge, it calls propagate_through_phis and that walks the PHIs in the
> destination bb of the edge and when it sees a PHI whose argument matches
> that of the currently tracked value (ass_var), it updates ass_var to
> PHI result of that PHI. I think it is theoretically dangerous that it
> picks the first one, perhaps there could be multiple PHIs, so perhaps safer
> would be walk backwards from the return value up to the call.
>
> Anyway, this PR is about the IPA-VRP workaround, there ass_var is NULL
> because the potential tail call has no lhs, but ret_var is not TREE_CONSTANT
> but SSA_NAME with PHI as SSA_NAME_DEF_STMT. The following patch handles
> it by pushing the edges we've walked through when ass_var is NULL into a
> vector and if ret_var is SSA_NAME set to PHI result, it attempts to walk
> back from the ret_var through arguments of PHIs corresponding to the
> edges we've walked back until we reach a constant and compare that constant
> against the singleton value as well.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
I do wonder with all these patches whether it would be better to
preserve the LHS on musttail calls instead?
> 2025-04-07 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/119614
> * tree-tailcall.cc (find_tail_calls): Remember edges which have been
> walked through if !ass_var. Perform IPA-VRP workaround even when
> ret_var is not TREE_CONSTANT, in that case check in a loop if it is
> a PHI result and in that case look at the PHI argument from
> corresponding edge in the edge vector.
>
> * g++.dg/opt/pr119613.C: Change { c || c++11 } in obviously C++ only
> test to just c++11.
> * g++.dg/opt/pr119614.C: New test.
>
> --- gcc/tree-tailcall.cc.jj 2025-04-04 20:52:34.450015821 +0200
> +++ gcc/tree-tailcall.cc 2025-04-05 14:50:50.106693562 +0200
> @@ -920,6 +920,7 @@ find_tail_calls (basic_block bb, struct
> auto_bitmap to_move_defs;
> auto_vec<gimple *> to_move_stmts;
> bool is_noreturn = gimple_call_noreturn_p (call);
> + auto_vec<edge> edges;
>
> abb = bb;
> agsi = gsi;
> @@ -933,6 +934,8 @@ find_tail_calls (basic_block bb, struct
> {
> edge e = single_non_eh_succ_edge (abb);
> ass_var = propagate_through_phis (ass_var, e);
> + if (!ass_var)
> + edges.safe_push (e);
> abb = e->dest;
> agsi = gsi_start_bb (abb);
> }
> @@ -1040,9 +1043,7 @@ find_tail_calls (basic_block bb, struct
> /* If IPA-VRP proves called function always returns a singleton range,
> the return value is replaced by the only value in that range.
> For tail call purposes, pretend such replacement didn't happen. */
> - if (ass_var == NULL_TREE
> - && !tail_recursion
> - && TREE_CONSTANT (ret_var))
> + if (ass_var == NULL_TREE && !tail_recursion)
> if (tree type = gimple_range_type (call))
> if (tree callee = gimple_call_fndecl (call))
> if ((INTEGRAL_TYPE_P (type)
> @@ -1052,9 +1053,43 @@ find_tail_calls (basic_block bb, struct
> type)
> && useless_type_conversion_p (TREE_TYPE (ret_var), type)
> && ipa_return_value_range (val, callee)
> - && val.singleton_p (&valr)
> - && operand_equal_p (ret_var, valr, 0))
> - ok = true;
> + && val.singleton_p (&valr))
> + {
> + tree rv = ret_var;
> + unsigned int i = edges.length ();
> + /* If ret_var is equal to valr, we can tail optimize. */
> + if (operand_equal_p (ret_var, valr, 0))
> + ok = true;
> + else
> + /* Otherwise, if ret_var is a PHI result, try to find out
> + if valr isn't propagated through PHIs on the path from
> + call's bb to SSA_NAME_DEF_STMT (ret_var)'s bb. */
> + while (TREE_CODE (rv) == SSA_NAME
> + && gimple_code (SSA_NAME_DEF_STMT (rv)) == GIMPLE_PHI)
> + {
> + tree nrv = NULL_TREE;
> + gimple *g = SSA_NAME_DEF_STMT (rv);
> + for (; i; --i)
> + {
> + if (edges[i - 1]->dest == gimple_bb (g))
> + {
> + nrv
> + = gimple_phi_arg_def_from_edge (g,
> + edges[i - 1]);
> + --i;
> + break;
> + }
> + }
> + if (nrv == NULL_TREE)
> + break;
> + if (operand_equal_p (nrv, valr, 0))
> + {
> + ok = true;
> + break;
> + }
> + rv = nrv;
> + }
> + }
> if (!ok)
> {
> maybe_error_musttail (call,
> --- gcc/testsuite/g++.dg/opt/pr119613.C.jj 2025-04-04 20:51:42.482706589
> +0200
> +++ gcc/testsuite/g++.dg/opt/pr119613.C 2025-04-05 14:57:31.157353618
> +0200
> @@ -1,5 +1,5 @@
> // PR middle-end/119613
> -// { dg-do compile { target { musttail && { c || c++11 } } } }
> +// { dg-do compile { target { musttail && c++11 } } }
> // { dg-options "-O0" }
>
> struct S { S () {} };
> --- gcc/testsuite/g++.dg/opt/pr119614.C.jj 2025-04-05 14:57:16.276551780
> +0200
> +++ gcc/testsuite/g++.dg/opt/pr119614.C 2025-04-05 14:56:46.020954674
> +0200
> @@ -0,0 +1,30 @@
> +// PR tree-optimization/119614
> +// { dg-do compile { target musttail } }
> +// { dg-options "-O2" }
> +
> +struct S {} b;
> +char *foo ();
> +int e, g;
> +void bar ();
> +void corge (S);
> +
> +[[gnu::noinline]] char *
> +baz ()
> +{
> + bar ();
> + return 0;
> +}
> +
> +const char *
> +qux ()
> +{
> + if (e)
> + {
> + S a = b;
> + corge (a);
> + if (g)
> + return 0;
> + [[gnu::musttail]] return baz ();
> + }
> + return foo ();
> +}
>
> 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)
