public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew Pinski <apinski@marvell.com>
To: <gcc-patches@gcc.gnu.org>
Cc: Andrew Pinski <apinski@marvell.com>
Subject: [PATCH 2/3] PHIOPT: Loop over calling factor_out_conditional_conversion
Date: Sat, 6 May 2023 21:43:31 -0700	[thread overview]
Message-ID: <20230507044332.1122612-2-apinski@marvell.com> (raw)
In-Reply-To: <20230507044332.1122612-1-apinski@marvell.com>

After adding diamond shaped bb support to factor_out_conditional_conversion,
we can get a case where we have two conversions that needs factored out
and then would have another phiopt happen.
An example is:
```
static inline unsigned long long g(int t)
{
  unsigned t1 = t;
  return t1;
}
unsigned long long f(int c, int d, int e)
{
  unsigned long long t;
  if (c > d)
    t = g(c);
  else
    t = g(d);
  return t;
}
```
In this case we should get a MAX_EXPR in phiopt1 with two casts.
Before this patch, we would just factor out the outer cast and then
wait till phiopt2 to factor out the inner cast.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* tree-ssa-phiopt.cc (pass_phiopt::execute): Loop
	over factor_out_conditional_conversion.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/minmax-17.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c | 21 ++++++++++++++++++
 gcc/tree-ssa-phiopt.cc                    | 27 +++++++++++++----------
 2 files changed, 36 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c b/gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c
new file mode 100644
index 00000000000..bd737e6b4cb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-phiopt1-details" } */
+
+static inline unsigned long long g(int t)
+{
+  unsigned t1 = t;
+  return t1;
+}
+unsigned long long test_max(int c, int d, int e)
+{
+  unsigned long long t;
+  if (c > d)
+    t = g(c);
+  else
+    t = g(d);
+  return t;
+}
+
+/* We should figure out that test_max has an MAX_EXPR in it. */
+/* { dg-final { scan-tree-dump " = MAX_EXPR" "phiopt1"} } */
+/* { dg-final { scan-tree-dump-times "changed to factor conversion out from" 2 "phiopt1"} } */
diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 41fea78dc8d..7fe088b13ff 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -4085,20 +4085,23 @@ pass_phiopt::execute (function *)
 	  node.  */
       gcc_assert (arg0 != NULL_TREE && arg1 != NULL_TREE);
 
-      gphi *newphi;
       if (single_pred_p (bb1)
-	  && EDGE_COUNT (merge->preds) == 2
-	  && (newphi = factor_out_conditional_conversion (e1, e2, phi,
-							  arg0, arg1,
-							  cond_stmt)))
+	  && EDGE_COUNT (merge->preds) == 2)
 	{
-	  phi = newphi;
-	  /* factor_out_conditional_conversion may create a new PHI in
-	     BB2 and eliminate an existing PHI in BB2.  Recompute values
-	     that may be affected by that change.  */
-	  arg0 = gimple_phi_arg_def (phi, e1->dest_idx);
-	  arg1 = gimple_phi_arg_def (phi, e2->dest_idx);
-	  gcc_assert (arg0 != NULL_TREE && arg1 != NULL_TREE);
+	  gphi *newphi = phi;
+	  while (newphi)
+	    {
+	      phi = newphi;
+	      /* factor_out_conditional_conversion may create a new PHI in
+		 BB2 and eliminate an existing PHI in BB2.  Recompute values
+		 that may be affected by that change.  */
+	      arg0 = gimple_phi_arg_def (phi, e1->dest_idx);
+	      arg1 = gimple_phi_arg_def (phi, e2->dest_idx);
+	      gcc_assert (arg0 != NULL_TREE && arg1 != NULL_TREE);
+	      newphi = factor_out_conditional_conversion (e1, e2, phi,
+							  arg0, arg1,
+							  cond_stmt);
+	    }
 	}
 
       /* Do the replacement of conditional if it can be done.  */
-- 
2.31.1


  reply	other threads:[~2023-05-07  4:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-07  4:43 [PATCH 1/3] PHIOPT: Add diamond bb form to factor_out_conditional_conversion Andrew Pinski
2023-05-07  4:43 ` Andrew Pinski [this message]
2023-05-08  6:47   ` [PATCH 2/3] PHIOPT: Loop over calling factor_out_conditional_conversion Richard Biener
2023-05-07  4:43 ` [PATCH 3/3] PHIOPT: factor out unary operations instead of just conversions Andrew Pinski
2023-05-08  6:46   ` Richard Biener
2023-05-08  6:51 ` [PATCH 1/3] PHIOPT: Add diamond bb form to factor_out_conditional_conversion Richard Biener
2023-05-08  6:52   ` Richard Biener
2023-05-08  7:09     ` Andrew Pinski
2023-05-08  7:28       ` 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=20230507044332.1122612-2-apinski@marvell.com \
    --to=apinski@marvell.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).