Re: [PATCH GCC8][17/33]Treat complex cand step as invriant expression

["Bin.Cheng" <amker dot cheng at gmail dot com>]

On Wed, May 3, 2017 at 2:43 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Apr 18, 2017 at 12:46 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>> Hi,
>> We generally need to compute cand step in loop preheader and use it in loop body.
>> Unless it's an SSA_NAME of constant integer, an invariant expression is needed.
>
> I'm confused as to what this patch does.  Comments talk about "Handle step as"
> but then we print "Depend on inv...".  And we share bitmaps, well it seems
>
> +         find_inv_vars (data, &step, &cand->inv_vars);
> +
> +         iv_inv_expr_ent *inv_expr = get_loop_invariant_expr (data, step);
> +         /* Share bitmap between inv_vars and inv_exprs for cand.  */
> +         if (inv_expr != NULL)
> +           {
> +             cand->inv_exprs = cand->inv_vars;
> +             cand->inv_vars = NULL;
> +             if (cand->inv_exprs)
> +               bitmap_clear (cand->inv_exprs);
> +             else
> +               cand->inv_exprs = BITMAP_ALLOC (NULL);
> +
> +             bitmap_set_bit (cand->inv_exprs, inv_expr->id);
>
> just shares the bitmap allocation (and leaks cand->inv_exprs?).
>
> Note that generally it might be cheaper to use bitmap_head instead of
> bitmap in the various structures (and then bitmap_initialize ()), this
> saves one indirection.
>
> Anyway, the relation between inv_vars and inv_exprs is what confuses me.
> Maybe it's the same as for cost_pair?  invariants vs. loop invariants?
> whatever that means...
>
> That is, can you expand the comments in cost_pair / iv_cand for inv_vars
> vs. inv_exprs, esp what "invariant" actually means?
When we represent use with cand, there will be computation which is
loop invariant.  The invariant computation is an newly created
invariant expression and is based on ssa_vars existed before ivopts.
If the invariant expression is complicated, we handle and call it as
invariant expression.  We say the cost_pair depends on the inv.exprs.
If the invariant expression is simple enough, we record all existing
ssa_vars it based on in inv_vars.  We say the cost_pair depends on the
inv.vars.   The same words stand for struct iv_cand.  If cand.step is
simple enough, we simply record the ssa_var it based on in inv_vars,
otherwise, the step is a new invariant expression which doesn't exist
before, we record it in cand.inv_exprs.

Add comment for inv_vars/inv_exprs, is this OK?   I noticed there is a
redundant field cost_pair.inv_expr, I deleted it as obvious in a
standalone patch.

Thanks,
bin
>
> Thanks,
> Richard.
>
>> Thanks,
>> bin
>>
>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>
>>         * tree-ssa-loop-ivopts.c (struct iv_cand): New field inv_exprs.
>>         (dump_cand): Support iv_cand.inv_exprs.
>>         (add_candidate_1): Record invariant exprs in iv_cand.inv_exprs
>>         for candidates.
>>         (iv_ca_set_no_cp, iv_ca_set_cp, free_loop_data): Support
>>         iv_cand.inv_exprs.

Attachment: 0017-treat-cand_step-as-inv_expr-20170501.txt
Description: Text document