Hello,

This should have the changes you all asked for! Let me know if I
missed anything!

Thank you,
Matthew Beliveau

On Tue, Aug 13, 2019 at 5:15 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Tue, Aug 13, 2019 at 10:18 AM Richard Sandiford
> <richard.sandif...@arm.com> wrote:
> >
> > Thanks for doing this.
> >
> > Matthew Beliveau <mbeli...@redhat.com> writes:
> > > diff --git gcc/tree-ssa-dse.c gcc/tree-ssa-dse.c
> > > index 5b7c4fc6d1a..dcaeb8edbfe 100644
> > > --- gcc/tree-ssa-dse.c
> > > +++ gcc/tree-ssa-dse.c
> > > @@ -628,11 +628,8 @@ dse_optimize_redundant_stores (gimple *stmt)
> > >        tree fndecl;
> > >        if ((is_gimple_assign (use_stmt)
> > >          && gimple_vdef (use_stmt)
> > > -        && ((gimple_assign_rhs_code (use_stmt) == CONSTRUCTOR
> > > -             && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (use_stmt)) == 0
> > > -             && !gimple_clobber_p (stmt))
> > > -            || (gimple_assign_rhs_code (use_stmt) == INTEGER_CST
> > > -                && integer_zerop (gimple_assign_rhs1 (use_stmt)))))
> > > +        && initializer_zerop (gimple_op (use_stmt, 1), NULL)
> > > +        && !gimple_clobber_p (stmt))
> > >         || (gimple_call_builtin_p (use_stmt, BUILT_IN_NORMAL)
> > >             && (fndecl = gimple_call_fndecl (use_stmt)) != NULL
> > >             && (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_MEMSET
> > > @@ -1027,15 +1024,13 @@ dse_dom_walker::dse_optimize_stmt 
> > > (gimple_stmt_iterator *gsi)
> > >      {
> > >        bool by_clobber_p = false;
> > >
> > > -      /* First see if this store is a CONSTRUCTOR and if there
> > > -      are subsequent CONSTRUCTOR stores which are totally
> > > -      subsumed by this statement.  If so remove the subsequent
> > > -      CONSTRUCTOR store.
> > > +      /* Check if this store initalizes zero, or some aggregate of zeros,
> > > +      and check if there are subsequent stores which are subsumed by this
> > > +      statement.  If so, remove the subsequent store.
> > >
> > >        This will tend to make fewer calls into memset with longer
> > >        arguments.  */
> > > -      if (gimple_assign_rhs_code (stmt) == CONSTRUCTOR
> > > -       && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (stmt)) == 0
> > > +      if (initializer_zerop (gimple_op (stmt, 1), NULL)
> > >         && !gimple_clobber_p (stmt))
> > >       dse_optimize_redundant_stores (stmt);
> > >
> >
> > In addition to Jeff's comment, the original choice of gimple_assign_rhs1
> > is the preferred way to write this (applies to both hunks).
>
> And the !gimple_clobber_p test is now redundant.
>
> > Richard
Bootstrapped/regtested on x86_64-linux, ok for trunk?

2019-08-13  Matthew Beliveau  <mbeli...@redhat.com>

	* tree-ssa-dse.c (dse_optimize_redundant_stores): Improved check to
	catch more redundant zero initialization cases.
	(dse_dom_walker::dse_optimize_stmt): Likewise.
	
	* gcc.dg/tree-ssa/redundant-assign-zero-1.c: New test.
	* gcc.dg/tree-ssa/redundant-assign-zero-2.c: New test.

diff --git gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
new file mode 100644
index 00000000000..b8d01d1644b
--- /dev/null
+++ gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-dse-details" } */
+
+void blah (char *);
+
+void bar ()
+{
+  char a[256] = "";
+  a[3] = 0; 
+  blah (a);
+}
+
+/* { dg-final { scan-tree-dump-times "Deleted redundant store" 1 "dse1"} } */
diff --git gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c
new file mode 100644
index 00000000000..8cefa6f0cb7
--- /dev/null
+++ gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-dse-details" } */
+
+#include <string.h>
+
+void blahd (double *);
+
+void fubar ()
+{
+  double d;
+  double *x = &d;
+
+  memset (&d, 0 , sizeof d);
+  *x = 0.0;
+  blahd (x);
+}
+
+/* { dg-final { scan-tree-dump-times "Deleted redundant store" 1 "dse1"} } */
diff --git gcc/tree-ssa-dse.c gcc/tree-ssa-dse.c
index 5b7c4fc6d1a..14b66228e1e 100644
--- gcc/tree-ssa-dse.c
+++ gcc/tree-ssa-dse.c
@@ -628,11 +628,7 @@ dse_optimize_redundant_stores (gimple *stmt)
       tree fndecl;
       if ((is_gimple_assign (use_stmt)
 	   && gimple_vdef (use_stmt)
-	   && ((gimple_assign_rhs_code (use_stmt) == CONSTRUCTOR
-	        && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (use_stmt)) == 0
-	        && !gimple_clobber_p (stmt))
-	       || (gimple_assign_rhs_code (use_stmt) == INTEGER_CST
-		   && integer_zerop (gimple_assign_rhs1 (use_stmt)))))
+	   && initializer_zerop (gimple_assign_rhs1 (use_stmt)))
 	  || (gimple_call_builtin_p (use_stmt, BUILT_IN_NORMAL)
 	      && (fndecl = gimple_call_fndecl (use_stmt)) != NULL
 	      && (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_MEMSET
@@ -1027,16 +1023,10 @@ dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator *gsi)
     {
       bool by_clobber_p = false;
 
-      /* First see if this store is a CONSTRUCTOR and if there
-	 are subsequent CONSTRUCTOR stores which are totally
-	 subsumed by this statement.  If so remove the subsequent
-	 CONSTRUCTOR store.
-
-	 This will tend to make fewer calls into memset with longer
-	 arguments.  */
-      if (gimple_assign_rhs_code (stmt) == CONSTRUCTOR
-	  && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (stmt)) == 0
-	  && !gimple_clobber_p (stmt))
+      /* Check if this statement stores zero to a memory location,
+	 and if there is a subsequent store of zero to the same
+	 memory location.  If so, remove the subsequent store.  */
+      if (initializer_zerop (gimple_assign_rhs1 (stmt)))
 	dse_optimize_redundant_stores (stmt);
 
       /* Self-assignments are zombies.  */

Reply via email to