On Mon, 13 Feb 2023, Jakub Jelinek wrote:
> On Mon, Feb 13, 2023 at 12:41:48PM +0000, Richard Biener wrote:
> > > Could we e.g. prevent turning such indirect calls into direct calls?
> >
> > We do exactly have gimple_call_fntype and gimple_call_ctrl_altering_p
> > to not require special-casing indirect to direct call promotion here.
>
> Ah, so if we make returns_twice apply to function types, then we could
> just compare if gimple_call_fntype has also returns_twice and if not, not
> consider it actually returns_twice.
>
> > > Anyway, notice_special_calls is called in various spots, not just DCE,
> > > wouldn't it be better to simply not set calls_setjmp flag in there if
> > > the current function already has cfg and the call isn't ctrl altering?
> >
> > I thought about changing gimple_call_flags instead, filtering out
> > ECF_RETURNS_TWICE. I just didn't make up my mind on what
> > property to key at (and to require 'cfun' to be set to query it).
> > But sure, changing notice_special_calls also works - the only
> > other relevant caller is the inliner I think, and that could be
> > replaced by caller |= callee of the two flags tracked instead of
> > re-scanning each inlined stmt.
> >
> > Would you be happy with changing notice_special_calls, dropping the
> > DCE hunk but keeping the cfgexpand/calls.cc hunks?
>
> I think so.
I'm testing the following and am queueing the second patch below
for next stage1.
Richard.
>From d5e62f27489c1e7a8696a85e7bc98cc0a26564d2 Mon Sep 17 00:00:00 2001
From: Richard Biener <rguent...@suse.de>
Date: Mon, 13 Feb 2023 10:41:51 +0100
Subject: [PATCH 1/2] tree-optimization/108691 - indirect calls to setjmp
To: gcc-patches@gcc.gnu.org
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
in notice_special_calls.
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_p was set.
PR tree-optimization/108691
* tree-cfg.cc (notice_special_calls): When the CFG is built
honor gimple_call_ctrl_altering_p.
* 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.
* gcc.dg/pr108691.c: New testcase.
---
gcc/calls.cc | 10 +++++-----
gcc/cfgexpand.cc | 7 +++++++
gcc/testsuite/gcc.dg/pr108691.c | 9 +++++++++
gcc/tree-cfg.cc | 4 +++-
4 files changed, 24 insertions(+), 6 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-cfg.cc b/gcc/tree-cfg.cc
index a9fcc7fd050..e23293e5cd1 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -2280,7 +2280,9 @@ notice_special_calls (gcall *call)
if (flags & ECF_MAY_BE_ALLOCA)
cfun->calls_alloca = true;
- if (flags & ECF_RETURNS_TWICE)
+ if (flags & ECF_RETURNS_TWICE
+ && (!(cfun->curr_properties & PROP_cfg)
+ || gimple_call_ctrl_altering_p (call)))
cfun->calls_setjmp = true;
}
--
2.35.3
>From 01375d8b209c91ea3d5b9e6f6ee6fa3e814c4142 Mon Sep 17 00:00:00 2001
From: Richard Biener <rguent...@suse.de>
Date: Mon, 13 Feb 2023 14:14:02 +0100
Subject: [PATCH 2/2] TLC to notice_special_calls
To: gcc-patches@gcc.gnu.org
The following adds a struct function argument to notice_special_calls,
removes the use from the inliner and make sure to not switch cfun
for each stmt outlined to another function.
* tree-cfg.h (notice_speical_calls): Add struct function
argument.
* tree-cfg.cc (notice_special_calls): Likewise.
(move_block_to_fn): Do not switch cfun around update_stmt and
notice_special_calls.
* gimplify.cc (gimplify_call_expr): Adjust.
(gimplify_modify_expr): Likewise.
* tree-ssa-dce.cc (eliminate_unnecessary_stmts): Likewise.
---
gcc/gimplify.cc | 4 ++--
gcc/tree-cfg.cc | 14 ++++++--------
gcc/tree-cfg.h | 2 +-
gcc/tree-inline.cc | 5 +++--
gcc/tree-ssa-dce.cc | 2 +-
5 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 1b362dd83e3..254ddc3c211 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -3864,7 +3864,7 @@ gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool
want_value)
have to do is replicate it as a GIMPLE_CALL tuple. */
gimple_stmt_iterator gsi;
call = gimple_build_call_from_tree (*expr_p, fnptrtype);
- notice_special_calls (call);
+ notice_special_calls (cfun, call);
gimplify_seq_add_stmt (pre_p, call);
gsi = gsi_last (*pre_p);
maybe_fold_stmt (&gsi);
@@ -6298,7 +6298,7 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p,
call_stmt = gimple_build_call_from_tree (*from_p, fnptrtype);
}
}
- notice_special_calls (call_stmt);
+ notice_special_calls (cfun, call_stmt);
if (!gimple_call_noreturn_p (call_stmt) || !should_remove_lhs_p (*to_p))
gimple_call_set_lhs (call_stmt, *to_p);
else if (TREE_CODE (*to_p) == SSA_NAME)
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index e23293e5cd1..1dffcc6a693 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -2274,16 +2274,16 @@ single_noncomplex_succ (basic_block bb)
/* T is CALL_EXPR. Set current_function_calls_* flags. */
void
-notice_special_calls (gcall *call)
+notice_special_calls (function *fn, gcall *call)
{
int flags = gimple_call_flags (call);
if (flags & ECF_MAY_BE_ALLOCA)
- cfun->calls_alloca = true;
+ fn->calls_alloca = true;
if (flags & ECF_RETURNS_TWICE
- && (!(cfun->curr_properties & PROP_cfg)
+ && (!(fn->curr_properties & PROP_cfg)
|| gimple_call_ctrl_altering_p (call)))
- cfun->calls_setjmp = true;
+ fn->calls_setjmp = true;
}
@@ -7446,11 +7446,9 @@ move_block_to_fn (struct function *dest_cfun,
basic_block bb,
/* We cannot leave any operands allocated from the operand caches of
the current function. */
free_stmt_operands (cfun, stmt);
- push_cfun (dest_cfun);
- update_stmt (stmt);
+ update_stmt_fn (dest_cfun, stmt);
if (is_gimple_call (stmt))
- notice_special_calls (as_a <gcall *> (stmt));
- pop_cfun ();
+ notice_special_calls (dest_cfun, as_a <gcall *> (stmt));
}
FOR_EACH_EDGE (e, ei, bb->succs)
diff --git a/gcc/tree-cfg.h b/gcc/tree-cfg.h
index 9b56a68fe9d..f845fcb9699 100644
--- a/gcc/tree-cfg.h
+++ b/gcc/tree-cfg.h
@@ -40,7 +40,7 @@ extern bool group_case_labels_stmt (gswitch *);
extern bool group_case_labels (void);
extern void replace_uses_by (tree, tree);
extern basic_block single_noncomplex_succ (basic_block bb);
-extern void notice_special_calls (gcall *);
+extern void notice_special_calls (struct function *, gcall *);
extern void clear_special_calls (void);
extern edge find_taken_edge (basic_block, tree);
extern void gimple_debug_bb (basic_block);
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index 7fd08310acf..af5b99fbaa3 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -2353,8 +2353,6 @@ copy_bb (copy_body_data *id, basic_block bb,
dest->dump_name ());
}
}
-
- notice_special_calls (as_a <gcall *> (stmt));
}
maybe_duplicate_eh_stmt_fn (cfun, stmt, id->src_cfun, orig_stmt,
@@ -3076,6 +3074,9 @@ copy_cfg_body (copy_body_data * id,
new_bb->loop_father = entry_block_map->loop_father;
}
+ cfun->calls_alloca |= src_cfun->calls_alloca;
+ cfun->calls_setjmp |= src_cfun->calls_setjmp;
+
last = last_basic_block_for_fn (cfun);
/* Now that we've duplicated the blocks, duplicate their edges. */
diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
index ceeb0ad5ab3..e9404c55bf9 100644
--- a/gcc/tree-ssa-dce.cc
+++ b/gcc/tree-ssa-dce.cc
@@ -1420,7 +1420,7 @@ eliminate_unnecessary_stmts (bool aggressive)
{
tree name = gimple_call_lhs (stmt);
- notice_special_calls (as_a <gcall *> (stmt));
+ notice_special_calls (cfun, as_a <gcall *> (stmt));
/* When LHS of var = call (); is dead, simplify it into
call (); saving one operand. */
--
2.35.3
