On Tue, 14 Apr 2020, Jan Hubicka wrote:
> > On Tue, 14 Apr 2020, Richard Sandiford wrote:
> >
> > > Richard Biener <rguent...@suse.de> 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 <rguent...@suse.de>
> > > >
> > > > 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 <stddef.h>
> > > > 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 <rguent...@suse.de>
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 <stddef.h>
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
