Remove bogus assert from CCP's insert_clobbers_for_var

Hi,

when testing a different patch of mine I hit the assert in
insert_clobbers_for_var which is there to make sure that there is a
call to builtin_stack_save in a BB with or dominating a call to
builtin_alloca_with_align.  In my case that was not true because the
DOM pass duplicated the call to builtin_stack_save and put it onto two
different paths to the same BB.

On IRC I've been told that is OK and the that CCP cannot make such
assumtions.  Since it is only a missed-optimization if the call to the
builtin is not found and processed (basically PR 51491 again but only
in cases like these), I thought it best to just remove the assert by
the following simple patch, bootstrapped and tested on x86_64-linux.

OK for trunk?

Thanks,

Martin


2012-03-28  Martin Jambor  <mjambor@suse.cz>

	* tree-ssa-ccp.c (insert_clobbers_for_var): Do not assert that there
	is a builtin_stack_save in a dominating BB.

On Wed, 28 Mar 2012, Martin Jambor wrote:

> Hi,
> 
> when testing a different patch of mine I hit the assert in
> insert_clobbers_for_var which is there to make sure that there is a
> call to builtin_stack_save in a BB with or dominating a call to
> builtin_alloca_with_align.  In my case that was not true because the
> DOM pass duplicated the call to builtin_stack_save and put it onto two
> different paths to the same BB.
> 
> On IRC I've been told that is OK and the that CCP cannot make such
> assumtions.  Since it is only a missed-optimization if the call to the
> builtin is not found and processed (basically PR 51491 again but only
> in cases like these), I thought it best to just remove the assert by
> the following simple patch, bootstrapped and tested on x86_64-linux.
> 
> OK for trunk?

Ok, but please add a comment before the loop why it is ok and why
it can happen that we do not find a stack-save.

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> 2012-03-28  Martin Jambor  <mjambor@suse.cz>
> 
> 	* tree-ssa-ccp.c (insert_clobbers_for_var): Do not assert that there
> 	is a builtin_stack_save in a dominating BB.
> 
> Index: src/gcc/tree-ssa-ccp.c
> ===================================================================
> --- src.orig/gcc/tree-ssa-ccp.c
> +++ src/gcc/tree-ssa-ccp.c
> @@ -1769,18 +1769,16 @@ gsi_prev_dom_bb_nondebug (gimple_stmt_it
>  static void
>  insert_clobbers_for_var (gimple_stmt_iterator i, tree var)
>  {
> -  bool save_found;
>    gimple stmt;
>    tree saved_val;
>    htab_t visited = NULL;
>  
> -  for (save_found = false; !gsi_end_p (i); gsi_prev_dom_bb_nondebug (&i))
> +  for (; !gsi_end_p (i); gsi_prev_dom_bb_nondebug (&i))
>      {
>        stmt = gsi_stmt (i);
>  
>        if (!gimple_call_builtin_p (stmt, BUILT_IN_STACK_SAVE))
>  	continue;
> -      save_found = true;
>  
>        saved_val = gimple_call_lhs (stmt);
>        if (saved_val == NULL_TREE)
> @@ -1792,7 +1790,6 @@ insert_clobbers_for_var (gimple_stmt_ite
>  
>    if (visited != NULL)
>      htab_delete (visited);
> -  gcc_assert (save_found);
>  }
>  
>  /* Detects a __builtin_alloca_with_align with constant size argument.  Declares
> 
>

> On IRC I've been told that is OK and the that CCP cannot make such
> assumtions.  Since it is only a missed-optimization if the call to the
> builtin is not found and processed (basically PR 51491 again but only
> in cases like these), I thought it best to just remove the assert by
> the following simple patch, bootstrapped and tested on x86_64-linux.

Please do that on the 4.7 branch as well if the assertion is incorrect.

Hi,

On Fri, Mar 30, 2012 at 01:23:00PM +0200, Eric Botcazou wrote:
> > On IRC I've been told that is OK and the that CCP cannot make such
> > assumtions.  Since it is only a missed-optimization if the call to the
> > builtin is not found and processed (basically PR 51491 again but only
> > in cases like these), I thought it best to just remove the assert by
> > the following simple patch, bootstrapped and tested on x86_64-linux.
> 
> Please do that on the 4.7 branch as well if the assertion is incorrect.

Done.

Thanks,

Martin

> 
> -- 
> Eric Botcazou

> > Please do that on the 4.7 branch as well if the assertion is incorrect.
>
> Done.

Thanks.  I've now reverted the kludge I put for Ada because of it:
  http://gcc.gnu.org/ml/gcc-patches/2012-03/msg01616.html