| Message ID | 20130402110550.GH20616@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <jakub@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.
On Tue, Apr 02, 2013 at 01:44:46PM +0200, Richard Biener wrote: > > @@ -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? gimple_store_p is young, so perhaps we can tweak its meaning. By changing gimple_store_p, we could drop this hunk, but perhaps would want to add early return for gimple_clobber_p into will_be_nonconstant_predicate? Your call. > > { > > 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? Say with -O2: struct A { virtual ~A (); char a[10]; }; struct B : public A { virtual ~B (); char b[10]; }; A::~A () { a[5] = 7; } B::~B () { b[5] = 8; } B b; we end up in ~B with: this_3(D)->D.2229._vptr.A = &MEM[(void *)&_ZTV1B + 16B]; this_3(D)->b[5] = 8; _6 = &this_3(D)->D.2229; MEM[(struct A *)this_3(D)]._vptr.A = &MEM[(void *)&_ZTV1A + 16B]; MEM[(struct A *)this_3(D)].a[5] = 7; MEM[(struct A *)this_3(D)] ={v} {CLOBBER}; *this_3(D) ={v} {CLOBBER}; and the intent of the above hunk (and the one immediately below this) is that we DSE the first clobber (with smaller clobbered size), something that IMHO happens very frequently in C++ code, and allows us to better DSEthe other stores. There is no point in keeping the smaller clobbers around, on the other side, without the second hunk we'd remove pretty much all the MEM_REF clobbers at the end of functions, which is undesriable. > > 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"? See above. > > @@ -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 MEM_REF[&MEM_REF[...], 0] and similar won't ever get through, perhaps. > > > + 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?) The above change fixed tons of ICEs, fnsplit pass ignored clobber stmts (desirable) when deciding what to split, and end up copying the clobber into the *.part.0 function, but because nothing but the clobber used the this parameter, it wasn't passed. So, we ended up first referencing a default definition of a local this variable (just useless stmt), and later on when tree-ssa-live.c run we've ignored the clobber again for decisions, so that local this variable became !is_used_p and we've ended up referencing in-free-list SSA_NAME, which triggered assertion failure. See a few lines above this where we similarly remove !is_used_p VAR_DECLs. So, IMHO the !is_used_p code belongs to this spot, we can do the clobber with SSA_NAME_IS_DEFAULT_DEF (op) && TREE_CODE (SSA_NAME_VAR (op)) != PARM_DECL) MEM_REF removal in DCE (or both DCE and here). Without this code in tree-ssa-live.c we couldn't do: if (gimple_clobber_p (stmt)) { have_local_clobbers = true; continue; } in remove_unused_locals safely (i.e. don't bother with mark_all_vars_used on the lhs of the clobber). > 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. Sure, MEM_REF[&decl] I've seen frequently, but unless it is a whole var access and not say clobbering of just a portion of the var, we want to treat it as the patch does for DSE reasons, but not for variable coalescing reasons during expansion. Perhaps for MEM_REF[&decl, 0] with the size of access the same as decl's size cfgexpand.c could treat it as a clobber for the whole decl (but then, won't we have there a non-MEM_REF clobber in that case after it too and DSE supposedly already removed the first MEM_REF clobber?). I mean like adding void bar (B &); void foo () { { B b; bar (b); } { B c; bar (c); } { B d; bar (d); } } to the above testcase. Jakub
On Tue, 2 Apr 2013, Jakub Jelinek wrote: > On Tue, Apr 02, 2013 at 01:44:46PM +0200, Richard Biener wrote: > > > @@ -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? > > gimple_store_p is young, so perhaps we can tweak its meaning. > By changing gimple_store_p, we could drop this hunk, but perhaps > would want to add early return for gimple_clobber_p > into will_be_nonconstant_predicate? Your call. I've just come along 56787 and decided that not collecting CLOBBERs during data-reference gathering would be wrong. Thus changing gimple_store_p would be wrong, too. So in the end the above hunk looks ok and we shouldn't change gimple_store_p. (well, for now at least) > > > { > > > 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? > > Say with -O2: > struct A { virtual ~A (); char a[10]; }; > struct B : public A { virtual ~B (); char b[10]; }; > A::~A () { a[5] = 7; } > B::~B () { b[5] = 8; } > B b; > we end up in ~B with: > this_3(D)->D.2229._vptr.A = &MEM[(void *)&_ZTV1B + 16B]; > this_3(D)->b[5] = 8; > _6 = &this_3(D)->D.2229; > MEM[(struct A *)this_3(D)]._vptr.A = &MEM[(void *)&_ZTV1A + 16B]; > MEM[(struct A *)this_3(D)].a[5] = 7; > MEM[(struct A *)this_3(D)] ={v} {CLOBBER}; > *this_3(D) ={v} {CLOBBER}; > and the intent of the above hunk (and the one immediately below this) > is that we DSE the first clobber (with smaller clobbered size), something > that IMHO happens very frequently in C++ code, and allows us to better > DSEthe other stores. There is no point in keeping the smaller clobbers > around, on the other side, without the second hunk we'd remove pretty much > all the MEM_REF clobbers at the end of functions, which is undesriable. Ah, ok. > > > 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"? > > See above. > > > > @@ -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 MEM_REF[&MEM_REF[...], 0] and similar won't ever get through, perhaps. It won't, that's not valid GIMPLE. See is_gimple_mem_ref_addr (). > > > > > + 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?) > > The above change fixed tons of ICEs, fnsplit pass ignored clobber stmts > (desirable) when deciding what to split, and end up copying the clobber > into the *.part.0 function, but because nothing but the clobber used > the this parameter, it wasn't passed. So, we ended up first referencing > a default definition of a local this variable (just useless stmt), and later > on when tree-ssa-live.c run we've ignored the clobber again for decisions, > so that local this variable became !is_used_p and we've ended up referencing > in-free-list SSA_NAME, which triggered assertion failure. See a few lines > above this where we similarly remove !is_used_p VAR_DECLs. > So, IMHO the !is_used_p code belongs to this spot, we can do the clobber > with > SSA_NAME_IS_DEFAULT_DEF (op) && TREE_CODE (SSA_NAME_VAR (op)) != PARM_DECL) > MEM_REF removal in DCE (or both DCE and here). > Without this code in tree-ssa-live.c we couldn't do: > if (gimple_clobber_p (stmt)) > { > have_local_clobbers = true; > continue; > } > in remove_unused_locals safely (i.e. don't bother with mark_all_vars_used > on the lhs of the clobber). Ok, note that I didn't object to the SSA_NAME_DEFAULT_DEF handling but to checking something on SSA_NAME_VAR - sure, if SSA_NAME_VAR is unused then each remaining SSA name must be a default definition. So ... what about removing the whole case handling unused SSA_NAME_VAR? > > 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. > > Sure, MEM_REF[&decl] I've seen frequently, but unless it is a whole > var access and not say clobbering of just a portion of the var, we want > to treat it as the patch does for DSE reasons, but not for variable > coalescing reasons during expansion. Perhaps for MEM_REF[&decl, 0] with > the size of access the same as decl's size cfgexpand.c could treat it as a > clobber for the whole decl (but then, won't we have there a non-MEM_REF > clobber in that case after it too and DSE supposedly already removed the > first MEM_REF clobber?). I mean like adding > void bar (B &); > void > foo () > { > { B b; bar (b); } { B c; bar (c); } { B d; bar (d); } > } > to the above testcase. I was only suggesting that because you only handle DECL = CLOBBER and MEM[ssa_ptr_2] = CLOBBER in tree-ssa-live.c you miss removing (partial) clobbers of the MEM[&decl, offset] = CLOBBER kind when DECL is otherwise unused. I believe this can easily occur with in-place construction / destruction. A very easy fix for that is to use get_base_address on the LHS of the CLOBBER. Richard.
--- 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)) { 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; 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; + /* 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) + if (SSA_NAME_VAR (op) + && TREE_CODE (SSA_NAME_VAR (op)) == VAR_DECL + && !is_used_p (SSA_NAME_VAR (op))) + remove = true; + 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));