Re: [PATCH 71/89] Concretize gimple_cond_make_{false|true}

[David Malcolm <dmalcolm at redhat dot com>]

On Mon, 2014-05-12 at 13:55 -0600, Jeff Law wrote:
> On 04/21/14 10:57, David Malcolm wrote:
> > gcc/
> > 	* gimple.h (gimple_cond_make_false): Require a gimple_cond.
> > 	(gimple_cond_make_true): Likewise.
> >
> > 	* tree-cfg.c (fold_cond_expr_cond): Add a checked cast to
> > 	gimple_cond within region guarded by check for GIMPLE_COND.
> > 	* tree-ssa-ccp.c (ccp_fold_stmt): Likewise.
> >
> > 	* tree-loop-distribution.c (generate_loops_for_partition): Replace
> > 	a check for GIMPLE_COND with a dyn_cast_gimple_cond.
> > 	* tree-ssa-ccp.c (optimize_unreachable): Likewise.
> > 	* tree-ssa-loop-niter.c (number_of_iterations_exit): Likewise.
> > 	* tree-ssa-pre.c (eliminate_dom_walker::before_dom_children):
> > 	Likewise.
> >
> > 	* tree-vrp.c (fold_predicate_in): Add a checked cast to
> > 	gimple_cond.  We must be dealing with a GIMPLE_COND since logic
> > 	at top of the function ensures we only act on GIMPLE_ASSIGN and
> > 	GIMPLE_COND statements, and we're now within a "not a GIMPLE_ASSIGN"
> > 	clause.
> >
> > 	* tree-ssa-loop-ivcanon.c (remove_exits_and_undefined_stmts): Add
> > 	checked cast of elt->stmt to gimple_cond.  The existing code requires
> > 	this to be a GIMPLE_COND, though it's not clear to me how this
> > 	requirement is enforced.

You asked me to speak up if my factoring turned up a case where the
existing code might be broken - and I'm slightly nervous about the last
change above.

AIUI, the invariants that hold here are that we're walking the
singly-linked list of the (struct nb_iter_bound *) of a loop, and have a
loop exit - and the existing code assumed that the nb_iter_bound's stmt
is a GIMPLE_COND.

Could that ever not be the case; could we ever be dealing with another
kind of statement?

As I understand it, the "is_exit" field is set by "record_estimate",
which is called by "estimate_numbers_of_iterations_loop" on each element
of get_loop_exit_edges (loop).

Is it possible for, say, a suitably-constructed function full of gotos
and a switch to fool the loop-handling code into considering a switch
statement as an exit from a loop? (e.g. after optimizing a goto inside a
case, maybe?).

Then again, the existing code calls gimple_cond_make_true/make_false on
the stmt, so implicitly requires it to be a GIMPLE_COND.

Sorry for my ignorance here; I'm less familiar with the loop-analysis
code.

> > 	(remove_redundant_iv_tests): Likewise.
> > 	(try_unroll_loop_completely): Likewise, for the last_stmt of the
> > 	preceding bb along edge_to_cancel.
> > 	* tree-ssa-reassoc.c (maybe_optimize_range_tests): Likewise, for the
> > 	last_stmt of bb.
> OK once prerequisites go in.

Thanks.