On Thu, Mar 20, 2025 at 2:34 AM Andi Kleen <a...@firstfloor.org> wrote:
>
> From: Andi Kleen <a...@gcc.gnu.org>
>
> When -fprofile-generate is used musttail often fails because the
> compiler adds instrumentation after the tail calls.
>
> This patch prevents adding exit extra edges after musttail because for a
> tail call the execution leaves the function and can never come back
> even on a unwind or exception.
>
> This is only done for musttail. In principle it could be done
> for any tail calls, but the problem is that there might be other reasons
> why the tail fails later, and then the edge might be still needed.
>
> Passes bootstrap and full testing on x86_64 with no new failures.
>
> [v2 version: Don't affect stmt_can_terminate_bb_p. Fix changelog.
> Adjust test case for clang:: removal.]
>
> Co-authored-by: Andrew Pinski
>
> gcc/ChangeLog:
>
> PR gcov-profile/118442
> * tree-cfg.cc (must_tail_call_p): New function.
> (gimple_flow_call_edges_add): Avoid exit edges for musttail
> calls.
>
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/pr118442.c: New test.
> ---
> gcc/testsuite/c-c++-common/pr118442.c | 15 +++++++++++++++
> gcc/tree-cfg.cc | 16 ++++++++++++++--
> 2 files changed, 29 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/c-c++-common/pr118442.c
>
> diff --git a/gcc/testsuite/c-c++-common/pr118442.c
> b/gcc/testsuite/c-c++-common/pr118442.c
> new file mode 100644
> index 00000000000..24b7f546c98
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/pr118442.c
> @@ -0,0 +1,15 @@
> +/* PR118442 */
> +/* { dg-do compile { target { struct_musttail && { c || c++11 } } } } */
> +/* { dg-options "-fprofile-generate -O2" } */
> +
> +struct Span {
> + int test[5];
> +};
> +
> +void resolveToBufferSlow(struct Span *buffer);
> +
> +void resolveToBuffer(struct Span *buffer)
> +{
> + buffer->test[0] = 4;
> + [[gnu::musttail]] return resolveToBufferSlow(buffer);
> +}
> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> index 2fa5678051a..85c8082c297 100644
> --- a/gcc/tree-cfg.cc
> +++ b/gcc/tree-cfg.cc
> @@ -8894,6 +8894,18 @@ stmt_can_terminate_bb_p (gimple *t)
> return false;
> }
>
> +/* Is T a gimple must tail call? */
> +
> +static bool
> +must_tail_call_p (gimple *t)
> +{
> + /* When it's a enforced tail call there is no edge because
> + the control flow has left the function and can never come back.
> + This prevents instrumenting the edge which would make the must
> + tail call fail. */
> + return is_gimple_call (t)
> + && gimple_call_must_tail_p (as_a <const gcall *> (t));
> +}
>
> /* Add fake edges to the function exit for any non constant and non
> noreturn calls (or noreturn calls with EH/abnormal edges),
> @@ -8942,7 +8954,7 @@ gimple_flow_call_edges_add (sbitmap blocks)
Hmm. I do wonder whether your earlier patch was more "correct" in the
sense that a tail call does not return to the calling function but its caller.
That means it should not have a fallthru edge, so our representation
with feeding a return value to a function-local return stmt isn't a good one.
That said, the CFG hook you are patching is supposed to add edges when
a function call can possibly not reach the return stmt and I think that's
the case for tail-calls. So IMO the patch is wrong. Iff instead the
instrumentation confuses later tail-call verification then this instrumentation
should be either not emitted or handled there.
It seems the hook is only used from GIMPLE, so the hookizing could be
undone and the code moved to profile.cc where then special-casing it
in this function would be OK. You'll see there
flow_call_edges_add (NULL);
add_noreturn_fake_exit_edges ();
so it adds noreturn edges that flow_call_edges_add avoids to add. Pruning
edges added for tail-calls here might be another option then.
Richard.
> if (!gsi_end_p (gsi))
> t = gsi_stmt (gsi);
>
> - if (t && stmt_can_terminate_bb_p (t))
> + if (t && stmt_can_terminate_bb_p (t) && !must_tail_call_p (t))
> {
> edge e;
>
> @@ -8977,7 +8989,7 @@ gimple_flow_call_edges_add (sbitmap blocks)
> do
> {
> stmt = gsi_stmt (gsi);
> - if (stmt_can_terminate_bb_p (stmt))
> + if (stmt_can_terminate_bb_p (stmt) && !must_tail_call_p (stmt))
> {
> edge e;
>
> --
> 2.47.1
>
