Re: [autovect] [patch] Extend store cpp

Re: [autovect] [patch] Extend store cpp

[Dorit Nuzman]

Revital1 Eres/Haifa/IBM wrote on 20/03/2007 10:33:11:

>
> > > Bootstrap and tested on ppc. OK for autovect branch?
> >
> > Why are you submitting this for autovect instead of mainline?
> > Seems like it will just cause you merge hassles later on.
> > It seems like now would be the perfect time to get this on mainline
> > instead of autovect.
>
> The latest adjustments to lim optimization; which is currently only in
> autovect branch; can open an opportunity for store ccp to be applied and
> as a result for the vectorizer as well. (as was shown in the example in -
> http://gcc.gnu.org/ml/gcc-patches/2007-03/msg01255.html). That is why
> autovect branch was chosen. I intend to submit this patch to mainline
> soon after committing it to the branch. I'll be happy to propose the
> patch for mainline right now, but if it turns out that the patch need
> to be considerably revised before it can be committed to mainline,
> I wish to commit it to the branch until I revise the implementation.
>

I'd be fine with that

dorit

> Thanks,
> Revital

Re: [autovect] [patch] Extend store cpp

[Revital1 Eres]

> > Bootstrap and tested on ppc. OK for autovect branch?
>
> Why are you submitting this for autovect instead of mainline?
> Seems like it will just cause you merge hassles later on.
> It seems like now would be the perfect time to get this on mainline
> instead of autovect.

The latest adjustments to lim optimization; which is currently only in
autovect branch; can open an opportunity for store ccp to be applied and
as a result for the vectorizer as well. (as was shown in the example in -
http://gcc.gnu.org/ml/gcc-patches/2007-03/msg01255.html). That is why
autovect branch was chosen. I intend to submit this patch to mainline
soon after committing it to the branch. I'll be happy to propose the
patch for mainline right now, but if it turns out that the patch need
to be considerably revised before it can be committed to mainline,
I wish to commit it to the branch until I revise the implementation.

Thanks,
Revital

Re: [autovect] [patch] Extend store cpp

[Daniel Berlin]

On 3/19/07, Revital1 Eres <ERES@il.ibm.com> wrote:
>
> Hello,
>
> This patch enhance store ccp to propagate constants from stores to
> loads when the previous implementation failed due to alias uncertainty.
> The testcase attached demonstrates this optimization:
>
> S[5].x = 0;
> S[5].y = 0;
>
> x = S[5].x;
> y = S[5].y;
>
> Previously x = S[5].x; failed to be replaced with x = 0.  This patch
> fixes this problem by allowing the propagation of constants if all the
> stores in the alias group are initialized with the same constant.
>
> I used the get_stmt_rhs_def_stmt function Richard Guenther
> introduced in his patch to walk the virtual def-use chains
> with adjustments to make it suitable for this optimization.
> (http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00669.html)
>
> Bootstrap and tested on ppc. OK for autovect branch?

Why are you submitting this for autovect instead of mainline?
Seems like it will just cause you merge hassles later on.
It seems like now would be the perfect time to get this on mainline
instead of autovect.

[autovect] [patch] Extend store cpp

[Revital1 Eres]

[-- Attachment #1: Type: text/plain, Size: 1433 bytes --]


Hello,

This patch enhance store ccp to propagate constants from stores to
loads when the previous implementation failed due to alias uncertainty.
The testcase attached demonstrates this optimization:

S[5].x = 0;
S[5].y = 0;

x = S[5].x;
y = S[5].y;

Previously x = S[5].x; failed to be replaced with x = 0.  This patch
fixes this problem by allowing the propagation of constants if all the
stores in the alias group are initialized with the same constant.

I used the get_stmt_rhs_def_stmt function Richard Guenther
introduced in his patch to walk the virtual def-use chains
with adjustments to make it suitable for this optimization.
(http://gcc.gnu.org/ml/gcc-patches/2006-02/msg00669.html)

Bootstrap and tested on ppc. OK for autovect branch?

Revital

2007-03-19  Revital Eres  <eres@il.ibm.com>

        * tree-ssa-propagate.c (all_vdef_have_same_lattice_const_value):
          New function to get the defining stmt.
          (get_def_of_stmt_rhs): New helper function
          which walks on the virtual use-def chain to get the
          defining stmt.
          (replace_vuses_in): Call it.
          (do_store_ccp): New function.
        * tree-ssa-ccp.c (visit_assignment): Call it.
        * tree-ssa-propagate.h (all_vdef_have_same_lattice_const_value):
          Declare.
        * testsuite/gcc.dg/store_ccp_test.c: New test.


(See attached file: store_cpp_patch_19_3.txt)(See attached file:
store_ccp_test.txt)

[-- Attachment #2: store_cpp_patch_19_3.txt --]
[-- Type: text/plain, Size: 4640 bytes --]

Index: tree-ssa-ccp.c
===================================================================
--- tree-ssa-ccp.c	(revision 123037)
+++ tree-ssa-ccp.c	(working copy)
@@ -1202,10 +1202,10 @@
 	 location that created the SSA_NAMEs for the virtual operands,
 	 we can propagate the value on the RHS.  */
       prop_value_t *nval = get_value_loaded_by (stmt, const_val);
-
       if (nval
-	  && nval->mem_ref
-	  && operand_equal_p (nval->mem_ref, rhs, 0))
+          && nval->mem_ref
+          && (operand_equal_p (nval->mem_ref, rhs, 0)
+              || all_vdef_have_same_lattice_const_value (nval, stmt)))
 	val = *nval;
       else
 	val = evaluate_stmt (stmt);
Index: tree-ssa-propagate.c
===================================================================
--- tree-ssa-propagate.c	(revision 123037)
+++ tree-ssa-propagate.c	(working copy)
@@ -892,7 +892,93 @@
   return replaced;
 }
 
+/* Return the defining stmt for the rhs of STMT following the virtual
+   use-def chains if all the VDEFs of this rhs up until the defining stmt
+   initialized by the same constant value as CONST_VAL.  Returns the
+   MODIFY_EXPR stmt which lhs is equal to the rhs of STMT or NULL_TREE
+   if no such stmt can be found.  */
+static tree
+get_def_of_stmt_rhs (tree stmt, tree const_val)
+{
+  /* See if we know exactly where this comes from.  */
+  tree def, rhs, def_stmt, t;
+  use_operand_p use;
 
+  if (TREE_CODE (stmt) != GIMPLE_MODIFY_STMT)
+    return NULL_TREE;
+
+  rhs = GIMPLE_STMT_OPERAND (stmt, 1);
+
+  /* The stmt must have a single VUSE.  */
+  use = SINGLE_SSA_USE_OPERAND (stmt, SSA_OP_VUSE);
+  if (use == NULL_USE_OPERAND_P)
+    return NULL_TREE;
+
+  do
+    {
+      /* Look at the DEF for the VUSE and see if it matches this USE
+         and if it's RHS is an SSA_NAME.  */
+      def_stmt = SSA_NAME_DEF_STMT (*use->use);
+      if (TREE_CODE (def_stmt) != GIMPLE_MODIFY_STMT)
+       return NULL_TREE;
+
+      def = GIMPLE_STMT_OPERAND (def_stmt, 0);
+      t = GIMPLE_STMT_OPERAND (def_stmt, 1);
+     
+      /* Verify that the VUSE is loaded with the same constant value.  */ 
+      if (!is_gimple_min_invariant (t) || !operand_equal_p (t, const_val, 0))
+       return NULL_TREE;
+
+      /* If def_stmt lhs is equal to the rhs of STMT then we found
+      the stmt which defines SSA_NAME of rhs.  */
+      if (operand_equal_p (def, rhs, 0))
+       return def_stmt;
+      use = SINGLE_SSA_USE_OPERAND (def_stmt, SSA_OP_VMAYUSE);
+      if (use == NULL_USE_OPERAND_P)
+       return NULL_TREE;
+    }
+  while (1);
+
+  return NULL_TREE;
+}
+
+static bool
+do_store_ccp (void)
+{
+  return (flag_tree_store_ccp != 0 && flag_tree_ccp != 0);
+}
+
+/* Return true if rhs of STMT has a known VUSE and all
+   it's VDEFs in the virtual use-def chain (up until the insn
+   which created it's SSA_NAME) have the same constant value as
+   recorded in VAL.  */
+bool
+all_vdef_have_same_lattice_const_value (prop_value_t *val, tree stmt)
+{
+  tree def_stmt;
+  tree rhs;
+  
+  if (!stmt_makes_single_load (stmt))
+    return false;
+
+  if (!is_gimple_min_invariant (val->value))
+    return false;
+
+  def_stmt = get_def_of_stmt_rhs (stmt, val->value);
+
+  if (def_stmt == NULL_TREE)
+    return false;
+
+  rhs = get_rhs (def_stmt);
+
+  if (rhs == NULL_TREE)
+    return false;
+
+  if (is_gimple_min_invariant (rhs) && operand_equal_p (rhs, val->value, 0))
+    return true;
+
+  return false;
+}
 /* Replace the VUSE references in statement STMT with the values
    stored in PROP_VALUE.  Return true if a reference was replaced.  If
    REPLACED_ADDRESSES_P is given, it will be set to true if an address
@@ -974,7 +1060,9 @@
 	  && val->value
 	  && (is_gimple_reg (val->value)
 	      || is_gimple_min_invariant (val->value))
-	  && simple_cst_equal (rhs, val->mem_ref) == 1)
+	  && ((simple_cst_equal (rhs, val->mem_ref) == 1)
+              || (do_store_ccp ()
+                  && all_vdef_have_same_lattice_const_value (val, stmt))))
 
 	{
 	  /* If we are replacing a constant address, inform our
Index: tree-ssa-propagate.h
===================================================================
--- tree-ssa-propagate.h	(revision 123037)
+++ tree-ssa-propagate.h	(working copy)
@@ -121,5 +121,5 @@
 prop_value_t *get_value_loaded_by (tree, prop_value_t *);
 bool replace_uses_in (tree, bool *, prop_value_t *);
 bool substitute_and_fold (prop_value_t *, bool);
-
+bool all_vdef_have_same_lattice_const_value (prop_value_t *, tree);
 #endif /* _TREE_SSA_PROPAGATE_H  */

[-- Attachment #3: store_ccp_test.txt --]
[-- Type: text/plain, Size: 719 bytes --]

Index: testsuite/gcc.dg/store_ccp.c
===================================================================
--- testsuite/gcc.dg/store_ccp.c	(revision 0)
+++ testsuite/gcc.dg/store_ccp.c	(revision 0)
@@ -0,0 +1,33 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-store_ccp-details" } */
+
+#define N 256
+
+struct
+{
+  int x;
+  int y;
+} S[100];
+
+int z[100];
+
+int
+foo (void)
+{
+  int i;
+  int x, y;
+
+  S[5].x = 0;
+  S[5].y = 0;
+
+  x = S[5].x;
+  
+  x = x + z[0];
+  y = z[0];
+  
+  return x + y;
+}
+
+/* { dg-final { scan-tree-dump-times "Lattice value changed to CONSTANT 0" 3 "store_ccp1" } } */
+/* { dg-final { cleanup-tree-dump "store_ccp*" } } */
+