On Mon, Jun 20, 2016 at 09:11:21PM +0100, Richard Sandiford wrote:
> tbsaunde+...@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

Reply via email to