Re: [PATCH 6/6] loop-iv.c: make cond_list a vec

[Trevor Saunders <tbsaunde at tbsaunde dot org>]

On Mon, Jun 20, 2016 at 09:11:21PM +0100, Richard Sandiford wrote:
> tbsaunde+gcc@tbsaunde.org writes:
> > diff --git a/gcc/loop-iv.c b/gcc/loop-iv.c
> > index 57fb8c1..21c3180 100644
> > --- a/gcc/loop-iv.c
> > +++ b/gcc/loop-iv.c
> > @@ -1860,7 +1860,6 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
> >  {
> >    bool expression_valid;
> >    rtx head, tail, last_valid_expr;
> > -  rtx_expr_list *cond_list;
> >    rtx_insn *insn;
> >    rtx neutral, aggr;
> >    regset altered, this_altered;
> > @@ -1936,7 +1935,7 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
> >  
> >    expression_valid = true;
> >    last_valid_expr = *expr;
> > -  cond_list = NULL;
> > +  auto_vec<rtx> cond_list;
> >    while (1)
> >      {
> >        insn = BB_END (e->src);
> 
> How about using "auto_vec<rtx, N>" for some small N, since we expect
> cond_list to be used fairly often?

sure, why not?

> > @@ -1988,39 +1988,30 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
> >  
> >  	  if (suitable_set_for_replacement (insn, &dest, &src))
> >  	    {
> > -	      rtx_expr_list **pnote, **pnote_next;
> > -
> >  	      replace_in_expr (expr, dest, src);
> >  	      if (CONSTANT_P (*expr))
> >  		goto out;
> >  
> > -	      for (pnote = &cond_list; *pnote; pnote = pnote_next)
> > +	      unsigned int len = cond_list.length ();
> > +	      for (unsigned int i = len - 1; i < len; i--)
> >  		{
> > -		  rtx_expr_list *note = *pnote;
> > -		  rtx old_cond = XEXP (note, 0);
> > +		  rtx old_cond = cond_list[i];
> >  
> > -		  pnote_next = (rtx_expr_list **)&XEXP (note, 1);
> > -		  replace_in_expr (&XEXP (note, 0), dest, src);
> > +		  replace_in_expr (&cond_list[i], dest, src);
> >  
> >  		  /* We can no longer use a condition that has been simplified
> >  		     to a constant, and simplify_using_condition will abort if
> >  		     we try.  */
> > -		  if (CONSTANT_P (XEXP (note, 0)))
> > -		    {
> > -		      *pnote = *pnote_next;
> > -		      pnote_next = pnote;
> > -		      free_EXPR_LIST_node (note);
> > -		    }
> > +		  if (CONSTANT_P (cond_list[i]))
> > +		    cond_list.ordered_remove (i);
> 
> Do we really need ordered removes here and below?  Obviously it turns
> the original O(1) operation into O(n), and it wasn't obvious from first
> glance that the order of the conditions was relevant.

I'm not sure, but I certainly don't know that we don't need them.  I
kind of ment to not send this patch because of that question but then
forgot.  I'm not really sure what to do with these, I don't know that I
know what's going on well enough to prove unordered removes are fine,
but I guess I can try.

Trev

> 
> Thanks,
> Richard