On Wed, Mar 19, 2025 at 6:34 PM 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
Note I think you need my email address here to pass verification of
the hooks, quic_apin...@quicinc.com .
>
> 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)
> 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))
The only question I have is flow_call_edges_add only called while
profiling or is it called some other time? So looking into who calls
flow_call_edges_add, it is only branch_prob (profile.cc) which is only
called from tree-profile.cc. So a cleanup (for GCC 16 is remove the
cfghook flow_call_edges_add and call gimple_flow_call_edges_add
directly from branch_prob).
Plus blocks argument is always NULL so we could even remove support
for that too.
So this LGTM; though I can't approve it.
Thanks,
Andrew Pinski
> {
> edge e;
>
> --
> 2.47.1
>
