Re: [PATCH] Use rtx_refs_may_alias_p instead of alias_sets_conflict_p in

[Xinliang David Li <davidxl at google dot com>]

A general comment.

A set of unified high level aliasing/memory disambiguation/data
dependence query interfaces can be handy for many different alias
information clients such as Scalar optimizer, Loop Transformations,
AutoPar,  Scheduler, ModSched etc.

1) Instead of returning a bool, it is useful to return aliasing kind:
no_alias, may_alias, must_alias, etc.
2) The aliasing query result is in many cases ambiguous without a
context, e.g, an intra-iteration no alias could become inter-iteration
may-alias. To solve this, the interface need to provide a way for the
client code to specify what it wants -- i.e. a bool flag indicating
whether it cares about cross iteration/carried dependence or not -- or
better yet, passing a loop context -- if the loop context is null, it
only cares about intra iteration aliasing. The implementation will
need to check the context and decide the most precise answer depending
on the source of the alias information. For instance, points-to info
is 'context insensitive', offset based tbaa is not either in cases;
restrict qualified locals can be tricky --- as depending on the loop
in the loop nest that is queried, it may return different answer.
3) Client code (e.g. ModSched) may also care about dependence
direction/distance -- the information can be returned if known

When it comes to cross iteration aliasing/dependence information (and
distance), the information needs to be updated properly with loop
unrolling etc.

Other useful things to have:

1. debug trace on alias queries -- indicating what alias result is
returned and what the source is (points to, tbaa, ...)
2. dbgcnt support for alias query for triaging alias problems.

Thanks,

David

On Wed, Aug 4, 2010 at 8:15 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Aug 4, 2010 at 5:06 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
>>
>>
>>> -----Original Message-----
>>> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>>> Sent: 04 August 2010 16:02
>>> To: Diego Novillo
>>> Cc: Bingfeng Mei; gcc-patches@gcc.gnu.org
>>> Subject: Re: [PATCH] Use rtx_refs_may_alias_p instead of
>>> alias_sets_conflict_p in
>>>
>>> On Wed, Aug 4, 2010 at 4:58 PM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>> > On Wed, Aug 4, 2010 at 4:57 PM, Richard Guenther
>>> > <richard.guenther@gmail.com> wrote:
>>> >> On Wed, Aug 4, 2010 at 4:56 PM, Diego Novillo <dnovillo@google.com>
>>> wrote:
>>> >>> On Wed, Aug 4, 2010 at 14:52, Bingfeng Mei <bmei@broadcom.com>
>>> wrote:
>>> >>>
>>> >>>> 2010-08-04 ?Bingfeng Mei ?<bmei@broadcom.com>
>>> >>>>
>>> >>>> ? ? ? ?* alias.c (walk_mems_2): Call rtx_refs_may_alias_p
>>> >>>> ? ? ? ?instead of alias_sets_conflict_p to get more accurate
>>> >>>> ? ? ? ?alias set information.
>>> >>>
>>> >>> OK.
>>> >>
>>> >> Wait. ?You need to use a proper dependence function instead,
>>> >> rtx_refs_may_alias_p isn't supposed to be used directly.
>>> >
>>> > Which would be true_dependence if using TBAA is valid here
>>> > (which I am not sure).
>>>
>>> After this patch insn_alias_sets_conflict_p is also seriously
>>> misnamed. ?As it is used only from within ddg.c it should
>>> be moved there and made private.
>>
>> So I need to make rtx_refs_may_alias_p global?
>
> No. ?You need to adjust add_inter_loop_mem_dep to, for the
> cases of {read,write} vs. {read,write} use the proper dependence queires
>
> Like
>
> /* Given two nodes, analyze their RTL insns and add inter-loop mem deps
> ? to ddg G. ?*/
> static void
> add_inter_loop_mem_dep (ddg_ptr g, ddg_node_ptr from, ddg_node_ptr to)
> {
> - ?if (!insn_alias_sets_conflict_p (from->insn, to->insn))
> - ? ?/* Do not create edge if memory references have disjoint alias sets. ?*/
> - ? ?return;
>
> ?if (mem_write_insn_p (from->insn))
> ? ?{
> ? ? ?if (mem_read_insn_p (to->insn))
> + {
> + ? if (XXX_dependence (the-mems-in from->insn, the-mems-in to->insn))
> ? ? ? ?create_ddg_dep_no_link (g, from, to,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DEBUG_INSN_P (to->insn)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?? ANTI_DEP : TRUE_DEP, MEM_DEP, 1);
> ... with XXX being output or anti or true.
>
> etc.
>
> Richard.
>
>>>
>>> /* Given two nodes, analyze their RTL insns and add inter-loop mem deps
>>> ? ?to ddg G. ?*/
>>> static void
>>> add_inter_loop_mem_dep (ddg_ptr g, ddg_node_ptr from, ddg_node_ptr to)
>>> {
>>> ? if (!insn_alias_sets_conflict_p (from->insn, to->insn))
>>> ? ? /* Do not create edge if memory references have disjoint alias sets.
>>> */
>>> ? ? return;
>>>
>>> the comment ("inter-loop mem deps") suggests that using TBAA
>>> is not valid here.
>> Does TBAA mean type-based alias analysis? Why is it not valid here?
>>
>>>
>>> Richard.
>>>
>>> > Richard.
>>> >
>>> >> Richard.
>>> >>
>>> >>>
>>> >>> Diego.
>>> >>>
>>> >>
>>> >
>>
>>
>>
>