Message ID | 20120328183936.GA2817@virgil.arch.suse.de |
---|---|
State | New |
Headers | show |
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
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