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 <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
