Patchwork Allow MEM_REF lhs on gimple_clobber_p stmts (PR c++/34949)

login
register
mail settings
Submitter Jakub Jelinek
Date April 2, 2013, 11:05 a.m.
Message ID <20130402110550.GH20616@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/232947/
State New
Headers show

Comments

Jakub Jelinek - April 2, 2013, 11:05 a.m.
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?

(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.


	Jakub
Richard Guenther - April 2, 2013, 11:44 a.m.
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.
Jakub Jelinek - April 2, 2013, 12:18 p.m.
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
Richard Guenther - April 2, 2013, 12:41 p.m.
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.

Patch

--- 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));