From: Richard Biener <rguenther@suse.de>
To: gcc-patches@gcc.gnu.org
Cc: Jakub Jelinek <jakub@redhat.com>
Subject: [PATCH] tree-optimization/108691 - indirect calls to setjmp
Date: Mon, 13 Feb 2023 12:00:56 +0100 (CET) [thread overview]
Message-ID: <20230213110058.0C9B61391B@imap2.suse-dmz.suse.de> (raw)
DCE now chokes on indirect setjmp calls becoming direct because
that exposes them too late to be subject to abnormal edge creation.
The following patch honors gimple_call_ctrl_altering for those and
_not_ treat formerly indirect calls to setjmp as calls to setjmp.
Unfortunately there's no way to have an indirect call to setjmp
properly annotated (the returns_twice attribute is ignored on types).
RTL expansion late discovers returns-twice for the purpose of
adding REG_SETJMP notes and also sets ->calls_setjmp
(instead of asserting it is set). There's no good way to
transfer proper knowledge around here so I'm using ->calls_setjmp
as a flag to indicate whether gimple_call_ctrl_altering was set.
Comments on what's the most sensible thing to do here? Supporting
returns_twice on indirect calls wouldn't be difficult, so we're
talking about how to handle this kind of "legacy" situation?
Bootstrapped and tested on x86_64-unknown-linux-gnu.
OK?
Thanks,
Richard.
PR tree-optimization/108691
* cfgexpand.cc (expand_call_stmt): Clear cfun->calls_setjmp
temporarily if the call is not control-altering.
* calls.cc (emit_call_1): Do not add REG_SETJMP if
cfun->calls_setjmp is not set. Do not alter cfun->calls_setjmp.
* tree-ssa-dce.cc (eliminate_unnecessary_stmts): For calls
that are not control-altering do not set cfun->calls_setjmp.
* gcc.dg/pr108691.c: New testcase.
---
gcc/calls.cc | 10 +++----
gcc/cfgexpand.cc | 7 +++++
gcc/testsuite/gcc.dg/pr108691.c | 9 ++++++
gcc/tree-ssa-dce.cc | 49 +++++++++++++++++++--------------
4 files changed, 49 insertions(+), 26 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr108691.c
diff --git a/gcc/calls.cc b/gcc/calls.cc
index 4d7f6c3d291..0242d52cfb3 100644
--- a/gcc/calls.cc
+++ b/gcc/calls.cc
@@ -506,11 +506,11 @@ emit_call_1 (rtx funexp, tree fntree ATTRIBUTE_UNUSED, tree fndecl ATTRIBUTE_UNU
if (ecf_flags & ECF_NORETURN)
add_reg_note (call_insn, REG_NORETURN, const0_rtx);
- if (ecf_flags & ECF_RETURNS_TWICE)
- {
- add_reg_note (call_insn, REG_SETJMP, const0_rtx);
- cfun->calls_setjmp = 1;
- }
+ if (ecf_flags & ECF_RETURNS_TWICE
+ /* We rely on GIMPLE setting this flag and here use it to
+ catch formerly indirect and not control-altering calls. */
+ && cfun->calls_setjmp)
+ add_reg_note (call_insn, REG_SETJMP, const0_rtx);
SIBLING_CALL_P (call_insn) = ((ecf_flags & ECF_SIBCALL) != 0);
diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
index 25b1558dcb9..ab143a6d2d3 100644
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -2808,6 +2808,11 @@ expand_call_stmt (gcall *stmt)
/* Must come after copying location. */
copy_warning (exp, stmt);
+ /* For calls that do not alter control flow avoid REG_SETJMP notes. */
+ bool saved_calls_setjmp = cfun->calls_setjmp;
+ if (!gimple_call_ctrl_altering_p (stmt))
+ cfun->calls_setjmp = false;
+
/* Ensure RTL is created for debug args. */
if (decl && DECL_HAS_DEBUG_ARGS_P (decl))
{
@@ -2846,6 +2851,8 @@ expand_call_stmt (gcall *stmt)
}
mark_transaction_restart_calls (stmt);
+
+ cfun->calls_setjmp = saved_calls_setjmp;
}
diff --git a/gcc/testsuite/gcc.dg/pr108691.c b/gcc/testsuite/gcc.dg/pr108691.c
new file mode 100644
index 00000000000..e412df10f22
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr108691.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+extern int __attribute__((returns_twice)) setjmp(void*);
+
+void bbb(void) {
+ int (*fnptr)(void*) = setjmp;
+ fnptr(0);
+}
diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
index ceeb0ad5ab3..a655b06f800 100644
--- a/gcc/tree-ssa-dce.cc
+++ b/gcc/tree-ssa-dce.cc
@@ -1313,7 +1313,6 @@ eliminate_unnecessary_stmts (bool aggressive)
basic_block bb;
gimple_stmt_iterator gsi, psi;
gimple *stmt;
- tree call;
auto_vec<edge> to_remove_edges;
if (dump_file && (dump_flags & TDF_DETAILS))
@@ -1416,50 +1415,58 @@ eliminate_unnecessary_stmts (bool aggressive)
remove_dead_stmt (&gsi, bb, to_remove_edges);
continue;
}
- else if (is_gimple_call (stmt))
+ else if (gcall *call = dyn_cast <gcall *> (stmt))
{
- tree name = gimple_call_lhs (stmt);
+ tree name = gimple_call_lhs (call);
- notice_special_calls (as_a <gcall *> (stmt));
+ bool saved_calls_setjmp = cfun->calls_setjmp;
+ notice_special_calls (call);
+ /* Ignore ECF_RETURNS_TWICE from calls not marked as
+ control altering. */
+ if (!saved_calls_setjmp
+ && cfun->calls_setjmp
+ && !gimple_call_ctrl_altering_p (call))
+ cfun->calls_setjmp = false;
/* When LHS of var = call (); is dead, simplify it into
call (); saving one operand. */
+ tree fndecl;
if (name
&& TREE_CODE (name) == SSA_NAME
&& !bitmap_bit_p (processed, SSA_NAME_VERSION (name))
/* Avoid doing so for allocation calls which we
did not mark as necessary, it will confuse the
special logic we apply to malloc/free pair removal. */
- && (!(call = gimple_call_fndecl (stmt))
- || ((DECL_BUILT_IN_CLASS (call) != BUILT_IN_NORMAL
- || (DECL_FUNCTION_CODE (call) != BUILT_IN_ALIGNED_ALLOC
- && DECL_FUNCTION_CODE (call) != BUILT_IN_MALLOC
- && DECL_FUNCTION_CODE (call) != BUILT_IN_CALLOC
+ && (!(fndecl = gimple_call_fndecl (call))
+ || ((DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL
+ || (DECL_FUNCTION_CODE (fndecl) != BUILT_IN_ALIGNED_ALLOC
+ && DECL_FUNCTION_CODE (fndecl) != BUILT_IN_MALLOC
+ && DECL_FUNCTION_CODE (fndecl) != BUILT_IN_CALLOC
&& !ALLOCA_FUNCTION_CODE_P
- (DECL_FUNCTION_CODE (call))))
- && !DECL_IS_REPLACEABLE_OPERATOR_NEW_P (call))))
+ (DECL_FUNCTION_CODE (fndecl))))
+ && !DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fndecl))))
{
something_changed = true;
if (dump_file && (dump_flags & TDF_DETAILS))
{
fprintf (dump_file, "Deleting LHS of call: ");
- print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
+ print_gimple_stmt (dump_file, call, 0, TDF_SLIM);
fprintf (dump_file, "\n");
}
- gimple_call_set_lhs (stmt, NULL_TREE);
- maybe_clean_or_replace_eh_stmt (stmt, stmt);
- update_stmt (stmt);
+ gimple_call_set_lhs (call, NULL_TREE);
+ maybe_clean_or_replace_eh_stmt (call, call);
+ update_stmt (call);
release_ssa_name (name);
/* GOMP_SIMD_LANE (unless three argument) or ASAN_POISON
without lhs is not needed. */
- if (gimple_call_internal_p (stmt))
- switch (gimple_call_internal_fn (stmt))
+ if (gimple_call_internal_p (call))
+ switch (gimple_call_internal_fn (call))
{
case IFN_GOMP_SIMD_LANE:
- if (gimple_call_num_args (stmt) >= 3
- && !integer_nonzerop (gimple_call_arg (stmt, 2)))
+ if (gimple_call_num_args (call) >= 3
+ && !integer_nonzerop (gimple_call_arg (call, 2)))
break;
/* FALLTHRU */
case IFN_ASAN_POISON:
@@ -1469,8 +1476,8 @@ eliminate_unnecessary_stmts (bool aggressive)
break;
}
}
- else if (gimple_call_internal_p (stmt))
- switch (gimple_call_internal_fn (stmt))
+ else if (gimple_call_internal_p (call))
+ switch (gimple_call_internal_fn (call))
{
case IFN_ADD_OVERFLOW:
maybe_optimize_arith_overflow (&gsi, PLUS_EXPR);
--
2.35.3
next reply other threads:[~2023-02-13 11:00 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-13 11:00 Richard Biener [this message]
2023-02-13 11:14 ` Jakub Jelinek
2023-02-13 12:41 ` Richard Biener
2023-02-13 12:46 ` Jakub Jelinek
2023-02-13 13:18 ` Richard Biener
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=20230213110058.0C9B61391B@imap2.suse-dmz.suse.de \
--to=rguenther@suse.de \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
/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).