public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] tailc: Extend the IPA-VRP workaround [PR119614]
Date: Mon, 7 Apr 2025 07:26:28 +0200	[thread overview]
Message-ID: <Z/NiBLuNG0iaDHwi@tucnak> (raw)

Hi!

The IPA-VRP workaround in the tailc/musttail passes was just comparing
the singleton constant from a tail call candidate return with the ret_val.
This unfortunately doesn't work in the following testcase, where we have
  <bb 5> [local count: 152205050]:
  baz (); [must tail call]
  goto <bb 4>; [100.00%]

  <bb 6> [local count: 762356696]:
  _8 = foo ();

  <bb 7> [local count: 1073741824]:
  # _3 = PHI <0B(4), _8(6)>
  return _3;
and in the unreduced testcase even more PHIs before we reach the return
stmt.

Normally when the call has lhs, whenever we follow a (non-EH) successor
edge, it calls propagate_through_phis and that walks the PHIs in the
destination bb of the edge and when it sees a PHI whose argument matches
that of the currently tracked value (ass_var), it updates ass_var to
PHI result of that PHI.  I think it is theoretically dangerous that it
picks the first one, perhaps there could be multiple PHIs, so perhaps safer
would be walk backwards from the return value up to the call.

Anyway, this PR is about the IPA-VRP workaround, there ass_var is NULL
because the potential tail call has no lhs, but ret_var is not TREE_CONSTANT
but SSA_NAME with PHI as SSA_NAME_DEF_STMT.  The following patch handles
it by pushing the edges we've walked through when ass_var is NULL into a
vector and if ret_var is SSA_NAME set to PHI result, it attempts to walk
back from the ret_var through arguments of PHIs corresponding to the
edges we've walked back until we reach a constant and compare that constant
against the singleton value as well.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2025-04-07  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/119614
	* tree-tailcall.cc (find_tail_calls): Remember edges which have been
	walked through if !ass_var.  Perform IPA-VRP workaround even when
	ret_var is not TREE_CONSTANT, in that case check in a loop if it is
	a PHI result and in that case look at the PHI argument from
	corresponding edge in the edge vector.

	* g++.dg/opt/pr119613.C: Change { c || c++11 } in obviously C++ only
	test to just c++11.
	* g++.dg/opt/pr119614.C: New test.

--- gcc/tree-tailcall.cc.jj	2025-04-04 20:52:34.450015821 +0200
+++ gcc/tree-tailcall.cc	2025-04-05 14:50:50.106693562 +0200
@@ -920,6 +920,7 @@ find_tail_calls (basic_block bb, struct
   auto_bitmap to_move_defs;
   auto_vec<gimple *> to_move_stmts;
   bool is_noreturn = gimple_call_noreturn_p (call);
+  auto_vec<edge> edges;
 
   abb = bb;
   agsi = gsi;
@@ -933,6 +934,8 @@ find_tail_calls (basic_block bb, struct
 	{
 	  edge e = single_non_eh_succ_edge (abb);
 	  ass_var = propagate_through_phis (ass_var, e);
+	  if (!ass_var)
+	    edges.safe_push (e);
 	  abb = e->dest;
 	  agsi = gsi_start_bb (abb);
 	}
@@ -1040,9 +1043,7 @@ find_tail_calls (basic_block bb, struct
       /* If IPA-VRP proves called function always returns a singleton range,
 	 the return value is replaced by the only value in that range.
 	 For tail call purposes, pretend such replacement didn't happen.  */
-      if (ass_var == NULL_TREE
-	  && !tail_recursion
-	  && TREE_CONSTANT (ret_var))
+      if (ass_var == NULL_TREE && !tail_recursion)
 	if (tree type = gimple_range_type (call))
 	  if (tree callee = gimple_call_fndecl (call))
 	    if ((INTEGRAL_TYPE_P (type)
@@ -1052,9 +1053,43 @@ find_tail_calls (basic_block bb, struct
 					      type)
 		&& useless_type_conversion_p (TREE_TYPE (ret_var), type)
 		&& ipa_return_value_range (val, callee)
-		&& val.singleton_p (&valr)
-		&& operand_equal_p (ret_var, valr, 0))
-	      ok = true;
+		&& val.singleton_p (&valr))
+	      {
+		tree rv = ret_var;
+		unsigned int i = edges.length ();
+		/* If ret_var is equal to valr, we can tail optimize.  */
+		if (operand_equal_p (ret_var, valr, 0))
+		  ok = true;
+		else
+		  /* Otherwise, if ret_var is a PHI result, try to find out
+		     if valr isn't propagated through PHIs on the path from
+		     call's bb to SSA_NAME_DEF_STMT (ret_var)'s bb.  */
+		  while (TREE_CODE (rv) == SSA_NAME
+			 && gimple_code (SSA_NAME_DEF_STMT (rv)) == GIMPLE_PHI)
+		    {
+		      tree nrv = NULL_TREE;
+		      gimple *g = SSA_NAME_DEF_STMT (rv);
+		      for (; i; --i)
+			{
+			  if (edges[i - 1]->dest == gimple_bb (g))
+			    {
+			      nrv
+				= gimple_phi_arg_def_from_edge (g,
+								edges[i - 1]);
+			      --i;
+			      break;
+			    }
+			}
+		      if (nrv == NULL_TREE)
+			break;
+		      if (operand_equal_p (nrv, valr, 0))
+			{
+			  ok = true;
+			  break;
+			}
+		      rv = nrv;
+		    }
+	      }
       if (!ok)
 	{
 	  maybe_error_musttail (call,
--- gcc/testsuite/g++.dg/opt/pr119613.C.jj	2025-04-04 20:51:42.482706589 +0200
+++ gcc/testsuite/g++.dg/opt/pr119613.C	2025-04-05 14:57:31.157353618 +0200
@@ -1,5 +1,5 @@
 // PR middle-end/119613
-// { dg-do compile { target { musttail && { c || c++11 } } } }
+// { dg-do compile { target { musttail && c++11 } } }
 // { dg-options "-O0" }
 
 struct S { S () {} };
--- gcc/testsuite/g++.dg/opt/pr119614.C.jj	2025-04-05 14:57:16.276551780 +0200
+++ gcc/testsuite/g++.dg/opt/pr119614.C	2025-04-05 14:56:46.020954674 +0200
@@ -0,0 +1,30 @@
+// PR tree-optimization/119614
+// { dg-do compile { target musttail } }
+// { dg-options "-O2" }
+
+struct S {} b;
+char *foo ();
+int e, g;
+void bar ();
+void corge (S);
+
+[[gnu::noinline]] char *
+baz ()
+{
+  bar ();
+  return 0;
+}
+
+const char *
+qux ()
+{
+  if (e)
+    {
+      S a = b;
+      corge (a);
+      if (g)
+        return 0;
+      [[gnu::musttail]] return baz ();
+    }
+  return foo ();
+}

	Jakub


             reply	other threads:[~2025-04-07  5:26 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-07  5:26 Jakub Jelinek [this message]
2025-04-07  9:43 ` Richard Biener
2025-04-07  9:49   ` Jakub Jelinek

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=Z/NiBLuNG0iaDHwi@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    /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).