Non-dominating loop bounds in tree-ssa-loop-niter 3/4

[Richard Biener]

On Wed, 31 Oct 2012, Jan Hubicka wrote:

> > > Index: tree-ssa-loop-niter.c
> > > ===================================================================
> > > --- tree-ssa-loop-niter.c	(revision 192989)
> > > @@ -3505,15 +3737,11 @@ scev_probably_wraps_p (tree base, tree s
> > >    return true;
> > >  }
> > >  
> > > -/* Frees the information on upper bounds on numbers of iterations of LOOP.  */
> > > -
> > 
> > Needs a comment.
> 
> Yep,
> also the reason I export it is because I use in other patch.
> (I think we should not hook the bounds into the loop structure and keep them dead,
> so i want the max_iter*friends to free them unless they are asked to preserve and
> explicitely free in the two users of them).
> > > +static bool
> > > +remove_exits_and_undefined_stmts (struct loop *loop, unsigned int npeeled)
> > > +{
> > > +  struct nb_iter_bound *elt;
> > > +  bool changed = false;
> > > +
> > > +  for (elt = loop->bounds; elt; elt = elt->next)
> > > +    {
> > > +      /* If statement is known to be undefined after peeling, turn it
> > > +	 into unreachable (or trap when debugging experience is supposed
> > > +	 to be good).  */
> > > +      if (!elt->is_exit
> > > +	  && elt->bound.ule (double_int::from_uhwi (npeeled)))
> > > +	{
> > > +	  gimple_stmt_iterator gsi = gsi_for_stmt (elt->stmt);
> > > +	  gimple stmt = gimple_build_call
> > > +	      (builtin_decl_implicit (optimize_debug
> > > +				      ? BUILT_IN_TRAP : BUILT_IN_UNREACHABLE),
> > 
> > I'm not sure we should do different things for -Og here.  In fact there
> > is no unrolling done for -Og at all, so just use unreachable.
> 
> OK, I was thinking about BUILT_IN_TRAP for -O1 too, but I am not sure either.
> i guess we will gather some experience on how much are users confused.
> 
> Why we do ont inline at -Og?

unroll you mean.  Because unrolling mutates the CFG too much.
Well - it was just a starting point, populating -Og with as little
as possible and 100% profitable transforms (in both debug and speed
metric).  In late opts we only do (early opt queue is shared):

  NEXT_PASS (pass_all_optimizations_g);
    {
      struct opt_pass **p = &pass_all_optimizations_g.pass.sub;
      NEXT_PASS (pass_remove_cgraph_callee_edges);
      NEXT_PASS (pass_strip_predict_hints);
      /* Lower remaining pieces of GIMPLE.  */
      NEXT_PASS (pass_lower_complex);
      NEXT_PASS (pass_lower_vector_ssa);
      /* Perform simple scalar cleanup which is constant/copy propagation.  
*/
      NEXT_PASS (pass_ccp);
      NEXT_PASS (pass_object_sizes);
      /* Copy propagation also copy-propagates constants, this is 
necessary
         to forward object-size results properly.  */
      NEXT_PASS (pass_copy_prop);
      NEXT_PASS (pass_rename_ssa_copies);
      NEXT_PASS (pass_dce);
      /* Fold remaining builtins.  */
      NEXT_PASS (pass_fold_builtins);
      /* ???  We do want some kind of loop invariant motion, but we 
possibly
         need to adjust LIM to be more friendly towards preserving 
accurate
         debug information here.  */
      NEXT_PASS (pass_late_warn_uninitialized);
      NEXT_PASS (pass_uncprop);
      NEXT_PASS (pass_local_pure_const);
    }

> > > +      /* If we know the exit will be taken after peeling, update.  */
> > > +      else if (elt->is_exit
> > > +	       && elt->bound.ule (double_int::from_uhwi (npeeled)))
> > > +	{
> > > +	  basic_block bb = gimple_bb (elt->stmt);
> > > +	  edge exit_edge = EDGE_SUCC (bb, 0);
> > > +
> > > +	  if (dump_file && (dump_flags & TDF_DETAILS))
> > > +	    {
> > > +	      fprintf (dump_file, "Forced exit to be taken: ");
> > > +	      print_gimple_stmt (dump_file, elt->stmt, 0, 0);
> > > +	    }
> > > +	  if (!loop_exit_edge_p (loop, exit_edge))
> > > +	    exit_edge = EDGE_SUCC (bb, 1);
> > > +	  if (exit_edge->flags & EDGE_TRUE_VALUE)
> > 
> > I think we can have abnormal/eh exit edges.  So I'm not sure you
> > can, without checking, assume the block ends in a GIMPLE_COND.
> 
> We can't - those are only edges that are found by the IV analysis and they
> always test IV with some bound. (it is all done by number_of_iterations_exit)
> > 
> > See above.  Otherwise the overall idea sounds fine.
> 
> Similarly here, simple exits are always conditionals. 

I see.  Then the patch is ok (with the comment added).

Thanks,
Richard.