On Tue, 2 Apr 2013, Jakub Jelinek wrote: > Hi! > > Jason's http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949#c12 > patch attempts to emit a clobber for *this at the end of the destructor, > so that we can DSE stores into the object when the object is dead, but > so far only VAR_DECLs have been allowed on the lhs of gimple_clobber_p > stmts. > This incremental patch allows there either a VAR_DECL, or MEM_REF. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Few comments below > (Jason's patch can be just committed as the last thing in the series). > > 2013-04-02 Jakub Jelinek <ja...@redhat.com> > > PR c++/34949 > * tree-cfg.c (verify_gimple_assign_single): Allow lhs > of gimple_clobber_p to be MEM_REF. > * gimplify.c (gimplify_modify_expr): Gimplify *to_p of > an assignment from TREE_CLOBBER_P. Allow it to be MEM_REF > after gimplification. > * asan.c (get_mem_ref_of_assignment): Don't instrument > gimple_clobber_p stmts. > * tree-ssa-dse.c (dse_optimize_stmt): Allow DSE of > gimple_clobber_p stmt if they have MEM_REF lhs and > are dead because of another gimple_clobber_p stmt. > * tree-ssa-live.c (clear_unused_block_pointer): Treat > gimple_clobber_p stmts like debug stmts. > (remove_unused_locals): Remove clobbers with MEM_REF lhs > that refer to unused VAR_DECLs or uninitialized values. > * tree-sra.c (sra_ipa_reset_debug_stmts): Also remove > gimple_clobber_p stmts if they refer to removed parameters. > (get_repl_default_def_ssa_name, sra_ipa_modify_expr): Fix up > formatting. > > --- gcc/tree-cfg.c.jj 2013-03-21 18:34:11.000000000 +0100 > +++ gcc/tree-cfg.c 2013-03-27 13:44:59.475007635 +0100 > @@ -3812,9 +3812,9 @@ verify_gimple_assign_single (gimple stmt > } > > if (gimple_clobber_p (stmt) > - && !DECL_P (lhs)) > + && !(DECL_P (lhs) || TREE_CODE (lhs) == MEM_REF)) > { > - error ("non-decl LHS in clobber statement"); > + error ("non-decl/MEM_REF LHS in clobber statement"); > debug_generic_expr (lhs); > return true; > } > --- gcc/gimplify.c.jj 2013-03-21 18:34:11.000000000 +0100 > +++ gcc/gimplify.c 2013-03-27 13:38:30.988181267 +0100 > @@ -4840,7 +4840,12 @@ gimplify_modify_expr (tree *expr_p, gimp > so handle it here. */ > if (TREE_CLOBBER_P (*from_p)) > { > - gcc_assert (!want_value && TREE_CODE (*to_p) == VAR_DECL); > + ret = gimplify_expr (to_p, pre_p, post_p, is_gimple_lvalue, fb_lvalue); > + if (ret == GS_ERROR) > + return ret; > + gcc_assert (!want_value > + && (TREE_CODE (*to_p) == VAR_DECL > + || TREE_CODE (*to_p) == MEM_REF)); > gimplify_seq_add_stmt (pre_p, gimple_build_assign (*to_p, *from_p)); > *expr_p = NULL; > return GS_ALL_DONE; > --- gcc/asan.c.jj 2013-02-28 22:22:03.000000000 +0100 > +++ gcc/asan.c 2013-03-27 14:29:54.531785577 +0100 > @@ -412,7 +412,8 @@ get_mem_ref_of_assignment (const gimple > { > gcc_assert (gimple_assign_single_p (assignment)); > > - if (gimple_store_p (assignment)) > + if (gimple_store_p (assignment) > + && !gimple_clobber_p (assignment)) hmm, maybe gimple_store_p should not consider a clobber a store? > { > ref->start = gimple_assign_lhs (assignment); > *ref_is_store = true; > --- gcc/tree-ssa-dse.c.jj 2013-01-11 09:02:37.000000000 +0100 > +++ gcc/tree-ssa-dse.c 2013-03-27 17:14:27.424466023 +0100 > @@ -218,7 +218,10 @@ dse_optimize_stmt (gimple_stmt_iterator > if (is_gimple_call (stmt) && gimple_call_fndecl (stmt)) > return; > > - if (gimple_has_volatile_ops (stmt)) > + /* Don't return early on *this_2(D) ={v} {CLOBBER}. */ > + if (gimple_has_volatile_ops (stmt) > + && (!gimple_clobber_p (stmt) > + || TREE_CODE (gimple_assign_lhs (stmt)) != MEM_REF)) > return; But this only means the clobber was not considered as "dead" if there was a later store to the same location. So, why would that change be needed? > if (is_gimple_assign (stmt)) > @@ -228,6 +231,12 @@ dse_optimize_stmt (gimple_stmt_iterator > if (!dse_possible_dead_store_p (stmt, &use_stmt)) > return; > > + /* But only remove *this_2(D) ={v} {CLOBBER} if killed by > + another clobber stmt. */ > + if (gimple_clobber_p (stmt) > + && !gimple_clobber_p (use_stmt)) > + return; Ah. Hmm, can that ever happen? I presume the issue with non-decl clobbers is that we will never remove them, even if the storage becomes "dead"? > /* If we have precisely one immediate use at this point and the > stores are to the same memory location or there is a chain of > virtual uses from stmt and the stmt which stores to that same > --- gcc/tree-ssa-live.c.jj 2013-03-21 18:34:11.000000000 +0100 > +++ gcc/tree-ssa-live.c 2013-03-28 11:10:15.889669796 +0100 > @@ -623,8 +623,8 @@ clear_unused_block_pointer_1 (tree *tp, > return NULL_TREE; > } > > -/* Set all block pointer in debug stmt to NULL if the block is unused, > - so that they will not be streamed out. */ > +/* Set all block pointer in debug or clobber stmt to NULL if the block > + is unused, so that they will not be streamed out. */ > > static void > clear_unused_block_pointer (void) > @@ -639,7 +639,7 @@ clear_unused_block_pointer (void) > tree b; > gimple stmt = gsi_stmt (gsi); > > - if (!is_gimple_debug (stmt)) > + if (!is_gimple_debug (stmt) && !gimple_clobber_p (stmt)) > continue; > b = gimple_block (stmt); > if (b && !TREE_USED (b)) > @@ -827,7 +827,26 @@ remove_unused_locals (void) > if (gimple_clobber_p (stmt)) > { > tree lhs = gimple_assign_lhs (stmt); > + bool remove = false; > + /* Remove clobbers referencing unused vars, or clobbers > + with MEM_REF lhs referencing unused vars or uninitialized > + pointers. */ > if (TREE_CODE (lhs) == VAR_DECL && !is_used_p (lhs)) > + remove = true; > + else if (TREE_CODE (lhs) == MEM_REF) > + { > + ssa_op_iter iter; > + tree op; > + FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_USE) The only SSA use on a clobber stmt can possibly be TREE_OPERAND (lhs, 0) when lhs is a MEM_REF, no need to use FOR_EACH_SSA_TREE_OPERAND. So just > + else if (TREE_CODE (lhs) == MEM_REF && TREE_CODE (TREE_OPERAND (lhs, 0)) == SSA_NAME) ... > + if (SSA_NAME_VAR (op) > + && TREE_CODE (SSA_NAME_VAR (op)) == VAR_DECL > + && !is_used_p (SSA_NAME_VAR (op))) > + remove = true; I'm not sure this is really desired ... isn't a better check to do has_single_use (op)? (which means it would be better suited to be handled in DCE?) Btw, due to propagation you can end up with MEM[&decl] = {CLOBBER}; which should be handled the same as the VAR_DECL case. Eventually just use lhs = get_base_address (gimple_assign_lhs (stmt)) here. > + else if (SSA_NAME_IS_DEFAULT_DEF (op) > + && TREE_CODE (SSA_NAME_VAR (op)) != PARM_DECL) > + remove = true; > + } > + if (remove) > { > unlink_stmt_vdef (stmt); > gsi_remove (&gsi, true); > --- gcc/tree-sra.c.jj 2013-03-21 18:34:11.000000000 +0100 > +++ gcc/tree-sra.c 2013-04-02 09:44:29.225462112 +0200 > @@ -2965,8 +2965,8 @@ sra_modify_constructor_assign (gimple *s > static tree > get_repl_default_def_ssa_name (struct access *racc) > { > - gcc_checking_assert (!racc->grp_to_be_replaced && > - !racc->grp_to_be_debug_replaced); > + gcc_checking_assert (!racc->grp_to_be_replaced > + && !racc->grp_to_be_debug_replaced); > if (!racc->replacement_decl) > racc->replacement_decl = create_access_replacement (racc); > return get_or_create_ssa_default_def (cfun, racc->replacement_decl); > @@ -4462,8 +4462,8 @@ sra_ipa_modify_expr (tree *expr, bool co > { > adj = &adjustments[i]; > > - if (adj->base == base && > - (adj->offset == offset || adj->remove_param)) > + if (adj->base == base > + && (adj->offset == offset || adj->remove_param)) > { > cand = adj; > break; > @@ -4676,6 +4676,14 @@ sra_ipa_reset_debug_stmts (ipa_parm_adju > if (name) > FOR_EACH_IMM_USE_STMT (stmt, ui, name) > { > + if (gimple_clobber_p (stmt)) > + { > + gimple_stmt_iterator cgsi = gsi_for_stmt (stmt); > + unlink_stmt_vdef (stmt); > + gsi_remove (&cgsi, true); > + release_defs (stmt); > + continue; > + } > /* All other users must have been removed by > ipa_sra_modify_function_body. */ > gcc_assert (is_gimple_debug (stmt)); Otherwise the patch looks fine to me. Thanks, Richard.