Message ID | CA+=Sn1=Tw68CwdNrNSwUo57hme7M6wEVf9fZq0B-4Egunmu-Ww@mail.gmail.com |
---|---|
State | New |
Headers | show |
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
Index: testsuite/gcc.c-torture/compile/pr49474.c =================================================================== --- testsuite/gcc.c-torture/compile/pr49474.c (revision 0) +++ testsuite/gcc.c-torture/compile/pr49474.c (revision 0) @@ -0,0 +1,16 @@ +typedef struct gfc_formal_arglist +{ + int next; +} +gfc_actual_arglist; +update_arglist_pass (gfc_actual_arglist* lst, int po, unsigned argpos, + const char *name) +{ + ((void)(__builtin_expect(!(argpos > 0), 0) ? __builtin_unreachable(), 0 : 0)); + if (argpos == 1) + return 0; + if (lst) + lst->next = update_arglist_pass (lst->next, po, argpos - 1, name); + else + lst = update_arglist_pass (((void *)0), po, argpos - 1, name); +} Index: cprop.c =================================================================== --- cprop.c (revision 176187) +++ cprop.c (working copy) @@ -1332,7 +1332,7 @@ find_implicit_sets (void) FOR_EACH_BB (bb) { /* Check for more than one successor. */ - if (! EDGE_COUNT (bb->succs) > 1) + if (EDGE_COUNT (bb->succs) <= 1) continue; cond = fis_get_condition (BB_END (bb));