From: Martin Jambor <mjambor@suse.cz>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: Jan Hubicka <hubicka@ucw.cz>
Subject: Re: [PATCH] ipa: Avoid excessive removing of SSAs (PR 113757)
Date: Thu, 07 Mar 2024 11:25:04 +0100 [thread overview]
Message-ID: <ri6edcm1gqn.fsf@virgil.suse.cz> (raw)
In-Reply-To: <ri6bk8r5kfi.fsf@virgil.suse.cz>
Hello,
and ping please.
Martin
On Thu, Feb 08 2024, Martin Jambor wrote:
> Hi,
>
> PR 113757 shows that the code which was meant to debug-reset and
> remove SSAs defined by LHSs of calls redirected to
> __builtin_unreachable can trigger also when speculative
> devirtualization creates a call to a noreturn function (and since it
> is noreturn, it does not bother dealing with its return value).
>
> What is more, it seems that the code handling this case is not really
> necessary. I feel slightly idiotic about this because I have a
> feeling that I added it because of a failing test-case but I can
> neither find the testcase nor a reason why the code in
> cgraph_edge::redirect_call_stmt_to_callee would not be sufficient (it
> turns the SSA name into a default-def, a bit like IPA-SRA, but any
> code dominated by a call to a noreturn is not dangerous when it comes
> to its side-effects). So this patch just removes the handling.
>
> Bootstrapped and tested on x86_64-linux and ppc64le-linux. I have also
> LTO-bootstrapped and LTO-profilebootstrapped the patch on x86_64-linux.
>
> OK for master?
>
> Thanks,
>
> Martin
>
>
> gcc/ChangeLog:
>
> 2024-02-07 Martin Jambor <mjambor@suse.cz>
>
> PR ipa/113757
> * tree-inline.cc (redirect_all_calls): Remove code adding SSAs to
> id->killed_new_ssa_names.
>
> gcc/testsuite/ChangeLog:
>
> 2024-02-07 Martin Jambor <mjambor@suse.cz>
>
> PR ipa/113757
> * g++.dg/ipa/pr113757.C: New test.
> ---
> gcc/testsuite/g++.dg/ipa/pr113757.C | 14 ++++++++++++++
> gcc/tree-inline.cc | 14 ++------------
> 2 files changed, 16 insertions(+), 12 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/ipa/pr113757.C
>
> diff --git a/gcc/testsuite/g++.dg/ipa/pr113757.C b/gcc/testsuite/g++.dg/ipa/pr113757.C
> new file mode 100644
> index 00000000000..885d4010a10
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/pr113757.C
> @@ -0,0 +1,14 @@
> +// { dg-do compile }
> +// { dg-options "-O2 -fPIC" }
> +// { dg-require-effective-target fpic }
> +
> +long size();
> +struct ll { virtual int hh(); };
> +ll *slice_owner;
> +int ll::hh() { __builtin_exit(0); }
> +int nn() {
> + if (size())
> + return 0;
> + return slice_owner->hh();
> +}
> +int (*a)() = nn;
> diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
> index 75c10eb7dfc..cac41b4f031 100644
> --- a/gcc/tree-inline.cc
> +++ b/gcc/tree-inline.cc
> @@ -2984,23 +2984,13 @@ redirect_all_calls (copy_body_data * id, basic_block bb)
> gimple *stmt = gsi_stmt (si);
> if (is_gimple_call (stmt))
> {
> - tree old_lhs = gimple_call_lhs (stmt);
> struct cgraph_edge *edge = id->dst_node->get_edge (stmt);
> if (edge)
> {
> if (!id->killed_new_ssa_names)
> id->killed_new_ssa_names = new hash_set<tree> (16);
> - gimple *new_stmt
> - = cgraph_edge::redirect_call_stmt_to_callee (edge,
> - id->killed_new_ssa_names);
> - if (old_lhs
> - && TREE_CODE (old_lhs) == SSA_NAME
> - && !gimple_call_lhs (new_stmt))
> - /* In case of IPA-SRA removing the LHS, the name should have
> - been already added to the hash. But in case of redirecting
> - to builtin_unreachable it was not and the name still should
> - be pruned from debug statements. */
> - id->killed_new_ssa_names->add (old_lhs);
> + cgraph_edge::redirect_call_stmt_to_callee (edge,
> + id->killed_new_ssa_names);
>
> if (stmt == last && id->call_stmt && maybe_clean_eh_stmt (stmt))
> gimple_purge_dead_eh_edges (bb);
> --
> 2.43.0
next prev parent reply other threads:[~2024-03-07 11:10 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-08 10:17 Martin Jambor
2024-03-07 10:25 ` Martin Jambor [this message]
2024-03-07 16:03 ` Jan Hubicka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ri6edcm1gqn.fsf@virgil.suse.cz \
--to=mjambor@suse.cz \
--cc=gcc-patches@gcc.gnu.org \
--cc=hubicka@ucw.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).