On Thu, May 4, 2017 at 7:55 PM, Bin.Cheng <amker.ch...@gmail.com> wrote:
> On Wed, May 3, 2017 at 2:43 PM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> On Tue, Apr 18, 2017 at 12:46 PM, Bin Cheng <bin.ch...@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 for the explanation.
Ok.
Thanks,
Richard.
> Thanks,
> bin
>>
>> Thanks,
>> Richard.
>>
>>> Thanks,
>>> bin
>>>
>>> 2017-04-11 Bin Cheng <bin.ch...@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.
