This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH GCC8][17/33]Treat complex cand step as invriant expression
- From: "Richard Biener via gcc-patches" <gcc-patches at gcc dot gnu dot org>
- To: "Bin.Cheng" <amker dot cheng at gmail dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 8 May 2017 13:53:42 +0200
- Subject: Re: [PATCH GCC8][17/33]Treat complex cand step as invriant expression
- Authentication-results: sourceware.org; auth=none
- References: <VI1PR0802MB2176BEAC53A37DCA039C0137E7190@VI1PR0802MB2176.eurprd08.prod.outlook.com> <CAFiYyc0OKU9bxyYKEYjGvVfcv-SwYzEF20n34KmgrwWaQfe9yQ@mail.gmail.com> <CAHFci29i96CWFR-4QdSMYz_w2uFoL3zKhvJEh61cT=fvO_SW4g@mail.gmail.com>
- Reply-to: Richard Biener <richard dot guenther at gmail dot com>
On Thu, May 4, 2017 at 7:55 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> 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 for the explanation.
Ok.
Thanks,
Richard.
> 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.