Re: [COMMITTED] [PATCH 1/2] Remove recursion from range_from_dom.

[Mikael Morin <morin-mikael@orange.fr>]

Hello,

I spotted a few nits.  See below.

Le 20/07/2022 à 00:10, Andrew MacLeod via Gcc-patches a écrit :
> diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
> index da7b8055d42..20dd5ead3bc 100644
> --- a/gcc/gimple-range-cache.cc
> +++ b/gcc/gimple-range-cache.cc
> @@ -1312,6 +1312,38 @@ ranger_cache::fill_block_cache (tree name, basic_block bb, basic_block def_bb)
>      fprintf (dump_file, "  Propagation update done.\n");
>  }
>  
> +// Resolve the range of BB if the dominators range is R by calculating incoming
> +// edges to this block.  All lead back to the dominator so should be cheap.
> +// The range for BB is set and returned in R.
> +
> +void
> +ranger_cache::resolve_dom (vrange &r, tree name, basic_block bb)
> +{
> +  basic_block def_bb = gimple_bb (SSA_NAME_DEF_STMT (name));
> +  basic_block dom_bb = get_immediate_dominator (CDI_DOMINATORS, bb);
> +
> +  // if it doesn't already have a value, store the incoming range.
> +  if (!m_on_entry.bb_range_p (name, dom_bb) && def_bb != dom_bb)
> +    {
> +      // If the range can't be store, don't try to accumulate
> +      // the range in PREV_BB due to excessive recalculations.

As a consequence of the refactoring, PREV_BB doesn’t exist anymore.  It 
should be BB, I think.

> +      if (!m_on_entry.set_bb_range (name, dom_bb, r))
> +	return;
> +    }
> +  // With the dominator set, we should be able to cheaply query
> +  // each incoming edge now and accumulate the results.
> +  r.set_undefined ();
> +  edge e;
> +  edge_iterator ei;
> +  Value_Range er (TREE_TYPE (name));
> +  FOR_EACH_EDGE (e, ei, bb->preds)
> +    {
> +      edge_range (er, e, name, RFD_READ_ONLY);
> +      r.union_ (er);
> +    }
> +  // Set the cache in PREV_BB so it is not calculated again.

Same here.

> +  m_on_entry.set_bb_range (name, bb, r);
> +}
>  
>  // Get the range of NAME from dominators of BB and return it in R.  Search the
>  // dominator tree based on MODE.

(...)

> @@ -1403,14 +1402,25 @@ ranger_cache::range_from_dom (vrange &r, tree name, basic_block start_bb,
>  	fprintf (dump_file, " at function top\n");
>      }
>  
> -  // Now process any outgoing edges that we seen along the way.
> +  // Now process any blocks wit incoming edges that nay have adjustemnts.
>    while (m_workback.length () > start_limit)
>      {
>        int_range_max er;
>        prev_bb = m_workback.pop ();
> +      if (!single_pred_p (prev_bb))
> +	{
> +	  // Non single pred means we need to cache a vsalue in the dominator

... cache a *value* in ...

> +	  // so we can cheaply calculate incoming edges to this block, and
> +	  // then store the resulting value.  If processing mode is not
> +	  // RFD_FILL, then the cache cant be stored to, so don't try.
> +	  // Otherwise this becomes a quadratic timed calculation.
> +	  if (mode == RFD_FILL)
> +	    resolve_dom (r, name, prev_bb);
> +	  continue;
> +	}
> +
>        edge e = single_pred_edge (prev_bb);
>        bb = e->src;
> -
>        if (m_gori.outgoing_edge_range_p (er, e, name, *this))
>  	{
>  	  r.intersect (er);