This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix loop_only_exit_p


On Wed, 13 Jul 2016, Jan Hubicka wrote:

> > 
> > I'd rather not expose/change need_fake_edge_p as that has a very
> > specific purpose.
> > 
> > Why don't you simply add a call to is_ctrl_altering_stmt on the
> > last stmt of the block in loop_only_exit_p?  It's a waste of
> > time doing stuff on every stmt that can only make a difference
> > on the last one of a BB.
> 
> I am not sure I understand your suggestion. is_ctrl_altering_stmt checks
> whether stmt terminates BB (i.e. it does control flow we represent explicitly).
> Here we look for cases where BB execution can terminate in the middle of BB
> in ways not explicitly represented in CFG.
> 
> For example:
> 
>  ...stmt...
>  *ptr=0
>  ...stmt...
> 
> the control flow can terminate by the memory access that may throw non-call
> exception. 
> 
> or
> 
>  ...stmt...
>  volatile asm statement eventually leading to longjmp
>  ...stmt...
> 
> I worte RTL version of need_fake_edge_p many years ago to check particularly
> these cases. This is what loop code checks - it wants to know that the loop
> will eventually terminate by the exit edge on hand, not by other ways.

Ah, ok.  I dove into the code and that indeed seems to be the case.

Why is that function ignoring volatiles but not volatile asms or
old-style asms?  I think the only control-flow transfer asms are
allowed to make are asm gotos?

Anyway, that's a pre-existing issue so the patch is ok.

Thanks,
Richard.

> Honza
> > 
> > Richard.
> > 
> > > Honza
> > > 
> > > 	* gimple.h (stmt_can_terminate_bb_p): New function.
> > > 	* tree-cfg.c (need_fake_edge_p): Rename to ...
> > > 	(stmt_can_terminate_bb_p): ... this; return true if stmt can
> > > 	throw external; handle const and pure calls.
> > > 	* tree-ssa-loop-niter.c (loop_only_exit_p): Use it.
> > > Index: gimple.h
> > > ===================================================================
> > > --- gimple.h	(revision 238191)
> > > +++ gimple.h	(working copy)
> > > @@ -1526,6 +1526,7 @@ extern void gimple_seq_set_location (gim
> > >  extern void gimple_seq_discard (gimple_seq);
> > >  extern void maybe_remove_unused_call_args (struct function *, gimple *);
> > >  extern bool gimple_inexpensive_call_p (gcall *);
> > > +extern bool stmt_can_terminate_bb_p (gimple *);
> > >  
> > >  /* Formal (expression) temporary table handling: multiple occurrences of
> > >     the same scalar expression are evaluated into the same temporary.  */
> > > Index: tree-cfg.c
> > > ===================================================================
> > > --- tree-cfg.c	(revision 238191)
> > > +++ tree-cfg.c	(working copy)
> > > @@ -7919,15 +7919,20 @@ gimple_block_ends_with_condjump_p (const
> > >  }
> > >  
> > >  
> > > -/* Return true if we need to add fake edge to exit at statement T.
> > > -   Helper function for gimple_flow_call_edges_add.  */
> > > +/* Return true if statement T may terminate execution of BB in ways not
> > > +   explicitly represtented in the CFG.  */
> > >  
> > > -static bool
> > > -need_fake_edge_p (gimple *t)
> > > +bool
> > > +stmt_can_terminate_bb_p (gimple *t)
> > >  {
> > >    tree fndecl = NULL_TREE;
> > >    int call_flags = 0;
> > >  
> > > +  /* Eh exception not handled internally terminates execution of the whole
> > > +     function.  */
> > > +  if (stmt_can_throw_external (t))
> > > +    return true;
> > > +
> > >    /* NORETURN and LONGJMP calls already have an edge to exit.
> > >       CONST and PURE calls do not need one.
> > >       We don't currently check for CONST and PURE here, although
> > > @@ -7960,6 +7965,13 @@ need_fake_edge_p (gimple *t)
> > >        edge e;
> > >        basic_block bb;
> > >  
> > > +      if (call_flags & (ECF_PURE | ECF_CONST)
> > > +	  && !(call_flags & ECF_LOOPING_CONST_OR_PURE))
> > > +	return false;
> > > +
> > > +      /* Function call may do longjmp, terminate program or do other things.
> > > +	 Special case noreturn that have non-abnormal edges out as in this case
> > > +	 the fact is sufficiently represented by lack of edges out of T.  */
> > >        if (!(call_flags & ECF_NORETURN))
> > >  	return true;
> > >  
> > > @@ -8024,7 +8036,7 @@ gimple_flow_call_edges_add (sbitmap bloc
> > >        if (!gsi_end_p (gsi))
> > >  	t = gsi_stmt (gsi);
> > >  
> > > -      if (t && need_fake_edge_p (t))
> > > +      if (t && stmt_can_terminate_bb_p (t))
> > >  	{
> > >  	  edge e;
> > >  
> > > @@ -8059,7 +8071,7 @@ gimple_flow_call_edges_add (sbitmap bloc
> > >  	  do
> > >  	    {
> > >  	      stmt = gsi_stmt (gsi);
> > > -	      if (need_fake_edge_p (stmt))
> > > +	      if (stmt_can_terminate_bb_p (stmt))
> > >  		{
> > >  		  edge e;
> > >  
> > > Index: tree-ssa-loop-niter.c
> > > ===================================================================
> > > --- tree-ssa-loop-niter.c	(revision 238191)
> > > +++ tree-ssa-loop-niter.c	(working copy)
> > > @@ -2159,7 +2159,6 @@ loop_only_exit_p (const struct loop *loo
> > >    basic_block *body;
> > >    gimple_stmt_iterator bsi;
> > >    unsigned i;
> > > -  gimple *call;
> > >  
> > >    if (exit != single_exit (loop))
> > >      return false;
> > > @@ -2168,17 +2167,8 @@ loop_only_exit_p (const struct loop *loo
> > >    for (i = 0; i < loop->num_nodes; i++)
> > >      {
> > >        for (bsi = gsi_start_bb (body[i]); !gsi_end_p (bsi); gsi_next (&bsi))
> > > -	{
> > > -	  call = gsi_stmt (bsi);
> > > -	  if (gimple_code (call) != GIMPLE_CALL)
> > > -	    continue;
> > > -
> > > -	  if (gimple_has_side_effects (call))
> > > -	    {
> > > -	      free (body);
> > > -	      return false;
> > > -	    }
> > > -	}
> > > +	if (stmt_can_terminate_bb_p (gsi_stmt (bsi)))
> > > +	  return true;
> > >      }
> > >  
> > >    free (body);
> > >        if (EDGE_COUNT (bb->succs) > 1)
> > > 
> > > 
> > 
> > -- 
> > Richard Biener <rguenther@suse.de>
> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]