public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
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

             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).