Re: [PATCH] tree-optimization/109046 - re-combine complex loads

Re: [PATCH] tree-optimization/109046 - re-combine complex loads

[Jeff Law]

On 3/7/23 04:08, Richard Biener via Gcc-patches wrote:
> The following addresses PR109046 by adding an optimization to forwprop
> to combine a piecewise complex load to a complex load when there are
> no uses of the components.  That's something useful in general and
> easier to do than avoiding the splitting in complex lowering.
Presumably instead of "no uses of the components" you meant to say "no 
other uses of the components".

> 
> The testcase exercises both the manual and the complex lowering case.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
> 
> Thanks,
> Richard.
> 
> 	PR tree-optimization/109046
> 	* tree-ssa-forwprop.cc (pass_forwprop::execute): Combine
> 	piecewise complex loads.
> 
> 	* gcc.dg/tree-ssa/forwprop-39.c: New testcase.
> ---

[ ... ]
> +	      /* Rewrite a component-wise load of a complex to a complex
> +		 load if the components are not used separately.  */
> +	      else if (TREE_CODE (rhs) == SSA_NAME
> +		       && has_single_use (rhs)
Did you really meant to test that RHS is single use?  Isn't that the 
COMPLEX_EXPR here?  It would seem fine to me to allow multiple uses of 
the COMPLEX_EXPR itself.  Am I missing something?

The only conceptual concern I have would be volatile objects.  But in 
that case were're replacing a piecewise load of the volatile with a 
single complex load.  That probably better matches what the developer 
expected anyway.

So OK as-is if you really wanted the COMPLEX_EXPR to have a single use. 
Or OK after removing that test.




Jeff

Re: [PATCH] tree-optimization/109046 - re-combine complex loads

[Richard Biener]

On Sat, 11 Mar 2023, Jeff Law wrote:

> 
> 
> On 3/7/23 04:08, Richard Biener via Gcc-patches wrote:
> > The following addresses PR109046 by adding an optimization to forwprop
> > to combine a piecewise complex load to a complex load when there are
> > no uses of the components.  That's something useful in general and
> > easier to do than avoiding the splitting in complex lowering.
> Presumably instead of "no uses of the components" you meant to say "no other
> uses of the components".

Yes, will reword.

> > 
> > The testcase exercises both the manual and the complex lowering case.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
> > 
> > Thanks,
> > Richard.
> > 
> >  PR tree-optimization/109046
> >  * tree-ssa-forwprop.cc (pass_forwprop::execute): Combine
> >  piecewise complex loads.
> > 
> > 	* gcc.dg/tree-ssa/forwprop-39.c: New testcase.
> > ---
> 
> [ ... ]
> > +	      /* Rewrite a component-wise load of a complex to a complex
> > +		 load if the components are not used separately.  */
> > +	      else if (TREE_CODE (rhs) == SSA_NAME
> > +		       && has_single_use (rhs)
> Did you really meant to test that RHS is single use?  Isn't that the
> COMPLEX_EXPR here?  It would seem fine to me to allow multiple uses of the
> COMPLEX_EXPR itself.  Am I missing something?

Yeah, rhs is actually rhs1, the first operand of the COMPLEX_EXPR ...

> The only conceptual concern I have would be volatile objects.  But in that
> case were're replacing a piecewise load of the volatile with a single complex
> load.  That probably better matches what the developer expected anyway.

Ah, but good that you notice - we shouldn't do this combination for 
volatile component loads.

I'll adjust accordingly re-test and commit.

> So OK as-is if you really wanted the COMPLEX_EXPR to have a single use. Or OK
> after removing that test.

Thanks,
Richard.

[PATCH] tree-optimization/109046 - re-combine complex loads

[Richard Biener]

The following addresses PR109046 by adding an optimization to forwprop
to combine a piecewise complex load to a complex load when there are
no uses of the components.  That's something useful in general and
easier to do than avoiding the splitting in complex lowering.

The testcase exercises both the manual and the complex lowering case.

Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?

Thanks,
Richard.

	PR tree-optimization/109046
	* tree-ssa-forwprop.cc (pass_forwprop::execute): Combine
	piecewise complex loads.

	* gcc.dg/tree-ssa/forwprop-39.c: New testcase.
---
 gcc/testsuite/gcc.dg/tree-ssa/forwprop-39.c | 15 ++++++++++
 gcc/tree-ssa-forwprop.cc                    | 31 ++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/forwprop-39.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/forwprop-39.c b/gcc/testsuite/gcc.dg/tree-ssa/forwprop-39.c
new file mode 100644
index 00000000000..eb2930e77fd
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/forwprop-39.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-std=c11 -O2 -fdump-tree-forwprop1 -fdump-tree-optimized" } */
+
+#include <complex.h>
+
+extern void push1(void *p, float _Complex x);
+void foo (void *q, float _Complex *x)
+{
+  float r = __real *x;
+  float i = __imag *x;
+  push1 (q, CMPLXF (r, i));
+}
+
+/* { dg-final { scan-tree-dump-not "COMPLEX_EXPR" "forwprop1" } } */
+/* { dg-final { scan-tree-dump-not "REALPART_EXPR" "optimized" } } */
diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc
index 03fe0a3f6df..3111a2b96a3 100644
--- a/gcc/tree-ssa-forwprop.cc
+++ b/gcc/tree-ssa-forwprop.cc
@@ -3669,7 +3669,8 @@ pass_forwprop::execute (function *fun)
 	      /* Rewrite stores of a single-use complex build expression
 	         to component-wise stores.  */
 	      use_operand_p use_p;
-	      gimple *use_stmt;
+	      gimple *use_stmt, *def1, *def2;
+	      tree rhs2;
 	      if (single_imm_use (lhs, &use_p, &use_stmt)
 		  && gimple_store_p (use_stmt)
 		  && !gimple_has_volatile_ops (use_stmt)
@@ -3703,6 +3704,34 @@ pass_forwprop::execute (function *fun)
 		  release_defs (stmt);
 		  gsi_remove (&gsi, true);
 		}
+	      /* Rewrite a component-wise load of a complex to a complex
+		 load if the components are not used separately.  */
+	      else if (TREE_CODE (rhs) == SSA_NAME
+		       && has_single_use (rhs)
+		       && ((rhs2 = gimple_assign_rhs2 (stmt)), true)
+		       && TREE_CODE (rhs2) == SSA_NAME
+		       && has_single_use (rhs2)
+		       && (def1 = SSA_NAME_DEF_STMT (rhs),
+			   gimple_assign_load_p (def1))
+		       && (def2 = SSA_NAME_DEF_STMT (rhs2),
+			   gimple_assign_load_p (def2))
+		       && (gimple_vuse (def1) == gimple_vuse (def2))
+		       && gimple_assign_rhs_code (def1) == REALPART_EXPR
+		       && gimple_assign_rhs_code (def2) == IMAGPART_EXPR
+		       && operand_equal_p (TREE_OPERAND (gimple_assign_rhs1
+								 (def1), 0),
+					   TREE_OPERAND (gimple_assign_rhs1
+								 (def2), 0)))
+		{
+		  tree cl = TREE_OPERAND (gimple_assign_rhs1 (def1), 0);
+		  gimple_assign_set_rhs_from_tree (&gsi, unshare_expr (cl));
+		  gcc_assert (gsi_stmt (gsi) == stmt);
+		  gimple_set_vuse (stmt, gimple_vuse (def1));
+		  gimple_set_modified (stmt, true);
+		  gimple_stmt_iterator gsi2 = gsi_for_stmt (def1);
+		  gsi_remove (&gsi, false);
+		  gsi_insert_after (&gsi2, stmt, GSI_SAME_STMT);
+		}
 	      else
 		gsi_next (&gsi);
 	    }
-- 
2.35.3