This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Use rtx_refs_may_alias_p instead of alias_sets_conflict_p in
On Mon, Aug 9, 2010 at 2:41 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> Hi,
> I created a new may_alias_p function as Richard proposed. The function
> disables offset-based memory disambiguation and TBAA, it is also slightly
> more conservative than true/output/anti_dependence. For example,
> write_dependence checks:
> ?if (GET_MODE (x) == BLKmode && GET_CODE (XEXP (x, 0)) == SCRATCH)
> ? ?return 1;
> ?if (GET_MODE (mem) == BLKmode && GET_CODE (XEXP (mem, 0)) == SCRATCH)
> ? ?return 1;
>
> true_depdence checks both above and following (isn't above
> statement redundant then?).
> ?if (mem_mode == BLKmode || GET_MODE (x) == BLKmode)
> ? ?return 1;
> may_alias_p takes more conservative check of the latter one only.
>
> Since insn_alias_sets_conflict_p is misnamed and only used once by ddg.c,
> it & friends (walk_mems_1, walk_mems_2) are moved into ddg.c and
> renamed as insns_may_alias_p.
>
> The patch is bootstrapped on x86_64-unknown-linux-gnu and tested.
> OK for trunk?
@@ -2190,7 +2153,7 @@ adjust_offset_for_component_ref (tree x,
X and Y and they do not overlap. */
int
-nonoverlapping_memrefs_p (const_rtx x, const_rtx y)
+nonoverlapping_memrefs_p (const_rtx x, const_rtx y, bool loop_invariant)
the new parameter needs documenting.
+
+/* Check whether x may be aliased with mem. Don't do offset-based
+ memory disambiguation & TBAA*/
+int
+may_alias_p (const_rtx mem, const_rtx x)
two spaces after '.', a full-stop and two spaces at the end of the comment.
'x' and 'mem' should be all-caps.
Otherwise the patch looks fine to me. Thus, the patch is ok
with the above changes.
Thanks,
Richard.
> Thanks,
> Bingfeng
>
>
> 2010-08-10 ?Bingfeng Mei ?<bmei@broadcom.com>
>
> ? ? ? ?* ddg.c (walk_mems_2): Moved from alias.c, use may_alias_p instead of
> ? ? ? ?alias_sets_conflict_p.
> ? ? ? ?(walk_mems_1): Moved from alias.c.
> ? ? ? ?(insns_may_alias_p): New function, originally insn_alias_sets_conflict_p
> ? ? ? ?in alias.c.
> ? ? ? ?(add_inter_loop_mem_dep): Use insns_may_alias_p now.
> ? ? ? ?* cse.c (cse_insn): New argument in calling nonoverlapping_memrefs_p.
> ? ? ? ?* alias.c (walk_mems_2): Moved to ddg.c.
> ? ? ? ?(walk_mems_1): Ditto.
> ? ? ? ?(insn_alias_sets_conflict_p): Renamed to insns_may_alias_p and moved
> ? ? ? ?to ddg.c.
> ? ? ? ?(nonoverlapping_memrefs_p): Add flag to guard offset-based memory
> ? ? ? ?disambiguation.
> ? ? ? ?*(may_alias_p): New function to check whether two memory expression
> ? ? ? ?may alias or not. Currently used in buidling inter-iteration memory
> ? ? ? ?dependence.
> ? ? ? ?*alias.h (nonoverlapping_memrefs_p): New flag as third argument.
> ? ? ? ?(insn_alias_sets_conflict_p): Removed
> ? ? ? ?*rtl.h (may_alias_p): New function prototype.
>
>> -----Original Message-----
>> From: Richard Guenther [mailto:richard.guenther@gmail.com]
>> Sent: 06 August 2010 11:10
>> To: Bingfeng Mei
>> Cc: Michael Matz; Diego Novillo; gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH] Use rtx_refs_may_alias_p instead of
>> alias_sets_conflict_p in
>>
>> On Thu, Aug 5, 2010 at 7:00 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
>> > Hi,
>> > If we are going to use true_dependence function etc in ddg.c,
>> > we should be able to disable loop variant stuff (currently
>> > I can only think of offset-based memory disambiguation and TBAA).
>> > I added a flag loop_invariant to relevant functions in attached
>> > patch. The patch is tested and bootstrapped. Is this OK?
>> >
>> > Next, I need to adapt ddg.c to use these XXX_dependence functions,
>> > move and rename insn_alias_sets_conflict_p & friends to ddg.c.
>> > The original add_inter_loop_mem_dep is over-simplified.
>> > For example, it doesn't consider parallel patterns with more than
>> > one memory expressions.
>>
>> + ?if (!loop_invariant &&
>>
>> &&s go to the next line.
>>
>> ? ?if (DIFFERENT_ALIAS_SETS_P (x, mem))
>> ? ? ?return 0;
>>
>> - ?if (nonoverlapping_memrefs_p (mem, x))
>> + ?if (nonoverlapping_memrefs_p (mem, x, loop_invariant))
>> ? ? ?return 0;
>>
>> The DIFFERENT_ALIAS_SETS_P would also need guarding.
>>
>> I think that instead of changing all present functions it would be
>> better to create a new entry to the alias oracle for inter-iteration
>> queries, as for example you can't distinguish true and anti dependence
>> (which is also why TBAA isn't valid here). ?In fact the implementation
>> would be the same for all of RW, WR and WW dependence types.
>>
>> The query would exactly match what Zdenek thinks of a
>> may-alias query, so I'd suggest to simply name it may_alias_p.
>>
>> Richard.
>>
>> > Thanks
>> > Bingfeng
>> >
>> >> -----Original Message-----
>> >> From: Michael Matz [mailto:matz@suse.de]
>> >> Sent: 04 August 2010 17:15
>> >> To: Bingfeng Mei
>> >> Cc: Richard Guenther; Diego Novillo; gcc-patches@gcc.gnu.org
>> >> Subject: RE: [PATCH] Use rtx_refs_may_alias_p instead of
>> >> alias_sets_conflict_p in
>> >>
>> >> Hi,
>> >>
>> >> On Wed, 4 Aug 2010, Bingfeng Mei wrote:
>> >>
>> >> > Does rtx_refs_may_alias_p include offset based disambiguation
>> >> (with/without
>> >> > tbaa flag).
>> >>
>> >> Actually it doesn't (we switch off recognizing SSA names as being
>> equal
>> >> when called from RTL land). ?I forgot that this was recently changed.
>> >>
>> >> Unfortunately I was wrong with my assertion that TBAA can be used
>> cross
>> >> iteration. ?The catch is the new memory model, with it's asymmetric
>> >> dependencies: you may move a load before a non-conflicting (type
>> >> wise) store, but you may not move a store before a non-conflicting
>> load.
>> >>
>> >> In cross-iteration situations you are posed with both cases, hence
>> you
>> >> can't really use type-based disambiguation (perhaps in some few
>> special
>> >> cases one might, but not generally).
>> >>
>> >> Basically for cross iteration analysis you can only use points-to
>> >> information (or generally everything that is loop invariant for the
>> >> loop
>> >> in question, but that isn't implemented).
>> >>
>> >> > If yes, this patch is not correct, I need to work out a better one.
>> >>
>> >> It would be better to use the four different predicates we have in
>> RTL
>> >> land as Richard suggested, they do the right thing for each
>> dependency
>> >> type (switching off TBAA when appropriate).
>> >>
>> >>
>> >> Ciao,
>> >> Michael.
>> >
>> >
>
>