On Wed, Sep 05, 2012 at 09:10:53PM -0700, Andrew Pinski wrote: > The inlininer likes to recreate some MEM_REF, it copies most of the > bits (TREE_THIS_NOTRAP, TREE_THIS_VOLATILE, etc.) but forgets about > TREE_SIDE_EFFECTS. This causes the strlen optimization to think the > memory store does not have a side effects. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > Thanks, > Andrew Pinski > > ChangeLog: > > * tree-inline.c (remap_gimple_op_r): Copy TREE_SIDE_EFFECTS also. > > testsuite/ChangeLog: > * gcc.dg/tree-ssa/strlen-1.c: New testcase.
Patch preapproved, but you've attached a different patch. I'd say copy_tree_body_r's MEM_REF handling should also copy TREE_SIDE_EFFECTS/TREE_THIS_NOTRAP (what about TREE_READONLY?), maybe copy_decl_to_var/copy_result_decl_to_var should also copy TREE_SIDE_EFFECTS, perhaps omp-low.c copy_var_decl, install_var_field, tree-nested.c lookup_field_for_decl, tree-sra.c sra_ipa_reset_debug_stmts (grep TREE_THIS_VOLATILE.*TREE_THIS_VOLATILE, looking for missing TREE_SIDE_EFFECTS copy nearby). That said, perhaps the tree-ssa-strlen.c change is desirable too, unless we add some checking that TREE_THIS_VOLATILE references/decls have TREE_SIDE_EFFECTS bit set in the IL. > --- testsuite/gcc.c-torture/compile/pr49474.c (revision 0) > +++ testsuite/gcc.c-torture/compile/pr49474.c (revision 0) > --- cprop.c (revision 176187) > +++ cprop.c (working copy) Jakub