On Wed, 20 Nov 2019, Jakub Jelinek wrote: > Hi! > > I agree that we shouldn't have made __builtin_stack_{save,restore} public, > but I'd fear it is too late to remove it now (and replace by internal fn), > I'd think some code might use it to control when e.g. alloca will be > released. As the comment on insert_clobber* says, the addition of the > clobbers is a best effort, we could end up not finding the right spot and > not adding the CLOBBER in there even without user __builtin_* calls, so > this patch just removes an unnecessary assertion and just will not find > __builtin_stack_restore if something weird is seen, such as in the testcase > storing of the __builtin_stack_save result into memory, or could be > a cast or whatever else too. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK. Thanks, Richard. > 2019-11-19 Jakub Jelinek <ja...@redhat.com> > > PR c/90898 > * tree-ssa-ccp.c (insert_clobber_before_stack_restore): Remove > assertion. > (insert_clobbers_for_var): Fix a typo in function comment. > > * gcc.dg/pr90898.c: New test. > > --- gcc/tree-ssa-ccp.c.jj 2019-11-13 10:54:47.093020613 +0100 > +++ gcc/tree-ssa-ccp.c 2019-11-19 19:13:35.332705130 +0100 > @@ -2114,8 +2114,6 @@ insert_clobber_before_stack_restore (tre > else if (gimple_assign_ssa_name_copy_p (stmt)) > insert_clobber_before_stack_restore (gimple_assign_lhs (stmt), var, > visited); > - else > - gcc_assert (is_gimple_debug (stmt)); > } > > /* Advance the iterator to the previous non-debug gimple statement in the > same > @@ -2140,9 +2138,9 @@ gsi_prev_dom_bb_nondebug (gimple_stmt_it > /* Find a BUILT_IN_STACK_SAVE dominating gsi_stmt (I), and insert > a clobber of VAR before each matching BUILT_IN_STACK_RESTORE. > > - It is possible that BUILT_IN_STACK_SAVE cannot be find in a dominator > when a > - previous pass (such as DOM) duplicated it along multiple paths to a BB. > In > - that case the function gives up without inserting the clobbers. */ > + It is possible that BUILT_IN_STACK_SAVE cannot be found in a dominator > when > + a previous pass (such as DOM) duplicated it along multiple paths to a BB. > + In that case the function gives up without inserting the clobbers. */ > > static void > insert_clobbers_for_var (gimple_stmt_iterator i, tree var) > --- gcc/testsuite/gcc.dg/pr90898.c.jj 2019-11-19 19:18:02.277712801 +0100 > +++ gcc/testsuite/gcc.dg/pr90898.c 2019-11-19 19:18:52.613959787 +0100 > @@ -0,0 +1,16 @@ > +/* PR c/90898 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +void *p; > +int bar (void); > +void baz (int *); > + > +void > +foo (void) > +{ > + p = __builtin_stack_save (); > + int a[(bar (), 2)]; > + baz (a); > + __builtin_stack_restore (p); > +} > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)