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. */