Commit Message

Hi,
This patch uses rtx_refs_may_alias_p instead of alias_sets_conflict_p to
get more accurate alias set information in walk_mems_2, which is used
in building DDG. In the following example, no false cross-iteration
dependence is drawn between writing to a[i] and reading b[i] in
next iteration after the patch.
Impact for compile-time is minimal because it only applies before modulo
scheduling. Bootstrapped and tested on x86_64-unknown-linux-gnu.
OK for trunk?
void foo(int * restrict a, int * restrict b, int n)
{
int i;
for(i = 0; i < n; i++)
{
a[i] = b[i] * 100;
}
}
Cheers,
Bingfeng Mei
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.

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.
/* 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.
Richard.
> Richard.>>> Richard.>>>>>>>> Diego.>>>>>>

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.

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.
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.

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.>>

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?
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.> >> >

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.>> >>> >>>