Re: [RFC] Request for comments on ivopts patch

[Richard Biener <richard.guenther@gmail.com>]

On Sat, Dec 12, 2015 at 12:48 AM, Steve Ellcey <sellcey@imgtec.com> wrote:
> On Wed, 2015-12-09 at 11:24 +0100, Richard Biener wrote:
>
>> > This second case (without the preference for the original IV)
>> > generates better code on MIPS because the final assembly
>> > has the increment instructions between the loads and the tests
>> > of the values being loaded and so there is no delay (or less delay)
>> > between the load and use.  It seems like this could easily be
>> > the case for other platforms too so I was wondering what people
>> > thought of this patch:
>>
>> You don't comment on the comment you remove ... debugging
>> programs is also important!
>>
>> So if then the cost of both cases should be distinguished
>> somewhere else, like granting a bonus for increment before
>> exit test or so.
>>
>> Richard.
>
> Here is new patch that tries to do that.  It accomplishes the same thing
> as my original patch but by checking different features.  Basically, for
> machines with no autoinc/autodec it has a preference for IVs that don't
> change during loop (i.e. var_before == var_after).
>
> What do you think about this approach?
>
> Steve Ellcey
> sellcey@imgtec.com
>
>
> 2015-12-11  Steve Ellcey  <sellcey@imgtec.com>
>
>         * tree-ssa-loop-ivopts.c (determine_iv_cost): Add cost to ivs that
>         need to be updated during loop.
>
>
> diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
> index 98dc451..ecf9737 100644
> --- a/gcc/tree-ssa-loop-ivopts.c
> +++ b/gcc/tree-ssa-loop-ivopts.c
> @@ -5826,6 +5826,14 @@ determine_iv_cost (struct ivopts_data *data, struct iv_cand *cand)
>        || DECL_ARTIFICIAL (SSA_NAME_VAR (cand->var_before)))
>      cost++;
>
> +  /* If we are not using autoincrement or autodecrement, prefer ivs that
> +     do not have to be incremented/decremented during the loop.  This can
> +     move loads ahead of the instructions that update the address.  */
> +  if (cand->pos != IP_BEFORE_USE
> +      && cand->pos != IP_AFTER_USE
> +      && cand->var_before != cand->var_after)
> +    cost++;
> +

I don't know enough to assess the effect of this but

 1) not all archs can do auto-incdec so either the comment is misleading
or the test should probably be amended
 2) I wonder why with the comment ("during the loop") you exclude IP_NORMAL/END

that said, the comment needs to explain the situation better.

Of course all such patches need some code-gen effect investigation
on more than one arch.

[I wonder if a IV cost adjust target hook makes sense at some point]

Richard.

>    /* Prefer not to insert statements into latch unless there are some
>       already (so that we do not create unnecessary jumps).  */
>    if (cand->pos == IP_END
>
>