Re: [PATCH] middle-end/94539 - void * aliases every other pointer
On Tue, 14 Apr 2020, Jan Hubicka wrote:
> >
> > But we're using it in indirect_ref_may_alias_decl_p as
> >
> > /* If second reference is view-converted, give up now. */
> > if (same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (ptrtype2)) !=
> > 1)
> > return true;
> >
> > and clearly if TREE_TYPE (ptrtype2) is void * while TREE_TYPE (dbase2)
> > is T * we've "view-converted" things.
> >
> > Not sure what the consequence is when we fall through here since those
> > checks may be redundant now(?) But given we're late for GCC 10 I'd
> > rather be safe than sorry ;)
>
> I think this check is about part of access path missing (i.e. we have
> access path like a.b.c.d.e.f while mem-ref took part of it away
> so we have a.b.c...f).
> So here c and f should not be both pointers.
> >
> > > For ICF checking we need to be more careful anyway (in particular we do
> > > not want all those -1s on arrays), so I would worry about that next
> > > stage1.
> > >
> > > With 1 we will probably save some work since the code will not continue
> > > looking for must aliases.
> >
> > I think we have still quite some cleanup to do for the path-based
> > disambiguations.
>
> Yep, one issue at a time
I've now pushed the last version, let's improve things in GCC 11.
Richard.
> Honza
> >
> > Richard.
> >
> > > Honza
> > >
> > > >
> > > > So, like the following.
> > > >
> > > > Richard.
> > > >
> > > >
> > > > middle-end/94539 - void * aliases every other pointer
> > > >
> > > > This makes same_type_for_tbaa_p conservative in the same way
> > > > get_alias_set is about void * which we allow to alias all other
> > > > pointers.
> > > >
> > > > 2020-04-14 Richard Biener
> > > >
> > > > PR middle-end/94539
> > > > * tree-ssa-alias.c (same_type_for_tbaa): Defer to
> > > > alias_sets_conflict_p for pointers.
> > > >
> > > > * gcc.dg/alias-14.c: Make dg-do run.
> > > > ---
> > > > gcc/testsuite/gcc.dg/alias-14.c | 2 +-
> > > > gcc/tree-ssa-alias.c| 11 ++-
> > > > 2 files changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/gcc/testsuite/gcc.dg/alias-14.c
> > > > b/gcc/testsuite/gcc.dg/alias-14.c
> > > > index 1ca1c09d5e3..24f0d1c1168 100644
> > > > --- a/gcc/testsuite/gcc.dg/alias-14.c
> > > > +++ b/gcc/testsuite/gcc.dg/alias-14.c
> > > > @@ -1,4 +1,4 @@
> > > > -/* { dg-do compile } */
> > > > +/* { dg-do run } */
> > > > /* { dg-options "-O2" } */
> > > > #include
> > > > void *a;
> > > > diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> > > > index df9ba0de0d6..ede4f198342 100644
> > > > --- a/gcc/tree-ssa-alias.c
> > > > +++ b/gcc/tree-ssa-alias.c
> > > > @@ -839,7 +839,16 @@ same_type_for_tbaa (tree type1, tree type2)
> > > > would mean that conversions between them are useless, whereas
> > > > they are
> > > > not (e.g. type and subtypes can have different modes). So, in
> > > > the end,
> > > > they are only guaranteed to have the same alias set. */
> > > > - if (get_alias_set (type1) == get_alias_set (type2))
> > > > + alias_set_type set1 = get_alias_set (type1);
> > > > + alias_set_type set2 = get_alias_set (type2);
> > > > + if (set1 == set2)
> > > > +return -1;
> > > > +
> > > > + /* Pointers to void are considered compatible with all other
> > > > pointers,
> > > > + so for two pointers see what the alias set resolution thinks. */
> > > > + if (POINTER_TYPE_P (type1)
> > > > + && POINTER_TYPE_P (type2)
> > > > + && alias_sets_conflict_p (set1, set2))
> > > > return -1;
> > > >
> > > >/* The types are known to be not equal. */
> > > > --
> > > > 2.13.7
> > > >
> > >
> >
> > --
> > Richard Biener
> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> > Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH] middle-end/94539 - void * aliases every other pointer
>
> But we're using it in indirect_ref_may_alias_decl_p as
>
> /* If second reference is view-converted, give up now. */
> if (same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (ptrtype2)) !=
> 1)
> return true;
>
> and clearly if TREE_TYPE (ptrtype2) is void * while TREE_TYPE (dbase2)
> is T * we've "view-converted" things.
>
> Not sure what the consequence is when we fall through here since those
> checks may be redundant now(?) But given we're late for GCC 10 I'd
> rather be safe than sorry ;)
I think this check is about part of access path missing (i.e. we have
access path like a.b.c.d.e.f while mem-ref took part of it away
so we have a.b.c...f).
So here c and f should not be both pointers.
>
> > For ICF checking we need to be more careful anyway (in particular we do
> > not want all those -1s on arrays), so I would worry about that next
> > stage1.
> >
> > With 1 we will probably save some work since the code will not continue
> > looking for must aliases.
>
> I think we have still quite some cleanup to do for the path-based
> disambiguations.
Yep, one issue at a time
Honza
>
> Richard.
>
> > Honza
> >
> > >
> > > So, like the following.
> > >
> > > Richard.
> > >
> > >
> > > middle-end/94539 - void * aliases every other pointer
> > >
> > > This makes same_type_for_tbaa_p conservative in the same way
> > > get_alias_set is about void * which we allow to alias all other
> > > pointers.
> > >
> > > 2020-04-14 Richard Biener
> > >
> > > PR middle-end/94539
> > > * tree-ssa-alias.c (same_type_for_tbaa): Defer to
> > > alias_sets_conflict_p for pointers.
> > >
> > > * gcc.dg/alias-14.c: Make dg-do run.
> > > ---
> > > gcc/testsuite/gcc.dg/alias-14.c | 2 +-
> > > gcc/tree-ssa-alias.c| 11 ++-
> > > 2 files changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/gcc/testsuite/gcc.dg/alias-14.c
> > > b/gcc/testsuite/gcc.dg/alias-14.c
> > > index 1ca1c09d5e3..24f0d1c1168 100644
> > > --- a/gcc/testsuite/gcc.dg/alias-14.c
> > > +++ b/gcc/testsuite/gcc.dg/alias-14.c
> > > @@ -1,4 +1,4 @@
> > > -/* { dg-do compile } */
> > > +/* { dg-do run } */
> > > /* { dg-options "-O2" } */
> > > #include
> > > void *a;
> > > diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> > > index df9ba0de0d6..ede4f198342 100644
> > > --- a/gcc/tree-ssa-alias.c
> > > +++ b/gcc/tree-ssa-alias.c
> > > @@ -839,7 +839,16 @@ same_type_for_tbaa (tree type1, tree type2)
> > > would mean that conversions between them are useless, whereas they
> > > are
> > > not (e.g. type and subtypes can have different modes). So, in the
> > > end,
> > > they are only guaranteed to have the same alias set. */
> > > - if (get_alias_set (type1) == get_alias_set (type2))
> > > + alias_set_type set1 = get_alias_set (type1);
> > > + alias_set_type set2 = get_alias_set (type2);
> > > + if (set1 == set2)
> > > +return -1;
> > > +
> > > + /* Pointers to void are considered compatible with all other pointers,
> > > + so for two pointers see what the alias set resolution thinks. */
> > > + if (POINTER_TYPE_P (type1)
> > > + && POINTER_TYPE_P (type2)
> > > + && alias_sets_conflict_p (set1, set2))
> > > return -1;
> > >
> > >/* The types are known to be not equal. */
> > > --
> > > 2.13.7
> > >
> >
>
> --
> Richard Biener
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH] middle-end/94539 - void * aliases every other pointer
On Tue, 14 Apr 2020, Jan Hubicka wrote:
> >
> > Note that now, looking again, the TYPE_STRUCTURAL_EQUALITY_P in my patch
> > doesn't match that of get_alias_set. Since we're looking at the alias
> > sets of type1 and type2 anyway we could resort to alias_sets_conflict_p
> > for the two POINTER_TYPE_P case and return -1 then. Not sure about
> > returning 1 - they are not the same as in, one cannot replace a
> > *(void *) access with a *(T *) access since the latter behaves differently
> > wrt a *(U *) access not visible to the call.
> >
> > But the semantics of same_type_for_tbaa are not entirely clear
> > (ISTR you suggested to use it for ICF-like compares).
>
> I think it is clear.
>
> 0 means that accesspath ending by one type can not continue by access path
> starting by the other type.
> 1 means that they can continue.
> -1 means that we do not know (or are too lazy to decide).
>
> In this partiuclar case we have the continuation path trivial and we
> only want to check if one continues the other, so 1 should work.
But we're using it in indirect_ref_may_alias_decl_p as
/* If second reference is view-converted, give up now. */
if (same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (ptrtype2)) !=
1)
return true;
and clearly if TREE_TYPE (ptrtype2) is void * while TREE_TYPE (dbase2)
is T * we've "view-converted" things.
Not sure what the consequence is when we fall through here since those
checks may be redundant now(?) But given we're late for GCC 10 I'd
rather be safe than sorry ;)
> For ICF checking we need to be more careful anyway (in particular we do
> not want all those -1s on arrays), so I would worry about that next
> stage1.
>
> With 1 we will probably save some work since the code will not continue
> looking for must aliases.
I think we have still quite some cleanup to do for the path-based
disambiguations.
Richard.
> Honza
>
> >
> > So, like the following.
> >
> > Richard.
> >
> >
> > middle-end/94539 - void * aliases every other pointer
> >
> > This makes same_type_for_tbaa_p conservative in the same way
> > get_alias_set is about void * which we allow to alias all other
> > pointers.
> >
> > 2020-04-14 Richard Biener
> >
> > PR middle-end/94539
> > * tree-ssa-alias.c (same_type_for_tbaa): Defer to
> > alias_sets_conflict_p for pointers.
> >
> > * gcc.dg/alias-14.c: Make dg-do run.
> > ---
> > gcc/testsuite/gcc.dg/alias-14.c | 2 +-
> > gcc/tree-ssa-alias.c| 11 ++-
> > 2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/gcc/testsuite/gcc.dg/alias-14.c
> > b/gcc/testsuite/gcc.dg/alias-14.c
> > index 1ca1c09d5e3..24f0d1c1168 100644
> > --- a/gcc/testsuite/gcc.dg/alias-14.c
> > +++ b/gcc/testsuite/gcc.dg/alias-14.c
> > @@ -1,4 +1,4 @@
> > -/* { dg-do compile } */
> > +/* { dg-do run } */
> > /* { dg-options "-O2" } */
> > #include
> > void *a;
> > diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> > index df9ba0de0d6..ede4f198342 100644
> > --- a/gcc/tree-ssa-alias.c
> > +++ b/gcc/tree-ssa-alias.c
> > @@ -839,7 +839,16 @@ same_type_for_tbaa (tree type1, tree type2)
> > would mean that conversions between them are useless, whereas they are
> > not (e.g. type and subtypes can have different modes). So, in the
> > end,
> > they are only guaranteed to have the same alias set. */
> > - if (get_alias_set (type1) == get_alias_set (type2))
> > + alias_set_type set1 = get_alias_set (type1);
> > + alias_set_type set2 = get_alias_set (type2);
> > + if (set1 == set2)
> > +return -1;
> > +
> > + /* Pointers to void are considered compatible with all other pointers,
> > + so for two pointers see what the alias set resolution thinks. */
> > + if (POINTER_TYPE_P (type1)
> > + && POINTER_TYPE_P (type2)
> > + && alias_sets_conflict_p (set1, set2))
> > return -1;
> >
> >/* The types are known to be not equal. */
> > --
> > 2.13.7
> >
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH] middle-end/94539 - void * aliases every other pointer
>
> Note that now, looking again, the TYPE_STRUCTURAL_EQUALITY_P in my patch
> doesn't match that of get_alias_set. Since we're looking at the alias
> sets of type1 and type2 anyway we could resort to alias_sets_conflict_p
> for the two POINTER_TYPE_P case and return -1 then. Not sure about
> returning 1 - they are not the same as in, one cannot replace a
> *(void *) access with a *(T *) access since the latter behaves differently
> wrt a *(U *) access not visible to the call.
>
> But the semantics of same_type_for_tbaa are not entirely clear
> (ISTR you suggested to use it for ICF-like compares).
I think it is clear.
0 means that accesspath ending by one type can not continue by access path
starting by the other type.
1 means that they can continue.
-1 means that we do not know (or are too lazy to decide).
In this partiuclar case we have the continuation path trivial and we
only want to check if one continues the other, so 1 should work.
For ICF checking we need to be more careful anyway (in particular we do
not want all those -1s on arrays), so I would worry about that next
stage1.
With 1 we will probably save some work since the code will not continue
looking for must aliases.
Honza
>
> So, like the following.
>
> Richard.
>
>
> middle-end/94539 - void * aliases every other pointer
>
> This makes same_type_for_tbaa_p conservative in the same way
> get_alias_set is about void * which we allow to alias all other
> pointers.
>
> 2020-04-14 Richard Biener
>
> PR middle-end/94539
> * tree-ssa-alias.c (same_type_for_tbaa): Defer to
> alias_sets_conflict_p for pointers.
>
> * gcc.dg/alias-14.c: Make dg-do run.
> ---
> gcc/testsuite/gcc.dg/alias-14.c | 2 +-
> gcc/tree-ssa-alias.c| 11 ++-
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/alias-14.c b/gcc/testsuite/gcc.dg/alias-14.c
> index 1ca1c09d5e3..24f0d1c1168 100644
> --- a/gcc/testsuite/gcc.dg/alias-14.c
> +++ b/gcc/testsuite/gcc.dg/alias-14.c
> @@ -1,4 +1,4 @@
> -/* { dg-do compile } */
> +/* { dg-do run } */
> /* { dg-options "-O2" } */
> #include
> void *a;
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index df9ba0de0d6..ede4f198342 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -839,7 +839,16 @@ same_type_for_tbaa (tree type1, tree type2)
> would mean that conversions between them are useless, whereas they are
> not (e.g. type and subtypes can have different modes). So, in the end,
> they are only guaranteed to have the same alias set. */
> - if (get_alias_set (type1) == get_alias_set (type2))
> + alias_set_type set1 = get_alias_set (type1);
> + alias_set_type set2 = get_alias_set (type2);
> + if (set1 == set2)
> +return -1;
> +
> + /* Pointers to void are considered compatible with all other pointers,
> + so for two pointers see what the alias set resolution thinks. */
> + if (POINTER_TYPE_P (type1)
> + && POINTER_TYPE_P (type2)
> + && alias_sets_conflict_p (set1, set2))
> return -1;
>
>/* The types are known to be not equal. */
> --
> 2.13.7
>
Re: [PATCH] middle-end/94539 - void * aliases every other pointer
On Tue, 14 Apr 2020, Jan Hubicka wrote:
> > On Tue, 14 Apr 2020, Richard Sandiford wrote:
> >
> > > Richard Biener writes:
> > > > This makes same_type_for_tbaa_p conservative in the same way
> > > > get_alias_set is about void * which we allow to alias all other
> > > > pointers.
> > > >
> > > > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> > > >
> > > > Honza, is this what you had in mind?
> > > >
> > > > Thanks,
> > > > Richard.
> > > >
> > > > 2020-04-14 Richard Biener
> > > >
> > > > PR middle-end/94539
> > > > * tree-ssa-alias.c (same_type_for_tbaa): Handle void *
> > > > pointers the same as get_alias_set, returning -1.
> > > >
> > > > * gcc.dg/alias-14.c: Make dg-do run.
> > > > ---
> > > > gcc/testsuite/gcc.dg/alias-14.c | 2 +-
> > > > gcc/tree-ssa-alias.c| 9 +
> > > > 2 files changed, 10 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/gcc/testsuite/gcc.dg/alias-14.c
> > > > b/gcc/testsuite/gcc.dg/alias-14.c
> > > > index 1ca1c09d5e3..24f0d1c1168 100644
> > > > --- a/gcc/testsuite/gcc.dg/alias-14.c
> > > > +++ b/gcc/testsuite/gcc.dg/alias-14.c
> > > > @@ -1,4 +1,4 @@
> > > > -/* { dg-do compile } */
> > > > +/* { dg-do run } */
> > > > /* { dg-options "-O2" } */
> > > > #include
> > > > void *a;
> > > > diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> > > > index df9ba0de0d6..2850141303e 100644
> > > > --- a/gcc/tree-ssa-alias.c
> > > > +++ b/gcc/tree-ssa-alias.c
> > > > @@ -831,6 +831,15 @@ same_type_for_tbaa (tree type1, tree type2)
> > > >&& TREE_CODE (type2) == ARRAY_TYPE)
> > > > return -1;
> > > >
> > > > + /* void * is compatible with all other pointers. */
> > > > + if (POINTER_TYPE_P (type1)
> > > > + && POINTER_TYPE_P (type2)
> > > > + && (TREE_CODE (TREE_TYPE (type1)) == VOID_TYPE
> > > > + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (type1))
> > > > + || TREE_CODE (TREE_TYPE (type2)) == VOID_TYPE
> > > > + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (type2
> > > > +return -1;
> > > > +
> > >
> > > Could you add a comment about the TYPE_STRUCTURAL_EQUALITY_P checks?
> > > Was surprised by them at first, since in aarch64 we use the flag to keep
> > > a distinct ABI identity between ACLE vector types and other vector types.
> > > I'm not sure we expected that to affect the aliasing rules between the
> > > vector types and (say) arbitrary structures.
> >
> > I copied that from the get_alias_set handling:
> >
> > /* Make void * compatible with char * and also void **.
> > Programs are commonly violating TBAA by this.
> >
> > We also make void * to conflict with every pointer
> > (see record_component_aliases) and thus it is safe it to use it
> > for
> > pointers to types with TYPE_STRUCTURAL_EQUALITY_P. */
> > if (TREE_CODE (p) == VOID_TYPE || TYPE_STRUCTURAL_EQUALITY_P (p))
> > set = get_alias_set (ptr_type_node);
> >
> > and the reason is that while we keep struct S * and struct R * have
> > different alias sets the case where 'S' requires structural equality
> > means we have to use an alias-set that conflicts with any other
> > pointer. I'll add a pointer to get_alias_set in the comment.
> >
> > Note that structures with TYPE_STRUCTURAL_EQUALITY_P use alias-set zero,
> > thus have TBAA disabled. Note for vector types this only matters as
> > of the pointer case (we're maybe missing to look one level deeper
> > here), vector types themselves use the alias-set of the component type.
>
> Thanks. I had same patch but failed to send it earlier. I believe it is
> safe to return 1 since the types really are considered same by TBAA.
Note that now, looking again, the TYPE_STRUCTURAL_EQUALITY_P in my patch
doesn't match that of get_alias_set. Since we're looking at the alias
sets of type1 and type2 anyway we could resort to alias_sets_conflict_p
for the two POINTER_TYPE_P case and return -1 then. Not sure about
returning 1 - they are not the same as in, one cannot replace a
*(void *) access with a *(T *) access since the latter behaves differently
wrt a *(U *) access not visible to the call.
But the semantics of same_type_for_tbaa are not entirely clear
(ISTR you suggested to use it for ICF-like compares).
So, like the following.
Richard.
middle-end/94539 - void * aliases every other pointer
This makes same_type_for_tbaa_p conservative in the same way
get_alias_set is about void * which we allow to alias all other
pointers.
2020-04-14 Richard Biener
PR middle-end/94539
* tree-ssa-alias.c (same_type_for_tbaa): Defer to
alias_sets_conflict_p for pointers.
* gcc.dg/alias-14.c: Make dg-do run.
---
gcc/testsuite/gcc.dg/alias-14.c | 2 +-
gcc/tree-ssa-alias.c| 11 ++-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/alias-14.c b/gcc/testsuite/gcc.dg/alias-14.c
index 1ca1c09d5e3..24f0
Re: [PATCH] middle-end/94539 - void * aliases every other pointer
> On Tue, 14 Apr 2020, Richard Sandiford wrote:
>
> > Richard Biener writes:
> > > This makes same_type_for_tbaa_p conservative in the same way
> > > get_alias_set is about void * which we allow to alias all other
> > > pointers.
> > >
> > > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> > >
> > > Honza, is this what you had in mind?
> > >
> > > Thanks,
> > > Richard.
> > >
> > > 2020-04-14 Richard Biener
> > >
> > > PR middle-end/94539
> > > * tree-ssa-alias.c (same_type_for_tbaa): Handle void *
> > > pointers the same as get_alias_set, returning -1.
> > >
> > > * gcc.dg/alias-14.c: Make dg-do run.
> > > ---
> > > gcc/testsuite/gcc.dg/alias-14.c | 2 +-
> > > gcc/tree-ssa-alias.c| 9 +
> > > 2 files changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/gcc/testsuite/gcc.dg/alias-14.c
> > > b/gcc/testsuite/gcc.dg/alias-14.c
> > > index 1ca1c09d5e3..24f0d1c1168 100644
> > > --- a/gcc/testsuite/gcc.dg/alias-14.c
> > > +++ b/gcc/testsuite/gcc.dg/alias-14.c
> > > @@ -1,4 +1,4 @@
> > > -/* { dg-do compile } */
> > > +/* { dg-do run } */
> > > /* { dg-options "-O2" } */
> > > #include
> > > void *a;
> > > diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> > > index df9ba0de0d6..2850141303e 100644
> > > --- a/gcc/tree-ssa-alias.c
> > > +++ b/gcc/tree-ssa-alias.c
> > > @@ -831,6 +831,15 @@ same_type_for_tbaa (tree type1, tree type2)
> > >&& TREE_CODE (type2) == ARRAY_TYPE)
> > > return -1;
> > >
> > > + /* void * is compatible with all other pointers. */
> > > + if (POINTER_TYPE_P (type1)
> > > + && POINTER_TYPE_P (type2)
> > > + && (TREE_CODE (TREE_TYPE (type1)) == VOID_TYPE
> > > + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (type1))
> > > + || TREE_CODE (TREE_TYPE (type2)) == VOID_TYPE
> > > + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (type2
> > > +return -1;
> > > +
> >
> > Could you add a comment about the TYPE_STRUCTURAL_EQUALITY_P checks?
> > Was surprised by them at first, since in aarch64 we use the flag to keep
> > a distinct ABI identity between ACLE vector types and other vector types.
> > I'm not sure we expected that to affect the aliasing rules between the
> > vector types and (say) arbitrary structures.
>
> I copied that from the get_alias_set handling:
>
> /* Make void * compatible with char * and also void **.
> Programs are commonly violating TBAA by this.
>
> We also make void * to conflict with every pointer
> (see record_component_aliases) and thus it is safe it to use it
> for
> pointers to types with TYPE_STRUCTURAL_EQUALITY_P. */
> if (TREE_CODE (p) == VOID_TYPE || TYPE_STRUCTURAL_EQUALITY_P (p))
> set = get_alias_set (ptr_type_node);
>
> and the reason is that while we keep struct S * and struct R * have
> different alias sets the case where 'S' requires structural equality
> means we have to use an alias-set that conflicts with any other
> pointer. I'll add a pointer to get_alias_set in the comment.
>
> Note that structures with TYPE_STRUCTURAL_EQUALITY_P use alias-set zero,
> thus have TBAA disabled. Note for vector types this only matters as
> of the pointer case (we're maybe missing to look one level deeper
> here), vector types themselves use the alias-set of the component type.
Thanks. I had same patch but failed to send it earlier. I believe it is
safe to return 1 since the types really are considered same by TBAA.
Honza
>
> Richard.
Re: [PATCH] middle-end/94539 - void * aliases every other pointer
On Tue, 14 Apr 2020, Richard Sandiford wrote:
> Richard Biener writes:
> > This makes same_type_for_tbaa_p conservative in the same way
> > get_alias_set is about void * which we allow to alias all other
> > pointers.
> >
> > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> >
> > Honza, is this what you had in mind?
> >
> > Thanks,
> > Richard.
> >
> > 2020-04-14 Richard Biener
> >
> > PR middle-end/94539
> > * tree-ssa-alias.c (same_type_for_tbaa): Handle void *
> > pointers the same as get_alias_set, returning -1.
> >
> > * gcc.dg/alias-14.c: Make dg-do run.
> > ---
> > gcc/testsuite/gcc.dg/alias-14.c | 2 +-
> > gcc/tree-ssa-alias.c| 9 +
> > 2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/testsuite/gcc.dg/alias-14.c
> > b/gcc/testsuite/gcc.dg/alias-14.c
> > index 1ca1c09d5e3..24f0d1c1168 100644
> > --- a/gcc/testsuite/gcc.dg/alias-14.c
> > +++ b/gcc/testsuite/gcc.dg/alias-14.c
> > @@ -1,4 +1,4 @@
> > -/* { dg-do compile } */
> > +/* { dg-do run } */
> > /* { dg-options "-O2" } */
> > #include
> > void *a;
> > diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> > index df9ba0de0d6..2850141303e 100644
> > --- a/gcc/tree-ssa-alias.c
> > +++ b/gcc/tree-ssa-alias.c
> > @@ -831,6 +831,15 @@ same_type_for_tbaa (tree type1, tree type2)
> >&& TREE_CODE (type2) == ARRAY_TYPE)
> > return -1;
> >
> > + /* void * is compatible with all other pointers. */
> > + if (POINTER_TYPE_P (type1)
> > + && POINTER_TYPE_P (type2)
> > + && (TREE_CODE (TREE_TYPE (type1)) == VOID_TYPE
> > + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (type1))
> > + || TREE_CODE (TREE_TYPE (type2)) == VOID_TYPE
> > + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (type2
> > +return -1;
> > +
>
> Could you add a comment about the TYPE_STRUCTURAL_EQUALITY_P checks?
> Was surprised by them at first, since in aarch64 we use the flag to keep
> a distinct ABI identity between ACLE vector types and other vector types.
> I'm not sure we expected that to affect the aliasing rules between the
> vector types and (say) arbitrary structures.
I copied that from the get_alias_set handling:
/* Make void * compatible with char * and also void **.
Programs are commonly violating TBAA by this.
We also make void * to conflict with every pointer
(see record_component_aliases) and thus it is safe it to use it
for
pointers to types with TYPE_STRUCTURAL_EQUALITY_P. */
if (TREE_CODE (p) == VOID_TYPE || TYPE_STRUCTURAL_EQUALITY_P (p))
set = get_alias_set (ptr_type_node);
and the reason is that while we keep struct S * and struct R * have
different alias sets the case where 'S' requires structural equality
means we have to use an alias-set that conflicts with any other
pointer. I'll add a pointer to get_alias_set in the comment.
Note that structures with TYPE_STRUCTURAL_EQUALITY_P use alias-set zero,
thus have TBAA disabled. Note for vector types this only matters as
of the pointer case (we're maybe missing to look one level deeper
here), vector types themselves use the alias-set of the component type.
Richard.
Re: [PATCH] middle-end/94539 - void * aliases every other pointer
Richard Biener writes:
> This makes same_type_for_tbaa_p conservative in the same way
> get_alias_set is about void * which we allow to alias all other
> pointers.
>
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>
> Honza, is this what you had in mind?
>
> Thanks,
> Richard.
>
> 2020-04-14 Richard Biener
>
> PR middle-end/94539
> * tree-ssa-alias.c (same_type_for_tbaa): Handle void *
> pointers the same as get_alias_set, returning -1.
>
> * gcc.dg/alias-14.c: Make dg-do run.
> ---
> gcc/testsuite/gcc.dg/alias-14.c | 2 +-
> gcc/tree-ssa-alias.c| 9 +
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/gcc.dg/alias-14.c b/gcc/testsuite/gcc.dg/alias-14.c
> index 1ca1c09d5e3..24f0d1c1168 100644
> --- a/gcc/testsuite/gcc.dg/alias-14.c
> +++ b/gcc/testsuite/gcc.dg/alias-14.c
> @@ -1,4 +1,4 @@
> -/* { dg-do compile } */
> +/* { dg-do run } */
> /* { dg-options "-O2" } */
> #include
> void *a;
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index df9ba0de0d6..2850141303e 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -831,6 +831,15 @@ same_type_for_tbaa (tree type1, tree type2)
>&& TREE_CODE (type2) == ARRAY_TYPE)
> return -1;
>
> + /* void * is compatible with all other pointers. */
> + if (POINTER_TYPE_P (type1)
> + && POINTER_TYPE_P (type2)
> + && (TREE_CODE (TREE_TYPE (type1)) == VOID_TYPE
> + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (type1))
> + || TREE_CODE (TREE_TYPE (type2)) == VOID_TYPE
> + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (type2
> +return -1;
> +
Could you add a comment about the TYPE_STRUCTURAL_EQUALITY_P checks?
Was surprised by them at first, since in aarch64 we use the flag to keep
a distinct ABI identity between ACLE vector types and other vector types.
I'm not sure we expected that to affect the aliasing rules between the
vector types and (say) arbitrary structures.
Thanks,
Richard
