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 Mon, 11 Jul 2016, Jan Hubicka wrote:

> Hi,
> while looking into loop code I noticed that loop_only_exit_p seems overly
> simplistic. The purpose of this predicate is to decide whether the loop
> must terminate by the exit given (so for example it is known that the number
> of iteration test in that particular exit won't overflow because undefined
> effect will happen then).
> 
> The code checks for calls with side effects, but it misses the fact that loop
> can exit by an external EH (with -fnon-call-exceptions) or by an asm statement.
> Exactly the same problem is being solved by the gcov code where it is necessary
> which tries to verify that given the first statement of bb is executed, the
> control flow will continue by one of the outgoing edges (which is necesssary
> to make Kirhoff law applicable).
> 
> This patch thus unifies the logic and also fixes probably ages long bug about
> ignoring external EH in profiling code.
> 
> Bootstrapped/regtested x86_64-linux, seems sane?

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.

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)


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