Do less generous pointer globbing in alias.c

Do less generous pointer globbing in alias.c

[Jan Hubicka]

Hi,
this patch makes it possible for non-LTO alias oracle to TBAA disambiguate
pointer types. It makes void * conflicting with all of them and does not put it
to alias set 0. It also preserves the property that qualifiers of pointer-to
type should not matter to determine the alias set and that pointer to array is
same as pointer to array element.  Finally it makes pointer void * to be
equivalent to void ** (and more *) and to types with structural equality only.

I think those are all globbing rules we discussed for the non-LTO patch.

It does two things.  First is kind of "canonicalization" where for a given pointer
it looks for non-pointer pointed-to type and then rebuilds is without qualifiers.
This is fast, because build_pointer_type will reuse existing types.

It makes void * to conflict with everyting by making its alias set to be subset
of alias set of any other pointer.  This means that writes to void * conflict
with writes to any other pointer without really need to glob all the pointers
to one equivalence class.

This patch makes quite some difference on C++.  For example on deal II the TBAA
stats reports 4344358 disambiguations and 7008576 queries, while with the patch
we get 5368737 and 5687399 queries (I did not chose deal II for reason, it is
just random C++ file)

The patch bootstrap and regtests ppc64le-linux with the following testsuite
differences:
@@ -30,7 +30,9 @@
 FAIL: c-c++-common/asan/null-deref-1.c   -O3 -g  output pattern test, is ASAN:SIGSEGV
 FAIL: c-c++-common/asan/null-deref-1.c   -Os  output pattern test, is ASAN:SIGSEGV
 FAIL: gcc.dg/cpp/_Pragma3.c (test for excess errors)
+XPASS: gcc.dg/alias-8.c  (test for warnings, line 11)
 FAIL: gcc.dg/loop-8.c scan-rtl-dump-times loop2_invariant "Decided" 1
+FAIL: gcc.dg/pr62167.c scan-tree-dump-not pre "Removing basic block"
 FAIL: gcc.dg/sms-4.c scan-rtl-dump-times sms "SMS succeeded" 1
 XPASS: gcc.dg/guality/example.c   -O0  execution test
 XPASS: gcc.dg/guality/example.c   -O1  execution test
@@ -304,6 +306,9 @@
 FAIL: c-c++-common/asan/null-deref-1.c   -O3 -g  output pattern test, is ASAN:SIGSEGV
 FAIL: g++.dg/cpp1y/vla-initlist1.C  -std=gnu++11 execution test
 FAIL: g++.dg/cpp1y/vla-initlist1.C  -std=gnu++14 execution test
+FAIL: g++.dg/ipa/ipa-icf-4.C  -std=gnu++11  scan-ipa-dump icf "Equal symbols: [67]"
+FAIL: g++.dg/ipa/ipa-icf-4.C  -std=gnu++14  scan-ipa-dump icf "Equal symbols: [67]"
+FAIL: g++.dg/ipa/ipa-icf-4.C  -std=gnu++98  scan-ipa-dump icf "Equal symbols: [67]"

ipa-icf-4 is about alias info now being more perceptive to block the merging.
pr62167 seems just confused.  The template checks that memory stores are not
unified.  It looks for BB removal message, but with the patch we get:
  <bb 2>:
  node.next = 0B;
  head.0_4 = head;
  node.prev = head.0_4;
  head.0_4->first = &node;
  k.1_7 = k;
  h_8 = &heads[k.1_7];
  heads[2].first = 0B;
  if (head.0_4 == h_8)
    goto <bb 3>;
  else
    goto <bb 5>;

  <bb 5>:
  goto <bb 4>;

  <bb 3>:
  p_10 = MEM[(struct head *)&heads][k.1_7].first;

  <bb 4>:
  # p_1 = PHI <p_10(3), &node(5)>
  _11 = p_1 != 0B;
  _12 = (int) _11;
  return _12;

before PR, the message is about the bb 5 sitting at critical edge removed.
The TBAA incompatible load it looks for is optimized away by FRE:
  head->first = &node;

  struct node *n = head->first;

  struct head *h = &heads[k];

  heads[2].first = n->next;

  if ((void*)n->prev == (void *)h)
    p = h->first;
  else
    /* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next.  */
    p = n->prev->next;

here n is known to be head->first that is known to be &node.
The testcase runtime checks that result is Ok and passes.

Bootstrapped/regtested ppc64le-linux.

	* alias.c (get_alias_set): Do not glob all pointer types into one;
	just produce euqivalence classes based on canonical type of pointed
	type type; make void * equivalent to void **.
	(record_component_aliases): Make void * to conflict with all other
	pointer types.
Index: alias.c
===================================================================
--- alias.c	(revision 223633)
+++ alias.c	(working copy)
@@ -903,35 +906,79 @@ get_alias_set (tree t)
      the pointed-to types.  This issue has been reported to the
      C++ committee.
 
-     In addition to the above canonicalization issue, with LTO
-     we should also canonicalize `T (*)[]' to `T *' avoiding
-     alias issues with pointer-to element types and pointer-to
-     array types.
-
-     Likewise we need to deal with the situation of incomplete
-     pointed-to types and make `*(struct X **)&a' and
-     `*(struct X {} **)&a' alias.  Otherwise we will have to
-     guarantee that all pointer-to incomplete type variants
-     will be replaced by pointer-to complete type variants if
-     they are available.
-
-     With LTO the convenient situation of using `void *' to
-     access and store any pointer type will also become
-     more apparent (and `void *' is just another pointer-to
-     incomplete type).  Assigning alias-set zero to `void *'
-     and all pointer-to incomplete types is a not appealing
-     solution.  Assigning an effective alias-set zero only
-     affecting pointers might be - by recording proper subset
-     relationships of all pointer alias-sets.
-
-     Pointer-to function types are another grey area which
-     needs caution.  Globbing them all into one alias-set
-     or the above effective zero set would work.
-
-     For now just assign the same alias-set to all pointers.
-     That's simple and avoids all the above problems.  */
-  else if (POINTER_TYPE_P (t)
-	   && t != ptr_type_node)
+     For this reason go to canonical type of the unqalified pointer type.
+     Until GCC 6 this code set all pointers sets to have alias set of
+     ptr_type_node but that is a bad idea, because it prevents disabiguations
+     in between pointers.  For Firefox this accounts about 20% of all
+     disambiguations in the program.  */
+  else if (POINTER_TYPE_P (t) && t != ptr_type_node && !in_lto_p)
+    {
+      tree p;
+      auto_vec <bool, 8> reference;
+
+      /* Unnest all pointers and references.
+         We also want to make pointer to array equivalent to pointer to its
+         element. So skip all array types, too.  */
+      for (p = t; POINTER_TYPE_P (p)
+	   || (TREE_CODE (p) == ARRAY_TYPE && !TYPE_NONALIASED_COMPONENT (p));
+	   p = TREE_TYPE (p))
+	{
+	  if (TREE_CODE (p) == REFERENCE_TYPE)
+	    reference.safe_push (true);
+	  if (TREE_CODE (p) == POINTER_TYPE)
+	    reference.safe_push (false);
+	}
+      p = TYPE_MAIN_VARIANT (p);
+
+      /* 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);
+      else
+	{
+	  /* Rebuild pointer type from starting from canonical types using
+	     unqualified pointers and references only.  This way all such
+	     pointers will have the same alias set and will conflict with
+	     each other.
+
+	     Most of time we already have pointers or references of a given type.
+	     If not build new one just to be sure that if someone later (probably
+	     only middle-end can, as we should assign all alias classes only after
+	     finishing translation unit) builds the pointer type, the canonical type
+	     will match.  */
+	  p = TYPE_CANONICAL (p);
+	  while (!reference.is_empty ())
+	    {
+	      if (reference.pop ())
+		p = build_reference_type (p);
+	      else
+		p = build_pointer_type (p);
+	      p = TYPE_CANONICAL (TYPE_MAIN_VARIANT (p));
+	    }
+          gcc_checking_assert (TYPE_CANONICAL (p) == p);
+
+	  /* Assign the alias set to both p and t.
+	     We can not call get_alias_set (p) here as that would triger
+	     infinite recursion when p == t.
+	     In other cases it would just trigger unnecesary legwork of
+	     rebuilding the pointer again.  */
+	  if (TYPE_ALIAS_SET_KNOWN_P (p))
+	    /* Return early; we do not need to record component aliases.  */
+	    return TYPE_ALIAS_SET (t) = TYPE_ALIAS_SET (p);
+	  else
+	    {
+	      set = new_alias_set ();
+	      TYPE_ALIAS_SET (p) = set;
+	    }
+	}
+    }
+  /* In LTO the rules above needs to be part of canonical type machinery.
+     For now just punt.  */
+  else if (POINTER_TYPE_P (t) && t != ptr_type_node && in_lto_p)
     set = get_alias_set (ptr_type_node);
 
   /* Otherwise make a new alias set for this type.  */
@@ -950,7 +997,8 @@ get_alias_set (tree t)
 
   /* If this is an aggregate type or a complex type, we must record any
      component aliasing information.  */
-  if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
+  if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE
+      || TREE_CODE (t) == POINTER_TYPE)
     record_component_aliases (t);
 
   return set;
@@ -1050,6 +1098,26 @@ record_component_aliases (tree type)
 
   switch (TREE_CODE (type))
     {
+    /* We want void * to be compatible with any other pointer without really
+       dropping it to alias set 0. Doing so would make it compatible with
+       all non-pointer types too.
+
+       Make thus ptr_type_node to be a component of every pointer type.
+       Tus memory operations of type "void *" conflict with operations of type
+       "T *" without impossing a conflict with memory operations of type "Q *"
+       in case T and Q do not conflict.
+ 
+       This is not strictly necessary by the language standards, but avoids
+       common type punning mistakes.  In addition to that, we need the existence
+       of such universal pointer to implement Fortran's C_PTR type (which is
+       defined as type compatible with all C pointers) and we use it in
+       get_alias_set to give alias set to pointers to types without
+       canonical types (those are typically nameless incomplete types)
+       that needs to be also compatible with each other and with pointers to
+       complete types.  */
+    case POINTER_TYPE:
+      record_alias_subset (superset, get_alias_set (ptr_type_node));
+      break;
     case RECORD_TYPE:
     case UNION_TYPE:
     case QUAL_UNION_TYPE:

Re: Do less generous pointer globbing in alias.c

[Jan Hubicka]

> Hi,
> this patch makes it possible for non-LTO alias oracle to TBAA disambiguate
> pointer types. It makes void * conflicting with all of them and does not put it
> to alias set 0. It also preserves the property that qualifiers of pointer-to
> type should not matter to determine the alias set and that pointer to array is
> same as pointer to array element.  Finally it makes pointer void * to be
> equivalent to void ** (and more *) and to types with structural equality only.
> 
> I think those are all globbing rules we discussed for the non-LTO patch.
> 
> It does two things.  First is kind of "canonicalization" where for a given pointer
> it looks for non-pointer pointed-to type and then rebuilds is without qualifiers.
> This is fast, because build_pointer_type will reuse existing types.
> 
> It makes void * to conflict with everyting by making its alias set to be subset
> of alias set of any other pointer.  This means that writes to void * conflict
> with writes to any other pointer without really need to glob all the pointers
> to one equivalence class.
> 
> This patch makes quite some difference on C++.  For example on deal II the TBAA
> stats reports 4344358 disambiguations and 7008576 queries, while with the patch
> we get 5368737 and 5687399 queries (I did not chose deal II for reason, it is
> just random C++ file)
Actually there is oversight in my patch - the number of queries is really
number of non-disambiguations.  I will fix that as obvious tomorrow.  In both
cases the number of querries is about 11M.  The increase in number of
disambiguations is 23% (earlier patch did over 30% for Firefox)

Honza

Re: Do less generous pointer globbing in alias.c

[Richard Biener]

On Wed, 27 May 2015, Jan Hubicka wrote:

> Hi, this patch makes it possible for non-LTO alias oracle to TBAA 
> disambiguate pointer types. It makes void * conflicting with all of them 
> and does not put it to alias set 0. It also preserves the property that 
> qualifiers of pointer-to type should not matter to determine the alias 
> set and that pointer to array is same as pointer to array element.  
> Finally it makes pointer void * to be equivalent to void ** (and more *) 
> and to types with structural equality only.

void * should be equivalent to incomplete-type * as well.

> I think those are all globbing rules we discussed for the non-LTO patch.
> 
> It does two things.  First is kind of "canonicalization" where for a given pointer
> it looks for non-pointer pointed-to type and then rebuilds is without qualifiers.
> This is fast, because build_pointer_type will reuse existing types.
> 
> It makes void * to conflict with everyting by making its alias set to be subset
> of alias set of any other pointer.  This means that writes to void * conflict
> with writes to any other pointer without really need to glob all the pointers
> to one equivalence class.

I think you need to make each pointer alias-set a subset of the one of 
void * as well because both of the following is valid:

  *(void *)p = ...
  ... = *(int *)p;

and

  *(int *)p = ...
  ... = *(void *)p;

not sure if it's possible to create a testcase that fails if you do
subsetting only one-way (because alias_sets_conflict queries both
ways and I think alias_set_subset_of is not used very much, only
by tree-ssa-alias.c:aliasing_component_refs_p which won't ever
use it on two pointer alias sets).  In theory true vs. anti-dependence
should use alias_set_subset_of and trigger the above cases.  But
as those queries are done wrong a lot (in the past?) we use
alias_sets_conflict there.

For efficiency you could use a new flag similar to has_zero_child
in alias_set_entry_d ... 

More comments inline below

> This patch makes quite some difference on C++.  For example on deal II the TBAA
> stats reports 4344358 disambiguations and 7008576 queries, while with the patch
> we get 5368737 and 5687399 queries (I did not chose deal II for reason, it is
> just random C++ file)
> 
> The patch bootstrap and regtests ppc64le-linux with the following testsuite
> differences:
> @@ -30,7 +30,9 @@
>  FAIL: c-c++-common/asan/null-deref-1.c   -O3 -g  output pattern test, is ASAN:SIGSEGV
>  FAIL: c-c++-common/asan/null-deref-1.c   -Os  output pattern test, is ASAN:SIGSEGV
>  FAIL: gcc.dg/cpp/_Pragma3.c (test for excess errors)
> +XPASS: gcc.dg/alias-8.c  (test for warnings, line 11)
>  FAIL: gcc.dg/loop-8.c scan-rtl-dump-times loop2_invariant "Decided" 1
> +FAIL: gcc.dg/pr62167.c scan-tree-dump-not pre "Removing basic block"
>  FAIL: gcc.dg/sms-4.c scan-rtl-dump-times sms "SMS succeeded" 1
>  XPASS: gcc.dg/guality/example.c   -O0  execution test
>  XPASS: gcc.dg/guality/example.c   -O1  execution test
> @@ -304,6 +306,9 @@
>  FAIL: c-c++-common/asan/null-deref-1.c   -O3 -g  output pattern test, is ASAN:SIGSEGV
>  FAIL: g++.dg/cpp1y/vla-initlist1.C  -std=gnu++11 execution test
>  FAIL: g++.dg/cpp1y/vla-initlist1.C  -std=gnu++14 execution test
> +FAIL: g++.dg/ipa/ipa-icf-4.C  -std=gnu++11  scan-ipa-dump icf "Equal symbols: [67]"
> +FAIL: g++.dg/ipa/ipa-icf-4.C  -std=gnu++14  scan-ipa-dump icf "Equal symbols: [67]"
> +FAIL: g++.dg/ipa/ipa-icf-4.C  -std=gnu++98  scan-ipa-dump icf "Equal symbols: [67]"
> 
> ipa-icf-4 is about alias info now being more perceptive to block the merging.
> pr62167 seems just confused.  The template checks that memory stores are not
> unified.  It looks for BB removal message, but with the patch we get:
>   <bb 2>:
>   node.next = 0B;
>   head.0_4 = head;
>   node.prev = head.0_4;
>   head.0_4->first = &node;
>   k.1_7 = k;
>   h_8 = &heads[k.1_7];
>   heads[2].first = 0B;
>   if (head.0_4 == h_8)
>     goto <bb 3>;
>   else
>     goto <bb 5>;
> 
>   <bb 5>:
>   goto <bb 4>;
> 
>   <bb 3>:
>   p_10 = MEM[(struct head *)&heads][k.1_7].first;
> 
>   <bb 4>:
>   # p_1 = PHI <p_10(3), &node(5)>
>   _11 = p_1 != 0B;
>   _12 = (int) _11;
>   return _12;
> 
> before PR, the message is about the bb 5 sitting at critical edge removed.
> The TBAA incompatible load it looks for is optimized away by FRE:
>   head->first = &node;
> 
>   struct node *n = head->first;
> 
>   struct head *h = &heads[k];
> 
>   heads[2].first = n->next;
> 
>   if ((void*)n->prev == (void *)h)
>     p = h->first;
>   else
>     /* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next.  */
>     p = n->prev->next;
> 
> here n is known to be head->first that is known to be &node.
> The testcase runtime checks that result is Ok and passes.
> 
> Bootstrapped/regtested ppc64le-linux.
> 
> 	* alias.c (get_alias_set): Do not glob all pointer types into one;
> 	just produce euqivalence classes based on canonical type of pointed
> 	type type; make void * equivalent to void **.
> 	(record_component_aliases): Make void * to conflict with all other
> 	pointer types.
> Index: alias.c
> ===================================================================
> --- alias.c	(revision 223633)
> +++ alias.c	(working copy)
> @@ -903,35 +906,79 @@ get_alias_set (tree t)
>       the pointed-to types.  This issue has been reported to the
>       C++ committee.
>  
> -     In addition to the above canonicalization issue, with LTO
> -     we should also canonicalize `T (*)[]' to `T *' avoiding
> -     alias issues with pointer-to element types and pointer-to
> -     array types.
> -
> -     Likewise we need to deal with the situation of incomplete
> -     pointed-to types and make `*(struct X **)&a' and
> -     `*(struct X {} **)&a' alias.  Otherwise we will have to
> -     guarantee that all pointer-to incomplete type variants
> -     will be replaced by pointer-to complete type variants if
> -     they are available.
> -
> -     With LTO the convenient situation of using `void *' to
> -     access and store any pointer type will also become
> -     more apparent (and `void *' is just another pointer-to
> -     incomplete type).  Assigning alias-set zero to `void *'
> -     and all pointer-to incomplete types is a not appealing
> -     solution.  Assigning an effective alias-set zero only
> -     affecting pointers might be - by recording proper subset
> -     relationships of all pointer alias-sets.
> -
> -     Pointer-to function types are another grey area which
> -     needs caution.  Globbing them all into one alias-set
> -     or the above effective zero set would work.
> -
> -     For now just assign the same alias-set to all pointers.
> -     That's simple and avoids all the above problems.  */
> -  else if (POINTER_TYPE_P (t)
> -	   && t != ptr_type_node)
> +     For this reason go to canonical type of the unqalified pointer type.
> +     Until GCC 6 this code set all pointers sets to have alias set of
> +     ptr_type_node but that is a bad idea, because it prevents disabiguations
> +     in between pointers.  For Firefox this accounts about 20% of all
> +     disambiguations in the program.  */
> +  else if (POINTER_TYPE_P (t) && t != ptr_type_node && !in_lto_p)
> +    {
> +      tree p;
> +      auto_vec <bool, 8> reference;
> +
> +      /* Unnest all pointers and references.
> +         We also want to make pointer to array equivalent to pointer to its
> +         element. So skip all array types, too.  */
> +      for (p = t; POINTER_TYPE_P (p)
> +	   || (TREE_CODE (p) == ARRAY_TYPE && !TYPE_NONALIASED_COMPONENT (p));

As we glob vector<int> to int we should look through VECTOR_TYPE as well
here.

> +	   p = TREE_TYPE (p))
> +	{
> +	  if (TREE_CODE (p) == REFERENCE_TYPE)
> +	    reference.safe_push (true);
> +	  if (TREE_CODE (p) == POINTER_TYPE)
> +	    reference.safe_push (false);
> +	}
> +      p = TYPE_MAIN_VARIANT (p);
> +
> +      /* 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))

This should also conver incomplete types (and luckily void is incomplete).
I'm quite sure TYPE_STRUCTURAL_EQUALITY_P doesn't cover all incomplete
types.

> +	set = get_alias_set (ptr_type_node);
> +      else
> +	{
> +	  /* Rebuild pointer type from starting from canonical types using
> +	     unqualified pointers and references only.  This way all such
> +	     pointers will have the same alias set and will conflict with
> +	     each other.
> +
> +	     Most of time we already have pointers or references of a given type.
> +	     If not build new one just to be sure that if someone later (probably
> +	     only middle-end can, as we should assign all alias classes only after
> +	     finishing translation unit) builds the pointer type, the canonical type
> +	     will match.  */
> +	  p = TYPE_CANONICAL (p);
> +	  while (!reference.is_empty ())
> +	    {
> +	      if (reference.pop ())
> +		p = build_reference_type (p);
> +	      else
> +		p = build_pointer_type (p);
> +	      p = TYPE_CANONICAL (TYPE_MAIN_VARIANT (p));

err - the pointer you build should be a main variant already, and
canonical as well.  So the assert belongs here ;)

Btw, you should definitely glob POINTER_TYPE and REFERENCE_TYPE.

class T;
void foo (T **);

void bar(T *p)
{
  T*&r = p;
  foo (&r)
}

compiles fine (so foo can store T* to r).

Please verify that your code globs at least those pointer types
the former c-family get_alias_set langhook globbed.

> +	    }
> +          gcc_checking_assert (TYPE_CANONICAL (p) == p);
> +
> +	  /* Assign the alias set to both p and t.
> +	     We can not call get_alias_set (p) here as that would triger
> +	     infinite recursion when p == t.
> +	     In other cases it would just trigger unnecesary legwork of
> +	     rebuilding the pointer again.  */
> +	  if (TYPE_ALIAS_SET_KNOWN_P (p))
> +	    /* Return early; we do not need to record component aliases.  */
> +	    return TYPE_ALIAS_SET (t) = TYPE_ALIAS_SET (p);

Well, see below.  I don't like that coding-style btw, so just do

         set = TYPE_ALIAS_SET (p);

> +	  else
> +	    {
> +	      set = new_alias_set ();
> +	      TYPE_ALIAS_SET (p) = set;
> +	    }
> +	}
> +    }
> +  /* In LTO the rules above needs to be part of canonical type machinery.
> +     For now just punt.  */

I see no reason for punting for LTO here.

Btw, please check if SPEC perl still works without -fno-strict-aliasing
(it finally did after the change to do pointer globbing).

> +  else if (POINTER_TYPE_P (t) && t != ptr_type_node && in_lto_p)
>      set = get_alias_set (ptr_type_node);
>  
>    /* Otherwise make a new alias set for this type.  */
> @@ -950,7 +997,8 @@ get_alias_set (tree t)
>  
>    /* If this is an aggregate type or a complex type, we must record any
>       component aliasing information.  */
> -  if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
> +  if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE
> +      || TREE_CODE (t) == POINTER_TYPE)
>      record_component_aliases (t);

Err...
>  
>    return set;
> @@ -1050,6 +1098,26 @@ record_component_aliases (tree type)
>  
>    switch (TREE_CODE (type))
>      {
> +    /* We want void * to be compatible with any other pointer without really
> +       dropping it to alias set 0. Doing so would make it compatible with
> +       all non-pointer types too.
> +
> +       Make thus ptr_type_node to be a component of every pointer type.
> +       Tus memory operations of type "void *" conflict with operations of type
> +       "T *" without impossing a conflict with memory operations of type "Q *"
> +       in case T and Q do not conflict.
> + 
> +       This is not strictly necessary by the language standards, but avoids
> +       common type punning mistakes.  In addition to that, we need the existence
> +       of such universal pointer to implement Fortran's C_PTR type (which is
> +       defined as type compatible with all C pointers) and we use it in
> +       get_alias_set to give alias set to pointers to types without
> +       canonical types (those are typically nameless incomplete types)
> +       that needs to be also compatible with each other and with pointers to
> +       complete types.  */
> +    case POINTER_TYPE:
> +      record_alias_subset (superset, get_alias_set (ptr_type_node));
> +      break;

... I'd rather add this to the pointer handling in get_alias_set directly.
It's not really components.  Also see my comment about symmetry.

>      case RECORD_TYPE:
>      case UNION_TYPE:
>      case QUAL_UNION_TYPE:
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)

Re: Do less generous pointer globbing in alias.c

[Jan Hubicka]

> > Hi, this patch makes it possible for non-LTO alias oracle to TBAA 
> > disambiguate pointer types. It makes void * conflicting with all of them 
> > and does not put it to alias set 0. It also preserves the property that 
> > qualifiers of pointer-to type should not matter to determine the alias 
> > set and that pointer to array is same as pointer to array element.  
> > Finally it makes pointer void * to be equivalent to void ** (and more *) 
> > and to types with structural equality only.
> 
> void * should be equivalent to incomplete-type * as well.

It will be in conflict with struct FOO * when FOO is incomplete.
In non-LTO build struct FOO * do not need to conflict wqith struct BAR *.
Or do I miss something here?
> 
> > I think those are all globbing rules we discussed for the non-LTO patch.
> > 
> > It does two things.  First is kind of "canonicalization" where for a given pointer
> > it looks for non-pointer pointed-to type and then rebuilds is without qualifiers.
> > This is fast, because build_pointer_type will reuse existing types.
> > 
> > It makes void * to conflict with everyting by making its alias set to be subset
> > of alias set of any other pointer.  This means that writes to void * conflict
> > with writes to any other pointer without really need to glob all the pointers
> > to one equivalence class.
> 
> I think you need to make each pointer alias-set a subset of the one of 
> void * as well because both of the following is valid:
> 
>   *(void *)p = ...
>   ... = *(int *)p;
> 
> and
> 
>   *(int *)p = ...
>   ... = *(void *)p;

Yes, so is

struct foo {struct bar a;};

  a.a = ...
  ... = a;

and

  a = ...
  ... = a.a;

this is why conflict is symmetrization of the subset relation.

You can not record both edges into the DAG, because record_alias_subset
compute transitive closure and it would end up in loop.  I will be hapy
to add the extra flag (has_zero_child), but I would like to make it
clear it an optimization.
> 
> not sure if it's possible to create a testcase that fails if you do
> subsetting only one-way (because alias_sets_conflict queries both
> ways and I think alias_set_subset_of is not used very much, only
> by tree-ssa-alias.c:aliasing_component_refs_p which won't ever
> use it on two pointer alias sets).  In theory true vs. anti-dependence

Yep, I noticed that subsets are querried by tree-ssa-alias.  I will try to
think if it is safe WRT the code above.

> should use alias_set_subset_of and trigger the above cases.  But
> as those queries are done wrong a lot (in the past?) we use
> alias_sets_conflict there.
> 
> For efficiency you could use a new flag similar to has_zero_child
> in alias_set_entry_d ... 

Yes, I can use new flag, but it should be unnecesary.  The alias set 0
is also just common subset of all aliases (that is not done by the code).
> 
> I see no reason for punting for LTO here.

I would rather go with non-LTO first and work on solving the canonical type
issues.  Yes, I think it should work for LTO as it is and I bootstrapped and
regtested it.  I only wanted to do one step at a time.

What I do not like is that build_pointer_type simply does not do the right
thing here.  Consdier

struct a {int a};
struct b {char b};

Now if you LTO in struct *a and struct *b their canonical type will be the same.
If you call build_pointer_type, it will assign different canonical types to them.

This does not lead to wrong code, because incomplete types no longer get
TYPE_CANONICAL, but I would like first to chase out the bugs out of canonical
type computation and arrange middle-end build pointer types to be the same as
LTOed-in pointer types.
> 
> Btw, please check if SPEC perl still works without -fno-strict-aliasing
> (it finally did after the change to do pointer globbing).

OK, I have SPEC perl available, so I will do.

I am teaching now, but so will reply in detail afterwards. I was just hoping
to discuss the symmetry thing above.  I think it is not needed.

I have no problem with moving the subset code to get_alias_set and will update
the patch (including testsuite compensation).

Honza

Re: Do less generous pointer globbing in alias.c

[Jan Hubicka]

> Yes, so is
> 
> struct foo {struct bar a;};
> 
>   a.a = ...
>   ... = a;
> 
> and
> 
>   a = ...
>   ... = a.a;
> 
> this is why conflict is symmetrization of the subset relation.


OK the statement above is true, but subsets alone are not quite right for use
in aliasing_component_refs_p

 void *a;
 char **ptr=&a;
 *ptr = ....

is defined for us, but the structure-substructure equivalent is not.
I will implement the variant with extra flag after teaching and send updated
patch.

Thanks,
Honza

Re: Do less generous pointer globbing in alias.c

[Jan Hubicka]

> > Yes, so is
> > 
> > struct foo {struct bar a;};
> > 
> >   a.a = ...
> >   ... = a;
> > 
> > and
> > 
> >   a = ...
> >   ... = a.a;
> > 
> > this is why conflict is symmetrization of the subset relation.
> 
> 
> OK the statement above is true, but subsets alone are not quite right for use
> in aliasing_component_refs_p
> 
>  void *a;
>  char **ptr=&a;
>  *ptr = ....
> 
> is defined for us, but the structure-substructure equivalent is not.
> I will implement the variant with extra flag after teaching and send updated
> patch.

Hmm, what about

union t {int a; char b;};

int a;
uniont t *ptr=&a;
*ptr = ...

If we want to define this, aliasing_component_refs_p would IMO need to be symmetrized, too.
I am happy leaving this undefined.

Honza

Re: Do less generous pointer globbing in alias.c

[Richard Biener]

On May 27, 2015 5:04:13 PM GMT+02:00, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > Yes, so is
>> > 
>> > struct foo {struct bar a;};
>> > 
>> >   a.a = ...
>> >   ... = a;
>> > 
>> > and
>> > 
>> >   a = ...
>> >   ... = a.a;
>> > 
>> > this is why conflict is symmetrization of the subset relation.
>> 
>> 
>> OK the statement above is true, but subsets alone are not quite right
>for use
>> in aliasing_component_refs_p
>> 
>>  void *a;
>>  char **ptr=&a;
>>  *ptr = ....
>> 
>> is defined for us, but the structure-substructure equivalent is not.
>> I will implement the variant with extra flag after teaching and send
>updated
>> patch.
>
>Hmm, what about
>
>union t {int a; char b;};
>
>int a;
>uniont t *ptr=&a;
>*ptr = ...
>
>If we want to define this, aliasing_component_refs_p would IMO need to
>be symmetrized, too.
>I am happy leaving this undefined.

Globbing all pointers was soo  simple... :)

Note that we are in the middle-end here and have to find cross-language common grounds.  People may experience regressions towards the previous globbing so I guess the question is which is the globbing we want to remove - that is, what makes the most difference in code-generation?

Richard.

>Honza

Re: Do less generous pointer globbing in alias.c

[Jan Hubicka]

> >Hmm, what about
> >
> >union t {int a; char b;};
> >
> >int a;
> >uniont t *ptr=&a;
> >*ptr = ...
> >
> >If we want to define this, aliasing_component_refs_p would IMO need to
> >be symmetrized, too.
> >I am happy leaving this undefined.
> 
> Globbing all pointers was soo  simple... :)

Indeed, but too restrictive ;)
The testcase above is not about globbing pointers, I do not think it is going
to be handled in defined manner by mainline (or any release).
> 
> Note that we are in the middle-end here and have to find cross-language common grounds.  People may experience regressions towards the previous globbing so I guess the question is which is the globbing we want to remove - that is, what makes the most difference in code-generation?

Yes, I expect to see some PRs with regress towards the previous globbing.  I
think the globbing as proposed by my patch should be generous enough for common
bugs in user code and it is quite easy to add new rules on demand.

For high-level C++ code definitely the most important point is that you have
many different class types and we care about differentiating these (struct *a
wrt struct *b).  We also want to make difference between vtbl pointer (that is
pointer to array of functions) and other stuff.

I think I will modify the patch the following way:
1) I will move the code adding subset to get_alias_set
2) I will add flag "is_pointer" to alias set datastructure
3) I will make alias_set_subset_of to additionally consider
   every "is_pointer" set to be subset of alias set of ptr_type_node's set.

This will fix the symmetry with void *a; variable and incompatible pointer write.

We need to do two things - arrange alias set to be subset of all pointer's alias sets
and all their superset and force equivalence between pointer alias sets.
While the first can be also done by means of special flag "contains_pointer"
I think it is cleaner to keep the DAG reprsented explicitely.  After all we do not
have that many alias sets and the hash table lookups should be fast enough
(we may special case lookup in hash of size 1)

Hona
> 
> Richard.
> 
> >Honza
> 

Re: Do less generous pointer globbing in alias.c

[Jan Hubicka]

Hi,
here is updated version of patch.  It makes alias_set_subset_of to be symmetric for 
ptr_type_node and other pointer type and moves the logic of creating subsets
to get_alias_set.

I tested that perlbmk works when built at -O3 x86_64

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* alias.c (alias_set_entry_d): Add is_pointer.
	(alias_set_subset_of): Special case pointers.
	(init_alias_set_entry): Break out from ...
	(record_alias_subset): ... here.
	(get_alias_set): Do less generous pointer globbing.
	* gcc.dg/alias-8.c: Do not xfail.
	* gcc.dg/pr62167.c: Prevent FRE.
Index: alias.c
===================================================================
--- alias.c	(revision 223772)
+++ alias.c	(working copy)
@@ -183,10 +184,6 @@ struct GTY(()) alias_set_entry_d {
   /* The alias set number, as stored in MEM_ALIAS_SET.  */
   alias_set_type alias_set;
 
-  /* Nonzero if would have a child of zero: this effectively makes this
-     alias set the same as alias set zero.  */
-  int has_zero_child;
-
   /* The children of the alias set.  These are not just the immediate
      children, but, in fact, all descendants.  So, if we have:
 
@@ -195,6 +192,15 @@ struct GTY(()) alias_set_entry_d {
      continuing our example above, the children here will be all of
      `int', `double', `float', and `struct S'.  */
   hash_map<int, int, alias_set_traits> *children;
+
+  /* Nonzero if would have a child of zero: this effectively makes this
+     alias set the same as alias set zero.  */
+  bool has_zero_child;
+  /* Nonzero if alias set corresponds to pointer type itself (i.e. not to
+     aggregate contaiing pointer.
+     This is used for a special case where we need an universal pointer type
+     compatible with all other pointer types.  */
+  bool is_pointer;
 };
 typedef struct alias_set_entry_d *alias_set_entry;
 
@@ -460,12 +466,33 @@ alias_set_subset_of (alias_set_type set1
   if (set2 == 0)
     return true;
 
-  /* Otherwise, check if set1 is a subset of set2.  */
+  /* Check if set1 is a subset of set2.  */
   ase = get_alias_set_entry (set2);
   if (ase != 0
       && (ase->has_zero_child
 	  || ase->children->get (set1)))
     return true;
+
+  /* As a special case we consider alias set of "void *" to be both subset
+     and superset of every alias set of a pointer.  This extra symmetry does
+     not matter for alias_sets_conflict_p but it makes aliasing_component_refs_p
+     to return true on the following testcase:
+
+     void *ptr;
+     char **ptr2=(char **)&ptr;
+     *ptr2 = ...
+
+     This makes void * truly universal pointer type.  See pointer handling in
+     get_alias_set for more details.  */
+  if (ase && ase->is_pointer)
+    {
+      alias_set_entry ase1 = get_alias_set_entry (set1);
+
+      if (ase1 && ase1->is_pointer
+	  && (set1 == TYPE_ALIAS_SET (ptr_type_node)
+	      || set2 == TYPE_ALIAS_SET (ptr_type_node)))
+	return true;
+    }
   return false;
 }
 
@@ -764,6 +791,21 @@ alias_ptr_types_compatible_p (tree t1, t
 	  == TYPE_MAIN_VARIANT (TREE_TYPE (t2)));
 }
 
+/* Create emptry alias set entry.  */
+
+alias_set_entry
+init_alias_set_entry (alias_set_type set)
+{
+  alias_set_entry ase = ggc_cleared_alloc<alias_set_entry_d> ();
+  ase->alias_set = set;
+  ase->children
+    = hash_map<int, int, alias_set_traits>::create_ggc (64);
+  ase->has_zero_child = 0;
+  gcc_checking_assert (!get_alias_set_entry (set));
+  (*alias_sets)[set] = ase;
+  return ase;
+}
+
 /* Return the alias set for T, which may be either a type or an
    expression.  Call language-specific routine for help, if needed.  */
 
@@ -903,35 +945,92 @@ get_alias_set (tree t)
      the pointed-to types.  This issue has been reported to the
      C++ committee.
 
-     In addition to the above canonicalization issue, with LTO
-     we should also canonicalize `T (*)[]' to `T *' avoiding
-     alias issues with pointer-to element types and pointer-to
-     array types.
-
-     Likewise we need to deal with the situation of incomplete
-     pointed-to types and make `*(struct X **)&a' and
-     `*(struct X {} **)&a' alias.  Otherwise we will have to
-     guarantee that all pointer-to incomplete type variants
-     will be replaced by pointer-to complete type variants if
-     they are available.
-
-     With LTO the convenient situation of using `void *' to
-     access and store any pointer type will also become
-     more apparent (and `void *' is just another pointer-to
-     incomplete type).  Assigning alias-set zero to `void *'
-     and all pointer-to incomplete types is a not appealing
-     solution.  Assigning an effective alias-set zero only
-     affecting pointers might be - by recording proper subset
-     relationships of all pointer alias-sets.
-
-     Pointer-to function types are another grey area which
-     needs caution.  Globbing them all into one alias-set
-     or the above effective zero set would work.
-
-     For now just assign the same alias-set to all pointers.
-     That's simple and avoids all the above problems.  */
-  else if (POINTER_TYPE_P (t)
-	   && t != ptr_type_node)
+     For this reason go to canonical type of the unqalified pointer type.
+     Until GCC 6 this code set all pointers sets to have alias set of
+     ptr_type_node but that is a bad idea, because it prevents disabiguations
+     in between pointers.  For Firefox this accounts about 20% of all
+     disambiguations in the program.  */
+  else if (POINTER_TYPE_P (t) && t != ptr_type_node && !in_lto_p)
+    {
+      tree p;
+      auto_vec <bool, 8> reference;
+
+      /* Unnest all pointers and references.
+         We also want to make pointer to array equivalent to pointer to its
+         element. So skip all array types, too.  */
+      for (p = t; POINTER_TYPE_P (p)
+	   || (TREE_CODE (p) == ARRAY_TYPE && !TYPE_NONALIASED_COMPONENT (p));
+	   p = TREE_TYPE (p))
+	{
+	  if (TREE_CODE (p) == REFERENCE_TYPE)
+	    reference.safe_push (true);
+	  if (TREE_CODE (p) == POINTER_TYPE)
+	    reference.safe_push (false);
+	}
+      p = TYPE_MAIN_VARIANT (p);
+
+      /* 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);
+      else
+	{
+	  /* Rebuild pointer type from starting from canonical types using
+	     unqualified pointers and references only.  This way all such
+	     pointers will have the same alias set and will conflict with
+	     each other.
+
+	     Most of time we already have pointers or references of a given type.
+	     If not we build new one just to be sure that if someone later
+	     (probably only middle-end can, as we should assign all alias
+	     classes only after finishing translation unit) builds the pointer
+	     type, the canonical type will match.  */
+	  p = TYPE_CANONICAL (p);
+	  while (!reference.is_empty ())
+	    {
+	      if (reference.pop ())
+		p = build_reference_type (p);
+	      else
+		p = build_pointer_type (p);
+	      p = TYPE_CANONICAL (TYPE_MAIN_VARIANT (p));
+	    }
+          gcc_checking_assert (TYPE_CANONICAL (p) == p);
+
+	  /* Assign the alias set to both p and t.
+	     We can not call get_alias_set (p) here as that would trigger
+	     infinite recursion when p == t.  In other cases it would just
+	     trigger unnecesary legwork of rebuilding the pointer again.  */
+	  if (TYPE_ALIAS_SET_KNOWN_P (p))
+	    set = TYPE_ALIAS_SET (p);
+	  else
+	    {
+	      set = new_alias_set ();
+	      TYPE_ALIAS_SET (p) = set;
+	      /* We want void * to be compatible with any other pointer without
+		 really dropping it to alias set 0. Doing so would make it
+	         compatible with all non-pointer types too.
+
+		 We make ptr_type_node to be a component of every pointer
+		 type.  Thus memory operations of type "void *" conflict with
+		 operations of type "T *" without impossing a conflict with
+		 memory operations of type "Q *" in case T and Q do not conflict.
+ 
+		 This is not strictly necessary by the C/C++ language
+		 standards, but avoids common type punning mistakes.  In
+		 addition to that, we need the existence of such universal
+		 pointer to implement Fortran's C_PTR type (which is defined as
+		 type compatible with all C pointers).  */
+	      record_alias_subset (set, get_alias_set (ptr_type_node));
+	    }
+	}
+    }
+  /* In LTO the rules above needs to be part of canonical type machinery.
+     For now just punt.  */
+  else if (POINTER_TYPE_P (t) && t != ptr_type_node && in_lto_p)
     set = get_alias_set (ptr_type_node);
 
   /* Otherwise make a new alias set for this type.  */
@@ -953,6 +1052,15 @@ get_alias_set (tree t)
   if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
     record_component_aliases (t);
 
+  /* We treat pointer types specially in alias_set_subset_of.  */
+  if (POINTER_TYPE_P (t) && set)
+    {
+      alias_set_entry ase = get_alias_set_entry (set);
+      if (!ase)
+	ase = init_alias_set_entry (set);
+      ase->is_pointer = true;
+    }
+
   return set;
 }
 
@@ -1003,12 +1111,7 @@ record_alias_subset (alias_set_type supe
     {
       /* Create an entry for the SUPERSET, so that we have a place to
 	 attach the SUBSET.  */
-      superset_entry = ggc_cleared_alloc<alias_set_entry_d> ();
-      superset_entry->alias_set = superset;
-      superset_entry->children
-	= hash_map<int, int, alias_set_traits>::create_ggc (64);
-      superset_entry->has_zero_child = 0;
-      (*alias_sets)[superset] = superset_entry;
+      superset_entry = init_alias_set_entry (superset);
     }
 
   if (subset == 0)
Index: testsuite/gcc.dg/alias-8.c
===================================================================
--- testsuite/gcc.dg/alias-8.c	(revision 223772)
+++ testsuite/gcc.dg/alias-8.c	(working copy)
@@ -8,5 +8,5 @@ struct s {
 void
 func(struct s *ptr)
 {
-  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { xfail *-*-* } } */
+  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { } } */
 }
Index: testsuite/gcc.dg/pr62167.c
===================================================================
--- testsuite/gcc.dg/pr62167.c	(revision 223772)
+++ testsuite/gcc.dg/pr62167.c	(working copy)
@@ -29,6 +29,8 @@ main ()
 
   node.prev = (void *)head;
 
+  asm("":"=m"(node.prev));
+
   head->first = &node;
 
   struct node *n = head->first;

Re: Do less generous pointer globbing in alias.c

[Richard Biener]

On Thu, 28 May 2015, Jan Hubicka wrote:

> Hi,
> here is updated version of patch.  It makes alias_set_subset_of to be symmetric for 
> ptr_type_node and other pointer type and moves the logic of creating subsets
> to get_alias_set.
> 
> I tested that perlbmk works when built at -O3 x86_64
> 
> Bootstrapped/regtested x86_64-linux, OK?
> 
> Honza
> 
> 	* alias.c (alias_set_entry_d): Add is_pointer.
> 	(alias_set_subset_of): Special case pointers.
> 	(init_alias_set_entry): Break out from ...
> 	(record_alias_subset): ... here.
> 	(get_alias_set): Do less generous pointer globbing.
> 	* gcc.dg/alias-8.c: Do not xfail.
> 	* gcc.dg/pr62167.c: Prevent FRE.
> Index: alias.c
> ===================================================================
> --- alias.c	(revision 223772)
> +++ alias.c	(working copy)
> @@ -183,10 +184,6 @@ struct GTY(()) alias_set_entry_d {
>    /* The alias set number, as stored in MEM_ALIAS_SET.  */
>    alias_set_type alias_set;
>  
> -  /* Nonzero if would have a child of zero: this effectively makes this
> -     alias set the same as alias set zero.  */
> -  int has_zero_child;
> -
>    /* The children of the alias set.  These are not just the immediate
>       children, but, in fact, all descendants.  So, if we have:
>  
> @@ -195,6 +192,15 @@ struct GTY(()) alias_set_entry_d {
>       continuing our example above, the children here will be all of
>       `int', `double', `float', and `struct S'.  */
>    hash_map<int, int, alias_set_traits> *children;
> +
> +  /* Nonzero if would have a child of zero: this effectively makes this
> +     alias set the same as alias set zero.  */
> +  bool has_zero_child;
> +  /* Nonzero if alias set corresponds to pointer type itself (i.e. not to
> +     aggregate contaiing pointer.
> +     This is used for a special case where we need an universal pointer type
> +     compatible with all other pointer types.  */
> +  bool is_pointer;
>  };
>  typedef struct alias_set_entry_d *alias_set_entry;
>  
> @@ -460,12 +466,33 @@ alias_set_subset_of (alias_set_type set1
>    if (set2 == 0)
>      return true;
>  
> -  /* Otherwise, check if set1 is a subset of set2.  */
> +  /* Check if set1 is a subset of set2.  */
>    ase = get_alias_set_entry (set2);
>    if (ase != 0
>        && (ase->has_zero_child
>  	  || ase->children->get (set1)))
>      return true;
> +
> +  /* As a special case we consider alias set of "void *" to be both subset
> +     and superset of every alias set of a pointer.  This extra symmetry does
> +     not matter for alias_sets_conflict_p but it makes aliasing_component_refs_p
> +     to return true on the following testcase:
> +
> +     void *ptr;
> +     char **ptr2=(char **)&ptr;
> +     *ptr2 = ...
> +
> +     This makes void * truly universal pointer type.  See pointer handling in
> +     get_alias_set for more details.  */
> +  if (ase && ase->is_pointer)
> +    {
> +      alias_set_entry ase1 = get_alias_set_entry (set1);
> +
> +      if (ase1 && ase1->is_pointer
> +	  && (set1 == TYPE_ALIAS_SET (ptr_type_node)
> +	      || set2 == TYPE_ALIAS_SET (ptr_type_node)))
> +	return true;
> +    }
>    return false;
>  }
>  
> @@ -764,6 +791,21 @@ alias_ptr_types_compatible_p (tree t1, t
>  	  == TYPE_MAIN_VARIANT (TREE_TYPE (t2)));
>  }
>  
> +/* Create emptry alias set entry.  */
> +
> +alias_set_entry
> +init_alias_set_entry (alias_set_type set)
> +{
> +  alias_set_entry ase = ggc_cleared_alloc<alias_set_entry_d> ();

no need to use cleared_alloc if you also init ->is_pointer to false.

> +  ase->alias_set = set;
> +  ase->children
> +    = hash_map<int, int, alias_set_traits>::create_ggc (64);

that seems a bit excessive, esp. for pointers which won't end
up with any children?  So better make children lazily allocated
in record_alias_subset.

> +  ase->has_zero_child = 0;
> +  gcc_checking_assert (!get_alias_set_entry (set));
> +  (*alias_sets)[set] = ase;
> +  return ase;
> +}
> +
>  /* Return the alias set for T, which may be either a type or an
>     expression.  Call language-specific routine for help, if needed.  */
>  
> @@ -903,35 +945,92 @@ get_alias_set (tree t)
>       the pointed-to types.  This issue has been reported to the
>       C++ committee.
>  
> -     In addition to the above canonicalization issue, with LTO
> -     we should also canonicalize `T (*)[]' to `T *' avoiding
> -     alias issues with pointer-to element types and pointer-to
> -     array types.
> -
> -     Likewise we need to deal with the situation of incomplete
> -     pointed-to types and make `*(struct X **)&a' and
> -     `*(struct X {} **)&a' alias.  Otherwise we will have to
> -     guarantee that all pointer-to incomplete type variants
> -     will be replaced by pointer-to complete type variants if
> -     they are available.
> -
> -     With LTO the convenient situation of using `void *' to
> -     access and store any pointer type will also become
> -     more apparent (and `void *' is just another pointer-to
> -     incomplete type).  Assigning alias-set zero to `void *'
> -     and all pointer-to incomplete types is a not appealing
> -     solution.  Assigning an effective alias-set zero only
> -     affecting pointers might be - by recording proper subset
> -     relationships of all pointer alias-sets.
> -
> -     Pointer-to function types are another grey area which
> -     needs caution.  Globbing them all into one alias-set
> -     or the above effective zero set would work.
> -
> -     For now just assign the same alias-set to all pointers.
> -     That's simple and avoids all the above problems.  */
> -  else if (POINTER_TYPE_P (t)
> -	   && t != ptr_type_node)
> +     For this reason go to canonical type of the unqalified pointer type.
> +     Until GCC 6 this code set all pointers sets to have alias set of
> +     ptr_type_node but that is a bad idea, because it prevents disabiguations
> +     in between pointers.  For Firefox this accounts about 20% of all
> +     disambiguations in the program.  */
> +  else if (POINTER_TYPE_P (t) && t != ptr_type_node && !in_lto_p)
> +    {
> +      tree p;
> +      auto_vec <bool, 8> reference;
> +
> +      /* Unnest all pointers and references.
> +         We also want to make pointer to array equivalent to pointer to its
> +         element. So skip all array types, too.  */
> +      for (p = t; POINTER_TYPE_P (p)
> +	   || (TREE_CODE (p) == ARRAY_TYPE && !TYPE_NONALIASED_COMPONENT (p));
> +	   p = TREE_TYPE (p))
> +	{
> +	  if (TREE_CODE (p) == REFERENCE_TYPE)
> +	    reference.safe_push (true);
> +	  if (TREE_CODE (p) == POINTER_TYPE)
> +	    reference.safe_push (false);
> +	}
> +      p = TYPE_MAIN_VARIANT (p);
> +
> +      /* 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);
> +      else
> +	{
> +	  /* Rebuild pointer type from starting from canonical types using
> +	     unqualified pointers and references only.  This way all such
> +	     pointers will have the same alias set and will conflict with
> +	     each other.
> +
> +	     Most of time we already have pointers or references of a given type.
> +	     If not we build new one just to be sure that if someone later
> +	     (probably only middle-end can, as we should assign all alias
> +	     classes only after finishing translation unit) builds the pointer
> +	     type, the canonical type will match.  */
> +	  p = TYPE_CANONICAL (p);
> +	  while (!reference.is_empty ())
> +	    {
> +	      if (reference.pop ())
> +		p = build_reference_type (p);
> +	      else
> +		p = build_pointer_type (p);
> +	      p = TYPE_CANONICAL (TYPE_MAIN_VARIANT (p));
> +	    }
> +          gcc_checking_assert (TYPE_CANONICAL (p) == p);
> +
> +	  /* Assign the alias set to both p and t.
> +	     We can not call get_alias_set (p) here as that would trigger
> +	     infinite recursion when p == t.  In other cases it would just
> +	     trigger unnecesary legwork of rebuilding the pointer again.  */
> +	  if (TYPE_ALIAS_SET_KNOWN_P (p))
> +	    set = TYPE_ALIAS_SET (p);
> +	  else
> +	    {
> +	      set = new_alias_set ();
> +	      TYPE_ALIAS_SET (p) = set;
> +	      /* We want void * to be compatible with any other pointer without
> +		 really dropping it to alias set 0. Doing so would make it
> +	         compatible with all non-pointer types too.
> +
> +		 We make ptr_type_node to be a component of every pointer
> +		 type.  Thus memory operations of type "void *" conflict with
> +		 operations of type "T *" without impossing a conflict with
> +		 memory operations of type "Q *" in case T and Q do not conflict.
> + 
> +		 This is not strictly necessary by the C/C++ language
> +		 standards, but avoids common type punning mistakes.  In
> +		 addition to that, we need the existence of such universal
> +		 pointer to implement Fortran's C_PTR type (which is defined as
> +		 type compatible with all C pointers).  */
> +	      record_alias_subset (set, get_alias_set (ptr_type_node));

I still wonder why you do this instead of changing alias_sets_conflict
in the same way you changed alias_set_subset_of.

Patch looks ok otherwise but please leave the patch for others to
comment on for a while.

Thanks,
Richard.

> +	    }
> +	}
> +    }
> +  /* In LTO the rules above needs to be part of canonical type machinery.
> +     For now just punt.  */
> +  else if (POINTER_TYPE_P (t) && t != ptr_type_node && in_lto_p)
>      set = get_alias_set (ptr_type_node);
>  
>    /* Otherwise make a new alias set for this type.  */
> @@ -953,6 +1052,15 @@ get_alias_set (tree t)
>    if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
>      record_component_aliases (t);
>  
> +  /* We treat pointer types specially in alias_set_subset_of.  */
> +  if (POINTER_TYPE_P (t) && set)
> +    {
> +      alias_set_entry ase = get_alias_set_entry (set);
> +      if (!ase)
> +	ase = init_alias_set_entry (set);
> +      ase->is_pointer = true;
> +    }
> +
>    return set;
>  }
>  
> @@ -1003,12 +1111,7 @@ record_alias_subset (alias_set_type supe
>      {
>        /* Create an entry for the SUPERSET, so that we have a place to
>  	 attach the SUBSET.  */
> -      superset_entry = ggc_cleared_alloc<alias_set_entry_d> ();
> -      superset_entry->alias_set = superset;
> -      superset_entry->children
> -	= hash_map<int, int, alias_set_traits>::create_ggc (64);
> -      superset_entry->has_zero_child = 0;
> -      (*alias_sets)[superset] = superset_entry;
> +      superset_entry = init_alias_set_entry (superset);
>      }
>  
>    if (subset == 0)
> Index: testsuite/gcc.dg/alias-8.c
> ===================================================================
> --- testsuite/gcc.dg/alias-8.c	(revision 223772)
> +++ testsuite/gcc.dg/alias-8.c	(working copy)
> @@ -8,5 +8,5 @@ struct s {
>  void
>  func(struct s *ptr)
>  {
> -  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { xfail *-*-* } } */
> +  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { } } */
>  }
> Index: testsuite/gcc.dg/pr62167.c
> ===================================================================
> --- testsuite/gcc.dg/pr62167.c	(revision 223772)
> +++ testsuite/gcc.dg/pr62167.c	(working copy)
> @@ -29,6 +29,8 @@ main ()
>  
>    node.prev = (void *)head;
>  
> +  asm("":"=m"(node.prev));
> +
>    head->first = &node;
>  
>    struct node *n = head->first;
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)

Re: Do less generous pointer globbing in alias.c

[Jan Hubicka]

> > +alias_set_entry
> > +init_alias_set_entry (alias_set_type set)
> > +{
> > +  alias_set_entry ase = ggc_cleared_alloc<alias_set_entry_d> ();
> 
> no need to use cleared_alloc if you also init ->is_pointer to false.
OK, will update the patch.
> 
> > +  ase->alias_set = set;
> > +  ase->children
> > +    = hash_map<int, int, alias_set_traits>::create_ggc (64);
> 
> that seems a bit excessive, esp. for pointers which won't end
> up with any children?  So better make children lazily allocated
> in record_alias_subset.

All pointers that are not in alias set of ptr_type_node will have a child.
So there is only one childless pointer set.  I will update the code though.
> 
> I still wonder why you do this instead of changing alias_sets_conflict
> in the same way you changed alias_set_subset_of.

Because I would need two flags otherwise. One denoting alias sets that
are pointers (who needs special treatment for subset_of) and one denoting
alias set that contains pointer.

i.e. for:
struct {int *a,b;}

I need to have its alias set to contain all of setof(int), setof(int *), setof(void *).
I however do not want setof(struct {int *a,b;}) to be subset of setof(void *)

Honza
> 
> Patch looks ok otherwise but please leave the patch for others to
> comment on for a while.
> 
> Thanks,
> Richard.
> 
> > +	    }
> > +	}
> > +    }
> > +  /* In LTO the rules above needs to be part of canonical type machinery.
> > +     For now just punt.  */
> > +  else if (POINTER_TYPE_P (t) && t != ptr_type_node && in_lto_p)
> >      set = get_alias_set (ptr_type_node);
> >  
> >    /* Otherwise make a new alias set for this type.  */
> > @@ -953,6 +1052,15 @@ get_alias_set (tree t)
> >    if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
> >      record_component_aliases (t);
> >  
> > +  /* We treat pointer types specially in alias_set_subset_of.  */
> > +  if (POINTER_TYPE_P (t) && set)
> > +    {
> > +      alias_set_entry ase = get_alias_set_entry (set);
> > +      if (!ase)
> > +	ase = init_alias_set_entry (set);
> > +      ase->is_pointer = true;
> > +    }
> > +
> >    return set;
> >  }
> >  
> > @@ -1003,12 +1111,7 @@ record_alias_subset (alias_set_type supe
> >      {
> >        /* Create an entry for the SUPERSET, so that we have a place to
> >  	 attach the SUBSET.  */
> > -      superset_entry = ggc_cleared_alloc<alias_set_entry_d> ();
> > -      superset_entry->alias_set = superset;
> > -      superset_entry->children
> > -	= hash_map<int, int, alias_set_traits>::create_ggc (64);
> > -      superset_entry->has_zero_child = 0;
> > -      (*alias_sets)[superset] = superset_entry;
> > +      superset_entry = init_alias_set_entry (superset);
> >      }
> >  
> >    if (subset == 0)
> > Index: testsuite/gcc.dg/alias-8.c
> > ===================================================================
> > --- testsuite/gcc.dg/alias-8.c	(revision 223772)
> > +++ testsuite/gcc.dg/alias-8.c	(working copy)
> > @@ -8,5 +8,5 @@ struct s {
> >  void
> >  func(struct s *ptr)
> >  {
> > -  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { xfail *-*-* } } */
> > +  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { } } */
> >  }
> > Index: testsuite/gcc.dg/pr62167.c
> > ===================================================================
> > --- testsuite/gcc.dg/pr62167.c	(revision 223772)
> > +++ testsuite/gcc.dg/pr62167.c	(working copy)
> > @@ -29,6 +29,8 @@ main ()
> >  
> >    node.prev = (void *)head;
> >  
> > +  asm("":"=m"(node.prev));
> > +
> >    head->first = &node;
> >  
> >    struct node *n = head->first;
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)

Re: Do less generous pointer globbing in alias.c

[Jan Hubicka]

hello,
only providing you the testcase why I need transitive closure of "contains
pointer" via the extra child I noticed that there is extra symmetry to handle:

     struct a {void *ptr;}
     char **ptr = (char **)&a.ptr;
     ptr = ...

This one doesn't really fly with my extra subset code, because ptr is not
universal pointer, but struct a contains one and thus should conflict with
every pointer.  Adding every pointer as subset of every structure with
universal pointer is impractical (childs of those structures would be appearing
as new pointer types get alias sets) and thus indeed it is better to handle it
same way as alias set 0 - by a special case in alias_set_subset_of
and alias_sets_conflict_p.

So I added the second flag - has_pointer that is transitive closure of
is_pointer and added the special case to alias_sets_conflict_p instead of 
adding the extra subset relation into the DAG.

I also added statistics and made changes you suggested (making child
hash to be possibly NULL and clenaing up alias set conflict construction)

I also constructed a testcase that covers all the new code paths.

The patch bootstrapped/regtested ppc64-linux.  I am not bound to teaching
next week, so if I hear no negative comments, I will schedule commiting the
patch for weekend to deal with possible fallout.

There are few cleanups possible incrementally - i.e. the hash set seems
irrationaly large for average type, we could avoid some pointer travelling
overhead and we could also do better at alias_sets_must_conflict_p.

Honza

	* alias.c (alias_set_entry_d): Add is_pointer and has_pointer.
	(alias_stats): Add num_universal.
	(alias_set_subset_of): Special case pointers; be ready for NULL
	children.
	(alias_sets_conflict_p): Special case pointers; be ready for NULL
	children.
	(init_alias_set_entry): Break out from ...
	(record_alias_subset): ... here; propagate new fields;
	allocate children only when really needed.
	(get_alias_set): Do less generous pointer globbing.
	(dump_alias_stats_in_alias_c): Update statistics.
	* gcc.dg/alias-8.c: Do not xfail.
	* gcc.dg/pr62167.c: Prevent FRE.
	* gcc.dg/alias-14.c: New testcase.
Index: alias.c
===================================================================
--- alias.c	(revision 223772)
+++ alias.c	(working copy)
@@ -183,10 +184,6 @@ struct GTY(()) alias_set_entry_d {
   /* The alias set number, as stored in MEM_ALIAS_SET.  */
   alias_set_type alias_set;
 
-  /* Nonzero if would have a child of zero: this effectively makes this
-     alias set the same as alias set zero.  */
-  int has_zero_child;
-
   /* The children of the alias set.  These are not just the immediate
      children, but, in fact, all descendants.  So, if we have:
 
@@ -195,6 +192,17 @@ struct GTY(()) alias_set_entry_d {
      continuing our example above, the children here will be all of
      `int', `double', `float', and `struct S'.  */
   hash_map<int, int, alias_set_traits> *children;
+
+  /* Nonzero if would have a child of zero: this effectively makes this
+     alias set the same as alias set zero.  */
+  bool has_zero_child;
+  /* Nonzero if alias set corresponds to pointer type itself (i.e. not to
+     aggregate contaiing pointer.
+     This is used for a special case where we need an universal pointer type
+     compatible with all other pointer types.  */
+  bool is_pointer;
+  /* Nonzero if is_pointer or if one of childs have has_pointer set.  */
+  bool has_pointer;
 };
 typedef struct alias_set_entry_d *alias_set_entry;
 
@@ -222,6 +230,7 @@ static struct {
   unsigned long long num_same_objects;
   unsigned long long num_volatile;
   unsigned long long num_dag;
+  unsigned long long num_universal;
   unsigned long long num_disambiguated;
 } alias_stats;
 
@@ -454,18 +463,58 @@ mems_in_disjoint_alias_sets_p (const_rtx
 bool
 alias_set_subset_of (alias_set_type set1, alias_set_type set2)
 {
-  alias_set_entry ase;
+  alias_set_entry ase2;
 
   /* Everything is a subset of the "aliases everything" set.  */
   if (set2 == 0)
     return true;
 
-  /* Otherwise, check if set1 is a subset of set2.  */
-  ase = get_alias_set_entry (set2);
-  if (ase != 0
-      && (ase->has_zero_child
-	  || ase->children->get (set1)))
+  /* Check if set1 is a subset of set2.  */
+  ase2 = get_alias_set_entry (set2);
+  if (ase2 != 0
+      && (ase2->has_zero_child
+	  || (ase2->children && ase2->children->get (set1))))
     return true;
+
+  /* As a special case we consider alias set of "void *" to be both subset
+     and superset of every alias set of a pointer.  This extra symmetry does
+     not matter for alias_sets_conflict_p but it makes aliasing_component_refs_p
+     to return true on the following testcase:
+
+     void *ptr;
+     char **ptr2=(char **)&ptr;
+     *ptr2 = ...
+
+     Additionally if a set contains universal pointer, we consider every pointer
+     to be a subset of it, but we do not represent this explicitely - doing so
+     would require us to update transitive closure each time we introduce new
+     pointer type.  This makes aliasing_component_refs_p to return true
+     on the following testcase:
+
+     struct a {void *ptr;}
+     char **ptr = (char **)&a.ptr;
+     ptr = ...
+
+     This makes void * truly universal pointer type.  See pointer handling in
+     get_alias_set for more details.  */
+  if (ase2 && ase2->has_pointer)
+    {
+      alias_set_entry ase1 = get_alias_set_entry (set1);
+
+      if (ase1 && ase1->is_pointer)
+	{
+          alias_set_type voidptr_set = TYPE_ALIAS_SET (ptr_type_node);
+	  /* If one is ptr_type_node and other is pointer, then we consider
+ 	     them subset of each other.  */
+	  if (set1 == voidptr_set || set2 == voidptr_set)
+	    return true;
+	  /* If SET2 contains universal pointer's alias set, then we consdier
+ 	     every (non-universal) pointer.  */
+	  if (ase2->children && set1 != voidptr_set
+	      && ase2->children->get (voidptr_set))
+	    return true;
+	}
+    }
   return false;
 }
 
@@ -474,29 +523,68 @@ alias_set_subset_of (alias_set_type set1
 int
 alias_sets_conflict_p (alias_set_type set1, alias_set_type set2)
 {
-  alias_set_entry ase;
+  alias_set_entry ase1;
+  alias_set_entry ase2;
 
   /* The easy case.  */
   if (alias_sets_must_conflict_p (set1, set2))
     return 1;
 
   /* See if the first alias set is a subset of the second.  */
-  ase = get_alias_set_entry (set1);
-  if (ase != 0
-      && ase->children->get (set2))
+  ase1 = get_alias_set_entry (set1);
+  if (ase1 != 0
+      && ase1->children && ase1->children->get (set2))
     {
       ++alias_stats.num_dag;
       return 1;
     }
 
   /* Now do the same, but with the alias sets reversed.  */
-  ase = get_alias_set_entry (set2);
-  if (ase != 0
-      && ase->children->get (set1))
+  ase2 = get_alias_set_entry (set2);
+  if (ase2 != 0
+      && ase2->children && ase2->children->get (set1))
     {
       ++alias_stats.num_dag;
       return 1;
     }
+
+  /* We want void * to be compatible with any other pointer without
+     really dropping it to alias set 0. Doing so would make it
+     compatible with all non-pointer types too.
+
+     This is not strictly necessary by the C/C++ language
+     standards, but avoids common type punning mistakes.  In
+     addition to that, we need the existence of such universal
+     pointer to implement Fortran's C_PTR type (which is defined as
+     type compatible with all C pointers).  */
+  if (ase1 && ase2 && ase1->has_pointer && ase2->has_pointer)
+    {
+      alias_set_type voidptr_set = TYPE_ALIAS_SET (ptr_type_node);
+
+      /* If one of the sets corresponds to universal pointer,
+ 	 we consider it to conflict with anything that is
+	 or contains pointer.  */
+      if (set1 == voidptr_set || set2 == voidptr_set)
+	{
+	  ++alias_stats.num_universal;
+	  return true;
+	}
+     /* If one of sets is (non-universal) pointer and the other
+ 	contains universal pointer, we also get conflict.  */
+     if (ase1->is_pointer && set2 != voidptr_set
+	 && ase2->children && ase2->children->get (voidptr_set))
+	{
+	  ++alias_stats.num_universal;
+	  return true;
+	}
+     if (ase2->is_pointer && set1 != voidptr_set
+	 && ase1->children && ase1->children->get (voidptr_set))
+	{
+	  ++alias_stats.num_universal;
+	  return true;
+	}
+    }
+
   ++alias_stats.num_disambiguated;
 
   /* The two alias sets are distinct and neither one is the
@@ -764,6 +852,22 @@ alias_ptr_types_compatible_p (tree t1, t
 	  == TYPE_MAIN_VARIANT (TREE_TYPE (t2)));
 }
 
+/* Create emptry alias set entry.  */
+
+alias_set_entry
+init_alias_set_entry (alias_set_type set)
+{
+  alias_set_entry ase = ggc_alloc<alias_set_entry_d> ();
+  ase->alias_set = set;
+  ase->children = NULL;
+  ase->has_zero_child = false;
+  ase->is_pointer = false;
+  ase->has_pointer = false;
+  gcc_checking_assert (!get_alias_set_entry (set));
+  (*alias_sets)[set] = ase;
+  return ase;
+}
+
 /* Return the alias set for T, which may be either a type or an
    expression.  Call language-specific routine for help, if needed.  */
 
@@ -903,35 +1007,77 @@ get_alias_set (tree t)
      the pointed-to types.  This issue has been reported to the
      C++ committee.
 
-     In addition to the above canonicalization issue, with LTO
-     we should also canonicalize `T (*)[]' to `T *' avoiding
-     alias issues with pointer-to element types and pointer-to
-     array types.
-
-     Likewise we need to deal with the situation of incomplete
-     pointed-to types and make `*(struct X **)&a' and
-     `*(struct X {} **)&a' alias.  Otherwise we will have to
-     guarantee that all pointer-to incomplete type variants
-     will be replaced by pointer-to complete type variants if
-     they are available.
-
-     With LTO the convenient situation of using `void *' to
-     access and store any pointer type will also become
-     more apparent (and `void *' is just another pointer-to
-     incomplete type).  Assigning alias-set zero to `void *'
-     and all pointer-to incomplete types is a not appealing
-     solution.  Assigning an effective alias-set zero only
-     affecting pointers might be - by recording proper subset
-     relationships of all pointer alias-sets.
-
-     Pointer-to function types are another grey area which
-     needs caution.  Globbing them all into one alias-set
-     or the above effective zero set would work.
-
-     For now just assign the same alias-set to all pointers.
-     That's simple and avoids all the above problems.  */
-  else if (POINTER_TYPE_P (t)
-	   && t != ptr_type_node)
+     For this reason go to canonical type of the unqalified pointer type.
+     Until GCC 6 this code set all pointers sets to have alias set of
+     ptr_type_node but that is a bad idea, because it prevents disabiguations
+     in between pointers.  For Firefox this accounts about 20% of all
+     disambiguations in the program.  */
+  else if (POINTER_TYPE_P (t) && t != ptr_type_node && !in_lto_p)
+    {
+      tree p;
+      auto_vec <bool, 8> reference;
+
+      /* Unnest all pointers and references.
+         We also want to make pointer to array equivalent to pointer to its
+         element. So skip all array types, too.  */
+      for (p = t; POINTER_TYPE_P (p)
+	   || (TREE_CODE (p) == ARRAY_TYPE && !TYPE_NONALIASED_COMPONENT (p));
+	   p = TREE_TYPE (p))
+	{
+	  if (TREE_CODE (p) == REFERENCE_TYPE)
+	    reference.safe_push (true);
+	  if (TREE_CODE (p) == POINTER_TYPE)
+	    reference.safe_push (false);
+	}
+      p = TYPE_MAIN_VARIANT (p);
+
+      /* 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);
+      else
+	{
+	  /* Rebuild pointer type from starting from canonical types using
+	     unqualified pointers and references only.  This way all such
+	     pointers will have the same alias set and will conflict with
+	     each other.
+
+	     Most of time we already have pointers or references of a given type.
+	     If not we build new one just to be sure that if someone later
+	     (probably only middle-end can, as we should assign all alias
+	     classes only after finishing translation unit) builds the pointer
+	     type, the canonical type will match.  */
+	  p = TYPE_CANONICAL (p);
+	  while (!reference.is_empty ())
+	    {
+	      if (reference.pop ())
+		p = build_reference_type (p);
+	      else
+		p = build_pointer_type (p);
+	      p = TYPE_CANONICAL (TYPE_MAIN_VARIANT (p));
+	    }
+          gcc_checking_assert (TYPE_CANONICAL (p) == p);
+
+	  /* Assign the alias set to both p and t.
+	     We can not call get_alias_set (p) here as that would trigger
+	     infinite recursion when p == t.  In other cases it would just
+	     trigger unnecesary legwork of rebuilding the pointer again.  */
+	  if (TYPE_ALIAS_SET_KNOWN_P (p))
+	    set = TYPE_ALIAS_SET (p);
+	  else
+	    {
+	      set = new_alias_set ();
+	      TYPE_ALIAS_SET (p) = set;
+	    }
+	}
+    }
+  /* In LTO the rules above needs to be part of canonical type machinery.
+     For now just punt.  */
+  else if (POINTER_TYPE_P (t) && t != ptr_type_node && in_lto_p)
     set = get_alias_set (ptr_type_node);
 
   /* Otherwise make a new alias set for this type.  */
@@ -953,6 +1099,16 @@ get_alias_set (tree t)
   if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
     record_component_aliases (t);
 
+  /* We treat pointer types specially in alias_set_subset_of.  */
+  if (POINTER_TYPE_P (t) && set)
+    {
+      alias_set_entry ase = get_alias_set_entry (set);
+      if (!ase)
+	ase = init_alias_set_entry (set);
+      ase->is_pointer = true;
+      ase->has_pointer = true;
+    }
+
   return set;
 }
 
@@ -1003,12 +1159,7 @@ record_alias_subset (alias_set_type supe
     {
       /* Create an entry for the SUPERSET, so that we have a place to
 	 attach the SUBSET.  */
-      superset_entry = ggc_cleared_alloc<alias_set_entry_d> ();
-      superset_entry->alias_set = superset;
-      superset_entry->children
-	= hash_map<int, int, alias_set_traits>::create_ggc (64);
-      superset_entry->has_zero_child = 0;
-      (*alias_sets)[superset] = superset_entry;
+      superset_entry = init_alias_set_entry (superset);
     }
 
   if (subset == 0)
@@ -1016,17 +1167,25 @@ record_alias_subset (alias_set_type supe
   else
     {
       subset_entry = get_alias_set_entry (subset);
+      if (!superset_entry->children)
+	superset_entry->children
+	  = hash_map<int, int, alias_set_traits>::create_ggc (64);
       /* If there is an entry for the subset, enter all of its children
 	 (if they are not already present) as children of the SUPERSET.  */
       if (subset_entry)
 	{
 	  if (subset_entry->has_zero_child)
-	    superset_entry->has_zero_child = 1;
+	    superset_entry->has_zero_child = true;
+          if (subset_entry->has_pointer)
+	    superset_entry->has_pointer = true;
 
-	  hash_map<int, int, alias_set_traits>::iterator iter
-	    = subset_entry->children->begin ();
-	  for (; iter != subset_entry->children->end (); ++iter)
-	    superset_entry->children->put ((*iter).first, (*iter).second);
+	  if (subset_entry->children)
+	    {
+	      hash_map<int, int, alias_set_traits>::iterator iter
+		= subset_entry->children->begin ();
+	      for (; iter != subset_entry->children->end (); ++iter)
+		superset_entry->children->put ((*iter).first, (*iter).second);
+	    }
 	}
 
       /* Enter the SUBSET itself as a child of the SUPERSET.  */
@@ -3086,13 +3245,15 @@ dump_alias_stats_in_alias_c (FILE *s)
 	      "               %llu queries asked about the same object\n"
 	      "               %llu queries asked about the same alias set\n"
 	      "               %llu access volatile\n"
-	      "               %llu are dependent in the DAG\n",
+	      "               %llu are dependent in the DAG\n"
+	      "               %llu are aritificially in conflict with void *\n",
 	   alias_stats.num_disambiguated,
 	   alias_stats.num_alias_zero + alias_stats.num_same_alias_set
 	   + alias_stats.num_same_objects + alias_stats.num_volatile
-	   + alias_stats.num_dag,
+	   + alias_stats.num_dag + alias_stats.num_disambiguated
+	   + alias_stats.num_universal,
 	   alias_stats.num_alias_zero, alias_stats.num_same_alias_set,
-	   + alias_stats.num_same_objects, alias_stats.num_volatile,
-	   + alias_stats.num_dag);
+	   alias_stats.num_same_objects, alias_stats.num_volatile,
+	   alias_stats.num_dag, alias_stats.num_universal);
 }
 #include "gt-alias.h"
Index: testsuite/gcc.dg/alias-14.c
===================================================================
--- testsuite/gcc.dg/alias-14.c	(revision 0)
+++ testsuite/gcc.dg/alias-14.c	(working copy)
@@ -0,0 +1,70 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+#include <stddef.h>
+void *a;
+int *b;
+struct c {void * a;} c;
+struct d {short * a;} d;
+
+int *ip= (int *)(size_t)2;
+int **ipp = &ip;
+
+int
+main()
+{
+  float **ptr;
+  void **uptr;
+  int* const* cipp = (int* const*)ipp;
+  /* as an extension we consider void * universal. Writes to it should alias.  */
+  asm ("":"=r"(ptr):"0"(&a));
+  a=NULL;
+  *ptr=(float*)(size_t)1;
+  if (!a)
+    __builtin_abort ();
+  a=NULL;
+  if (*ptr)
+    __builtin_abort ();
+
+  asm ("":"=r"(uptr):"0"(&b));
+  b=NULL;
+  *uptr=(void*)(size_t)1;
+  if (!b)
+    __builtin_abort ();
+  b=NULL;
+  if (*uptr)
+    __builtin_abort ();
+
+  /* Check that we disambiguate int * and char *.  */
+  asm ("":"=r"(ptr):"0"(&b));
+  b=NULL;
+  *ptr=(float*)(size_t)1;
+  if (b)
+    __builtin_abort ();
+
+  /* Again we should make void * in the structure conflict with any pointer.  */
+  asm ("":"=r"(ptr):"0"(&c));
+  c.a=NULL;
+  *ptr=(float*)(size_t)1;
+  if (!c.a)
+    __builtin_abort ();
+  c.a=NULL;
+  if (*ptr)
+    __builtin_abort ();
+
+  asm ("":"=r"(uptr):"0"(&d));
+  d.a=NULL;
+  *uptr=(void*)(size_t)1;
+  if (!d.a)
+    __builtin_abort ();
+  d.a=NULL;
+  if (*uptr)
+    __builtin_abort ();
+
+  if ((void *)*cipp != (void*)(size_t)2)
+    __builtin_abort ();
+  *ipp = NULL;
+  if (*cipp)
+    __builtin_abort ();
+
+  return 0;
+}
Index: testsuite/gcc.dg/alias-8.c
===================================================================
--- testsuite/gcc.dg/alias-8.c	(revision 223772)
+++ testsuite/gcc.dg/alias-8.c	(working copy)
@@ -8,5 +8,5 @@ struct s {
 void
 func(struct s *ptr)
 {
-  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { xfail *-*-* } } */
+  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { } } */
 }
Index: testsuite/gcc.dg/pr62167.c
===================================================================
--- testsuite/gcc.dg/pr62167.c	(revision 223772)
+++ testsuite/gcc.dg/pr62167.c	(working copy)
@@ -29,6 +29,8 @@ main ()
 
   node.prev = (void *)head;
 
+  asm("":"=m"(node.prev));
+
   head->first = &node;
 
   struct node *n = head->first;

Re: Do less generous pointer globbing in alias.c

[H.J. Lu]

On Thu, May 28, 2015 at 1:12 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> hello,
> only providing you the testcase why I need transitive closure of "contains
> pointer" via the extra child I noticed that there is extra symmetry to handle:
>
>      struct a {void *ptr;}
>      char **ptr = (char **)&a.ptr;
>      ptr = ...
>
> This one doesn't really fly with my extra subset code, because ptr is not
> universal pointer, but struct a contains one and thus should conflict with
> every pointer.  Adding every pointer as subset of every structure with
> universal pointer is impractical (childs of those structures would be appearing
> as new pointer types get alias sets) and thus indeed it is better to handle it
> same way as alias set 0 - by a special case in alias_set_subset_of
> and alias_sets_conflict_p.
>
> So I added the second flag - has_pointer that is transitive closure of
> is_pointer and added the special case to alias_sets_conflict_p instead of
> adding the extra subset relation into the DAG.
>
> I also added statistics and made changes you suggested (making child
> hash to be possibly NULL and clenaing up alias set conflict construction)
>
> I also constructed a testcase that covers all the new code paths.
>
> The patch bootstrapped/regtested ppc64-linux.  I am not bound to teaching
> next week, so if I hear no negative comments, I will schedule commiting the
> patch for weekend to deal with possible fallout.
>
> There are few cleanups possible incrementally - i.e. the hash set seems
> irrationaly large for average type, we could avoid some pointer travelling
> overhead and we could also do better at alias_sets_must_conflict_p.
>
> Honza
>
>         * alias.c (alias_set_entry_d): Add is_pointer and has_pointer.
>         (alias_stats): Add num_universal.
>         (alias_set_subset_of): Special case pointers; be ready for NULL
>         children.
>         (alias_sets_conflict_p): Special case pointers; be ready for NULL
>         children.
>         (init_alias_set_entry): Break out from ...
>         (record_alias_subset): ... here; propagate new fields;
>         allocate children only when really needed.
>         (get_alias_set): Do less generous pointer globbing.
>         (dump_alias_stats_in_alias_c): Update statistics.
>         * gcc.dg/alias-8.c: Do not xfail.
>         * gcc.dg/pr62167.c: Prevent FRE.
>         * gcc.dg/alias-14.c: New testcase.
==========================================
> --- testsuite/gcc.dg/alias-8.c  (revision 223772)
> +++ testsuite/gcc.dg/alias-8.c  (working copy)
> @@ -8,5 +8,5 @@ struct s {
>  void
>  func(struct s *ptr)
>  {
> -  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { xfail *-*-* } } */
> +  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { } } */
>  }

This caused:

ERROR: gcc.dg/alias-8.c: syntax error in target selector "" for "
dg-warning 11 "type-punned pointer" "" { } "

I checked in this fix.

H.J.
---
Index: ChangeLog
===================================================================
--- ChangeLog (revision 223886)
+++ ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2015-05-30  H.J. Lu  <hongjiu.lu@intel.com>
+
+ * gcc.dg/alias-8.c: Fix dg-warning.
+
 2015-05-30  Jan Hubicka  <hubicka@ucw.cz>

  * gcc.dg/alias-8.c: Do not xfail.
Index: gcc.dg/alias-8.c
===================================================================
--- gcc.dg/alias-8.c (revision 223886)
+++ gcc.dg/alias-8.c (working copy)
@@ -8,5 +8,5 @@
 void
 func(struct s *ptr)
 {
-  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { } } */
+  *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" } */
 }

Re: Do less generous pointer globbing in alias.c

[Andreas Schwab]

Jan Hubicka <hubicka@ucw.cz> writes:

> 	* alias.c (alias_set_entry_d): Add is_pointer and has_pointer.
> 	(alias_stats): Add num_universal.
> 	(alias_set_subset_of): Special case pointers; be ready for NULL
> 	children.
> 	(alias_sets_conflict_p): Special case pointers; be ready for NULL
> 	children.
> 	(init_alias_set_entry): Break out from ...
> 	(record_alias_subset): ... here; propagate new fields;
> 	allocate children only when really needed.
> 	(get_alias_set): Do less generous pointer globbing.
> 	(dump_alias_stats_in_alias_c): Update statistics.
> 	* gcc.dg/alias-8.c: Do not xfail.
> 	* gcc.dg/pr62167.c: Prevent FRE.
> 	* gcc.dg/alias-14.c: New testcase.

This is causing a miscompilation of the stage2 compiler on ia64.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Do less generous pointer globbing in alias.c

[Jan Hubicka]

> Jan Hubicka <hubicka@ucw.cz> writes:
> 
> > 	* alias.c (alias_set_entry_d): Add is_pointer and has_pointer.
> > 	(alias_stats): Add num_universal.
> > 	(alias_set_subset_of): Special case pointers; be ready for NULL
> > 	children.
> > 	(alias_sets_conflict_p): Special case pointers; be ready for NULL
> > 	children.
> > 	(init_alias_set_entry): Break out from ...
> > 	(record_alias_subset): ... here; propagate new fields;
> > 	allocate children only when really needed.
> > 	(get_alias_set): Do less generous pointer globbing.
> > 	(dump_alias_stats_in_alias_c): Update statistics.
> > 	* gcc.dg/alias-8.c: Do not xfail.
> > 	* gcc.dg/pr62167.c: Prevent FRE.
> > 	* gcc.dg/alias-14.c: New testcase.
> 
> This is causing a miscompilation of the stage2 compiler on ia64.

Hmm, lovely :( 
According to GCC compile farm wiki, GCC 60 and 66 are ia64 but they are not.
I am not sure I have access to working ia64 box.  Can you possibly help me
to debug this?  Is it devirtualization related?
With earlier versions of the patch I run into issue of ipa-icf ICEing in 
sem_function::parse because ipa_polymorphic_call_context::get_dynamic_type
missed initialization of VPTR.  I pushed out just partial fix to the issue
as I want to test it on Firefox first and I am running into intresting
issues with Firefox LTO right now (unrelated to the aliasing).
With some luck it is the same problem because IA-64 has different representation
of function pointers.

Honza
> 
> Andreas.
> 
> -- 
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."

Re: Do less generous pointer globbing in alias.c

[Andreas Schwab]

Jan Hubicka <hubicka@ucw.cz> writes:

> I am not sure I have access to working ia64 box.  Can you possibly help me
> to debug this?  Is it devirtualization related?

Here's a backtrace:

0x4000000000422311 in bitmap_obstack_alloc_stat (
    bit_obstack=0x400000000194cf20 <ipa_icf::sem_item_optimizer::parse_funcs_and_vars()+1312>) at ../../gcc/bitmap.c:288
288         bit_obstack->heads = (struct bitmap_head *) map->first;
(gdb) bt full
#0  0x4000000000422311 in bitmap_obstack_alloc_stat (
    bit_obstack=0x400000000194cf20 <ipa_icf::sem_item_optimizer::parse_funcs_and_vars()+1312>) at ../../gcc/bitmap.c:288
        map = 0x8401880020420020
#1  0x400000000192bf10 in ipa_icf::sem_item::setup (this=0x2000000000ab0350, 
    stack=0x400000000194cf20 <ipa_icf::sem_item_optimizer::parse_funcs_and_vars()+1312>) at ../../gcc/ipa-icf.c:224
No locals.
#2  0x400000000194c730 in ipa_icf::sem_variable::sem_variable (
    this=0x2000000000ab0350, node=0x6000000000309430, _hash=3188864, 
    stack=0x400000000194cf20 <ipa_icf::sem_item_optimizer::parse_funcs_and_vars()+1312>) at ../../gcc/ipa-icf.c:1847
No locals.
#3  0x400000000194c690 in sem_function (stack=0x600000000008c4d8 <dump_flags>, 
    hash=0, node=0x7fffffff, this=0x2000000000966920)
    at ../../gcc/ipa-icf.c:286
No locals.
#4  ipa_icf::sem_function::parse (node=0x7fffffff, 
    stack=0x600000000008c4d8 <dump_flags>) at ../../gcc/ipa-icf.c:1701
        fndecl = <optimized out>
        func = <optimized out>
        __FUNCTION__ = "parse"
#5  0x4000000000d24110 in execute_ipa_summary_passes (
    ipa_pass=0x2000000000ab0350) at ../../gcc/passes.c:2143
        pass = 0x2000000000ab0350
#6  0x400000000194eb00 in ipa_icf::ipa_icf_generate_summary ()
    at ../../gcc/ipa-icf.c:3502
No locals.
#7  0x60000000001230b0 in default_target_ira_int ()
No symbol table info available.
#8  0x400000000192bf10 in ipa_icf::sem_item::setup (this=0xffffffffffffffc0, 
    stack=0x0) at ../../gcc/ipa-icf.c:224
No locals.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Do less generous pointer globbing in alias.c

[Andreas Schwab]

The problem is that ipa_icf::sem_function::parse is lacking the f->init
call and the function epilog, falling through to the next function that
happens to follow.

Revision r223897:

Dump of assembler code for function ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*):
   0x400000000194c480 <+0>:     [MMI]       alloc r36=ar.pfs,12,6,0
   0x400000000194c481 <+1>:                 adds r14=16,r32
   0x400000000194c482 <+2>:                 mov r35=b0
   0x400000000194c490 <+16>:    [MMI]       mov r37=r1;;
   0x400000000194c491 <+17>:                ld8 r38=[r14]
   0x400000000194c492 <+18>:                nop.i 0x0;;
   0x400000000194c4a0 <+32>:    [MMI]       ld2 r14=[r38];;
   0x400000000194c4a1 <+33>:                cmp4.eq p6,p7=32,r14
   0x400000000194c4a2 <+34>:                nop.i 0x0;;
   0x400000000194c4b0 <+48>:    [MMI] (p07) addl r39=-833288,r1
   0x400000000194c4b1 <+49>:          (p07) mov r43=r0
   0x400000000194c4b2 <+50>:                nop.i 0x0
   0x400000000194c4c0 <+64>:    [MMI] (p07) mov r42=32
   0x400000000194c4c1 <+65>:          (p07) mov r40=1693
   0x400000000194c4c2 <+66>:          (p07) addl r41=-628512,r1;;
   0x400000000194c4d0 <+80>:    [MIB] (p07) ld8 r39=[r39]
   0x400000000194c4d1 <+81>:                nop.i 0x0
   0x400000000194c4d2 <+82>:          (p07) br.call.spnt.many b0=0x4000000001530200 <tree_check_failed(tree_node const*, char const*, int, char const*, ...)>;;
   0x400000000194c4e0 <+96>:    [MMI]       adds r14=152,r38;;
   0x400000000194c4e1 <+97>:                ld8 r14=[r14]
   0x400000000194c4e2 <+98>:                nop.i 0x0;;
   0x400000000194c4f0 <+112>:   [MIB]       cmp.eq p6,p7=0,r14
   0x400000000194c4f1 <+113>:               nop.i 0x0
   0x400000000194c4f2 <+114>:         (p06) br.cond.dpnt.few 0x400000000194c5f0 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+368>;;
   0x400000000194c500 <+128>:   [MMI]       nop.m 0x0
   0x400000000194c501 <+129>:               ld8 r14=[r32]
   0x400000000194c502 <+130>:               nop.i 0x0;;
   0x400000000194c510 <+144>:   [MIB]       nop.m 0x0
   0x400000000194c511 <+145>:               tbit.z p6,p7=r14,16
   0x400000000194c512 <+146>:         (p06) br.cond.dptk.few 0x400000000194c610 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+400>
   0x400000000194c520 <+160>:   [MMI]       adds r15=387,r32;;
   0x400000000194c521 <+161>:               ld1 r15=[r15]
   0x400000000194c522 <+162>:               nop.i 0x0;;
   0x400000000194c530 <+176>:   [MIB]       nop.m 0x0
   0x400000000194c531 <+177>:               cmp4.eq p7,p6=0,r15
   0x400000000194c532 <+178>:         (p06) br.cond.dptk.few 0x400000000194c550 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+208>;;
   0x400000000194c540 <+192>:   [MIB]       nop.m 0x0
   0x400000000194c541 <+193>:               tbit.z p7,p6=r14,17
   0x400000000194c542 <+194>:         (p06) br.cond.dptk.few 0x400000000194c5f0 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+368>
   0x400000000194c550 <+208>:   [MMI]       addl r14=-911832,r1;;
   0x400000000194c551 <+209>:               ld8 r14=[r14]
   0x400000000194c552 <+210>:               nop.i 0x0;;
   0x400000000194c560 <+224>:   [MMI]       adds r14=2059,r14;;
   0x400000000194c561 <+225>:               ld1 r14=[r14]
   0x400000000194c562 <+226>:               nop.i 0x0;;
   0x400000000194c570 <+240>:   [MII]       cmp4.eq p6,p7=1,r14
   0x400000000194c571 <+241>:               nop.i 0x0;;
   0x400000000194c572 <+242>:         (p07) addl r40=-833288,r1
   0x400000000194c580 <+256>:   [MMI] (p07) addl r42=-628512,r1
   0x400000000194c581 <+257>:         (p07) mov r41=1698
   0x400000000194c582 <+258>:         (p07) mov r39=11;;
   0x400000000194c590 <+272>:   [MIB] (p07) ld8 r40=[r40]
   0x400000000194c591 <+273>:               nop.i 0x0
   0x400000000194c592 <+274>:         (p07) br.call.spnt.many b0=0x4000000001532200 <tree_contains_struct_check_failed(tree_node const*, tree_node_structure_enum, char const*, int, char const*)>;;
   0x400000000194c5a0 <+288>:   [MMI]       adds r38=88,r38
   0x400000000194c5a1 <+289>:               nop.m 0x0
   0x400000000194c5a2 <+290>:               mov r39=4;;
   0x400000000194c5b0 <+304>:   [MMI]       ld8 r40=[r38]
   0x400000000194c5b1 <+305>:               nop.m 0x0
   0x400000000194c5b2 <+306>:               addl r38=-832208,r1;;
   0x400000000194c5c0 <+320>:   [MIB]       cmp.eq p7,p6=0,r40
   0x400000000194c5c1 <+321>:               nop.i 0x0
   0x400000000194c5c2 <+322>:         (p07) br.cond.dpnt.few 0x400000000194c650 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+464>;;
   0x400000000194c5d0 <+336>:   [MIB]       ld8 r38=[r38]
   0x400000000194c5d1 <+337>:               nop.i 0x0
   0x400000000194c5d2 <+338>:               br.call.sptk.many b0=0x4000000001534f40 <private_lookup_attribute_by_prefix(char const*, unsigned long, tree_node*)>;;
   0x400000000194c5e0 <+352>:   [MIB]       mov r1=r37
   0x400000000194c5e1 <+353>:               cmp.eq p6,p7=0,r8
   0x400000000194c5e2 <+354>:         (p06) br.cond.spnt.few 0x400000000194c650 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+464>
   0x400000000194c5f0 <+368>:   [MMI]       mov r8=r0
   0x400000000194c5f1 <+369>:               nop.m 0x0
   0x400000000194c5f2 <+370>:               mov.i ar.pfs=r36;;
   0x400000000194c600 <+384>:   [MIB]       nop.m 0x0
   0x400000000194c601 <+385>:               mov b0=r35
   0x400000000194c602 <+386>:               br.ret.sptk.many b0
   0x400000000194c610 <+400>:   [MMI]       adds r14=387,r32;;
   0x400000000194c611 <+401>:               ld1 r14=[r14]
   0x400000000194c612 <+402>:               nop.i 0x0;;
   0x400000000194c620 <+416>:   [MIB]       cmp4.eq p7,p6=0,r14
   0x400000000194c621 <+417>:               nop.i 0x0
   0x400000000194c622 <+418>:         (p06) br.cond.dpnt.few 0x400000000194c550 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+208>;;
   0x400000000194c630 <+432>:   [MMI]       mov r8=r0
   0x400000000194c631 <+433>:               nop.m 0x0
   0x400000000194c632 <+434>:               mov.i ar.pfs=r36;;
   0x400000000194c640 <+448>:   [MIB]       nop.m 0x0
   0x400000000194c641 <+449>:               mov b0=r35
   0x400000000194c642 <+450>:               br.ret.sptk.many b0;;
   0x400000000194c650 <+464>:   [MIB]       mov r38=216
   0x400000000194c651 <+465>:               nop.i 0x0
   0x400000000194c652 <+466>:               br.call.sptk.many b0=0x4000000001b83200 <operator new(unsigned long)>;;
   0x400000000194c660 <+480>:   [MMI]       mov r1=r37
   0x400000000194c661 <+481>:               mov r34=r8
   0x400000000194c662 <+482>:               mov r38=r8
   0x400000000194c670 <+496>:   [MMI]       mov r42=r33
   0x400000000194c671 <+497>:               mov r41=r0
   0x400000000194c672 <+498>:               mov r40=r32;;
   0x400000000194c680 <+512>:   [MIB]       mov r39=r0
   0x400000000194c681 <+513>:               nop.i 0x0
   0x400000000194c682 <+514>:               br.call.sptk.many b0=0x400000000194c2c0 <ipa_icf::sem_item::sem_item(ipa_icf::sem_item_type, symtab_node*, unsigned int, bitmap_obstack*)>;;
   0x400000000194c690 <+528>:   [MMI]       mov r1=r37
   0x400000000194c691 <+529>:               mov r8=r34
   0x400000000194c692 <+530>:               adds r17=208,r34
   0x400000000194c6a0 <+544>:   [MMI]       adds r16=152,r34
   0x400000000194c6a1 <+545>:               adds r15=168,r34
   0x400000000194c6a2 <+546>:               adds r14=192,r34;;
   0x400000000194c6b0 <+560>:   [MMI]       addl r18=-628624,r1
   0x400000000194c6b1 <+561>:               st8 [r17]=r0
   0x400000000194c6b2 <+562>:               nop.i 0x0
   0x400000000194c6c0 <+576>:   [MMI]       st8 [r16]=r0;;
   0x400000000194c6c1 <+577>:               ld8 r18=[r18]
   0x400000000194c6c2 <+578>:               nop.i 0x0
   0x400000000194c6d0 <+592>:   [MMI]       st8 [r15]=r0
   0x400000000194c6d1 <+593>:               st8 [r14]=r0
   0x400000000194c6d2 <+594>:               nop.i 0x0;;
   0x400000000194c6e0 <+608>:   [MMI]       nop.m 0x0
   0x400000000194c6e1 <+609>:               st8 [r8]=r18,200
   0x400000000194c6e2 <+610>:               nop.i 0x0;;
   0x400000000194c6f0 <+624>:   [MMI]       st8 [r8]=r0
   0x400000000194c6f1 <+625>:               nop.m 0x0
   0x400000000194c6f2 <+626>:               nop.i 0x0;;

Revision r223856:

Dump of assembler code for function ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*):
   0x400000000154fe00 <+0>:     [MMI]       alloc r36=ar.pfs,11,6,0
   0x400000000154fe01 <+1>:                 adds r14=16,r32
   0x400000000154fe02 <+2>:                 mov r35=b0
   0x400000000154fe10 <+16>:    [MMI]       mov r37=r1;;
   0x400000000154fe11 <+17>:                ld8 r14=[r14]
   0x400000000154fe12 <+18>:                nop.i 0x0;;
   0x400000000154fe20 <+32>:    [MMI]       adds r15=152,r14;;
   0x400000000154fe21 <+33>:                ld8 r15=[r15]
   0x400000000154fe22 <+34>:                nop.i 0x0;;
   0x400000000154fe30 <+48>:    [MIB]       cmp.eq p7,p6=0,r15
   0x400000000154fe31 <+49>:                nop.i 0x0
   0x400000000154fe32 <+50>:          (p07) br.cond.dpnt.few 0x400000000154fee0 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+224>;;
   0x400000000154fe40 <+64>:    [MMI]       nop.m 0x0
   0x400000000154fe41 <+65>:                ld8 r15=[r32]
   0x400000000154fe42 <+66>:                nop.i 0x0;;
   0x400000000154fe50 <+80>:    [MIB]       nop.m 0x0
   0x400000000154fe51 <+81>:                tbit.z p6,p7=r15,16
   0x400000000154fe52 <+82>:          (p06) br.cond.dptk.few 0x400000000154ff10 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+272>
   0x400000000154fe60 <+96>:    [MMI]       adds r16=387,r32;;
   0x400000000154fe61 <+97>:                ld1 r16=[r16]
   0x400000000154fe62 <+98>:                nop.i 0x0;;
   0x400000000154fe70 <+112>:   [MIB]       nop.m 0x0
   0x400000000154fe71 <+113>:               cmp4.eq p7,p6=0,r16
   0x400000000154fe72 <+114>:         (p06) br.cond.dptk.few 0x400000000154fe90 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+144>;;
   0x400000000154fe80 <+128>:   [MIB]       nop.m 0x0
   0x400000000154fe81 <+129>:               tbit.z p7,p6=r15,17
   0x400000000154fe82 <+130>:         (p06) br.cond.dptk.few 0x400000000154fee0 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+224>
   0x400000000154fe90 <+144>:   [MMI]       adds r14=88,r14
   0x400000000154fe91 <+145>:               addl r38=-855468,r1
   0x400000000154fe92 <+146>:               mov r39=4;;
   0x400000000154fea0 <+160>:   [MMI]       nop.m 0x0
   0x400000000154fea1 <+161>:               ld8 r40=[r14]
   0x400000000154fea2 <+162>:               nop.i 0x0;;
   0x400000000154feb0 <+176>:   [MIB]       cmp.eq p7,p6=0,r40
   0x400000000154feb1 <+177>:               nop.i 0x0
   0x400000000154feb2 <+178>:         (p07) br.cond.dpnt.few 0x400000000154ff40 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+320>;;
   0x400000000154fec0 <+192>:   [MIB]       ld8 r38=[r38]
   0x400000000154fec1 <+193>:               nop.i 0x0
   0x400000000154fec2 <+194>:               br.call.sptk.many b0=0x40000000011aa700 <private_lookup_attribute_by_prefix(char const*, unsigned long, tree_node*)>;;
   0x400000000154fed0 <+208>:   [MIB]       mov r1=r37
   0x400000000154fed1 <+209>:               cmp.eq p7,p6=0,r8
   0x400000000154fed2 <+210>:         (p07) br.cond.dpnt.few 0x400000000154ff40 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+320>
   0x400000000154fee0 <+224>:   [MMI]       mov r8=r0
   0x400000000154fee1 <+225>:               nop.m 0x0
   0x400000000154fee2 <+226>:               nop.i 0x0
   0x400000000154fef0 <+240>:   [MII]       nop.m 0x0
   0x400000000154fef1 <+241>:               mov.i ar.pfs=r36
   0x400000000154fef2 <+242>:               nop.i 0x0;;
   0x400000000154ff00 <+256>:   [MIB]       nop.m 0x0
   0x400000000154ff01 <+257>:               mov b0=r35
   0x400000000154ff02 <+258>:               br.ret.sptk.many b0
   0x400000000154ff10 <+272>:   [MMI]       adds r15=387,r32
   0x400000000154ff11 <+273>:               nop.m 0x0
   0x400000000154ff12 <+274>:               mov r8=r0;;
   0x400000000154ff20 <+288>:   [MMI]       nop.m 0x0
   0x400000000154ff21 <+289>:               ld1 r15=[r15]
   0x400000000154ff22 <+290>:               nop.i 0x0;;
   0x400000000154ff30 <+304>:   [MBB]       cmp4.eq p7,p6=0,r15
   0x400000000154ff31 <+305>:         (p06) br.cond.dpnt.few 0x400000000154fe90 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+144>
   0x400000000154ff32 <+306>:               br.few 0x400000000154fef0 <ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+240>
   0x400000000154ff40 <+320>:   [MIB]       mov r38=216
   0x400000000154ff41 <+321>:               nop.i 0x0
   0x400000000154ff42 <+322>:               br.call.sptk.many b0=0x40000000017531c0 <operator new(unsigned long)>;;
   0x400000000154ff50 <+336>:   [MMI]       mov r1=r37
   0x400000000154ff51 <+337>:               mov r34=r8
   0x400000000154ff52 <+338>:               mov r38=r8
   0x400000000154ff60 <+352>:   [MMI]       mov r42=r33
   0x400000000154ff61 <+353>:               mov r41=r0
   0x400000000154ff62 <+354>:               mov r40=r32;;
   0x400000000154ff70 <+368>:   [MIB]       mov r39=r0
   0x400000000154ff71 <+369>:               nop.i 0x0
   0x400000000154ff72 <+370>:               br.call.sptk.many b0=0x400000000154fc40 <ipa_icf::sem_item::sem_item(ipa_icf::sem_item_type, symtab_node*, unsigned int, bitmap_obstack*)>;;
   0x400000000154ff80 <+384>:   [MMI]       mov r1=r37
   0x400000000154ff81 <+385>:               mov r14=r34
   0x400000000154ff82 <+386>:               adds r18=208,r34
   0x400000000154ff90 <+400>:   [MMI]       adds r17=152,r34
   0x400000000154ff91 <+401>:               adds r16=168,r34
   0x400000000154ff92 <+402>:               adds r15=192,r34;;
   0x400000000154ffa0 <+416>:   [MMI]       addl r19=-672196,r1
   0x400000000154ffa1 <+417>:               nop.m 0x0
   0x400000000154ffa2 <+418>:               mov r38=r34
   0x400000000154ffb0 <+432>:   [MMI]       st8 [r18]=r0
   0x400000000154ffb1 <+433>:               st8 [r17]=r0
   0x400000000154ffb2 <+434>:               nop.i 0x0;;
   0x400000000154ffc0 <+448>:   [MMI]       ld8 r19=[r19]
   0x400000000154ffc1 <+449>:               st8 [r16]=r0
   0x400000000154ffc2 <+450>:               nop.i 0x0
   0x400000000154ffd0 <+464>:   [MMI]       st8 [r15]=r0;;
   0x400000000154ffd1 <+465>:               st8 [r14]=r19,200
   0x400000000154ffd2 <+466>:               nop.i 0x0;;
   0x400000000154ffe0 <+480>:   [MIB]       st8 [r14]=r0
   0x400000000154ffe1 <+481>:               nop.i 0x0
   0x400000000154ffe2 <+482>:               br.call.sptk.many b0=0x4000000001546f00 <ipa_icf::sem_function::init()>;;
   0x400000000154fff0 <+496>:   [MMI]       mov r8=r34
   0x400000000154fff1 <+497>:               mov r1=r37
   0x400000000154fff2 <+498>:               mov.i ar.pfs=r36;;
   0x4000000001550000 <+512>:   [MIB]       nop.m 0x0
   0x4000000001550001 <+513>:               mov b0=r35
   0x4000000001550002 <+514>:               br.ret.sptk.many b0;;

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Do less generous pointer globbing in alias.c

[Jan Hubicka]

> The problem is that ipa_icf::sem_function::parse is lacking the f->init
> call and the function epilog, falling through to the next function that
> happens to follow.

Thank you, that is indeed the devirtualization issue - I suppose the function
descriptors confuses the code even more.  I will finish the fix for that tonight.

Honza

Re: Do less generous pointer globbing in alias.c

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 11034 bytes --]

On 05/27/2015 07:28 AM, Jan Hubicka wrote:
> Hi,
> this patch makes it possible for non-LTO alias oracle to TBAA disambiguate
> pointer types. It makes void * conflicting with all of them and does not put it
> to alias set 0. It also preserves the property that qualifiers of pointer-to
> type should not matter to determine the alias set and that pointer to array is
> same as pointer to array element.  Finally it makes pointer void * to be
> equivalent to void ** (and more *) and to types with structural equality only.
> 
> I think those are all globbing rules we discussed for the non-LTO patch.
> 
> It does two things.  First is kind of "canonicalization" where for a given pointer
> it looks for non-pointer pointed-to type and then rebuilds is without qualifiers.
> This is fast, because build_pointer_type will reuse existing types.
> 
> It makes void * to conflict with everyting by making its alias set to be subset
> of alias set of any other pointer.  This means that writes to void * conflict
> with writes to any other pointer without really need to glob all the pointers
> to one equivalence class.
> 
> This patch makes quite some difference on C++.  For example on deal II the TBAA
> stats reports 4344358 disambiguations and 7008576 queries, while with the patch
> we get 5368737 and 5687399 queries (I did not chose deal II for reason, it is
> just random C++ file)
> 
> The patch bootstrap and regtests ppc64le-linux with the following testsuite
> differences:
> @@ -30,7 +30,9 @@
>  FAIL: c-c++-common/asan/null-deref-1.c   -O3 -g  output pattern test, is ASAN:SIGSEGV
>  FAIL: c-c++-common/asan/null-deref-1.c   -Os  output pattern test, is ASAN:SIGSEGV
>  FAIL: gcc.dg/cpp/_Pragma3.c (test for excess errors)
> +XPASS: gcc.dg/alias-8.c  (test for warnings, line 11)
>  FAIL: gcc.dg/loop-8.c scan-rtl-dump-times loop2_invariant "Decided" 1
> +FAIL: gcc.dg/pr62167.c scan-tree-dump-not pre "Removing basic block"
>  FAIL: gcc.dg/sms-4.c scan-rtl-dump-times sms "SMS succeeded" 1
>  XPASS: gcc.dg/guality/example.c   -O0  execution test
>  XPASS: gcc.dg/guality/example.c   -O1  execution test
> @@ -304,6 +306,9 @@
>  FAIL: c-c++-common/asan/null-deref-1.c   -O3 -g  output pattern test, is ASAN:SIGSEGV
>  FAIL: g++.dg/cpp1y/vla-initlist1.C  -std=gnu++11 execution test
>  FAIL: g++.dg/cpp1y/vla-initlist1.C  -std=gnu++14 execution test
> +FAIL: g++.dg/ipa/ipa-icf-4.C  -std=gnu++11  scan-ipa-dump icf "Equal symbols: [67]"
> +FAIL: g++.dg/ipa/ipa-icf-4.C  -std=gnu++14  scan-ipa-dump icf "Equal symbols: [67]"
> +FAIL: g++.dg/ipa/ipa-icf-4.C  -std=gnu++98  scan-ipa-dump icf "Equal symbols: [67]"
> 
> ipa-icf-4 is about alias info now being more perceptive to block the merging.
> pr62167 seems just confused.  The template checks that memory stores are not
> unified.  It looks for BB removal message, but with the patch we get:
>   <bb 2>:
>   node.next = 0B;
>   head.0_4 = head;
>   node.prev = head.0_4;
>   head.0_4->first = &node;
>   k.1_7 = k;
>   h_8 = &heads[k.1_7];
>   heads[2].first = 0B;
>   if (head.0_4 == h_8)
>     goto <bb 3>;
>   else
>     goto <bb 5>;
> 
>   <bb 5>:
>   goto <bb 4>;
> 
>   <bb 3>:
>   p_10 = MEM[(struct head *)&heads][k.1_7].first;
> 
>   <bb 4>:
>   # p_1 = PHI <p_10(3), &node(5)>
>   _11 = p_1 != 0B;
>   _12 = (int) _11;
>   return _12;
> 
> before PR, the message is about the bb 5 sitting at critical edge removed.
> The TBAA incompatible load it looks for is optimized away by FRE:
>   head->first = &node;
> 
>   struct node *n = head->first;
> 
>   struct head *h = &heads[k];
> 
>   heads[2].first = n->next;
> 
>   if ((void*)n->prev == (void *)h)
>     p = h->first;
>   else
>     /* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next.  */
>     p = n->prev->next;
> 
> here n is known to be head->first that is known to be &node.
> The testcase runtime checks that result is Ok and passes.
> 
> Bootstrapped/regtested ppc64le-linux.
> 
> 	* alias.c (get_alias_set): Do not glob all pointer types into one;
> 	just produce euqivalence classes based on canonical type of pointed
> 	type type; make void * equivalent to void **.
> 	(record_component_aliases): Make void * to conflict with all other
> 	pointer types.
> Index: alias.c
> ===================================================================
> --- alias.c	(revision 223633)
> +++ alias.c	(working copy)
> @@ -903,35 +906,79 @@ get_alias_set (tree t)
>       the pointed-to types.  This issue has been reported to the
>       C++ committee.
>  
> -     In addition to the above canonicalization issue, with LTO
> -     we should also canonicalize `T (*)[]' to `T *' avoiding
> -     alias issues with pointer-to element types and pointer-to
> -     array types.
> -
> -     Likewise we need to deal with the situation of incomplete
> -     pointed-to types and make `*(struct X **)&a' and
> -     `*(struct X {} **)&a' alias.  Otherwise we will have to
> -     guarantee that all pointer-to incomplete type variants
> -     will be replaced by pointer-to complete type variants if
> -     they are available.
> -
> -     With LTO the convenient situation of using `void *' to
> -     access and store any pointer type will also become
> -     more apparent (and `void *' is just another pointer-to
> -     incomplete type).  Assigning alias-set zero to `void *'
> -     and all pointer-to incomplete types is a not appealing
> -     solution.  Assigning an effective alias-set zero only
> -     affecting pointers might be - by recording proper subset
> -     relationships of all pointer alias-sets.
> -
> -     Pointer-to function types are another grey area which
> -     needs caution.  Globbing them all into one alias-set
> -     or the above effective zero set would work.
> -
> -     For now just assign the same alias-set to all pointers.
> -     That's simple and avoids all the above problems.  */
> -  else if (POINTER_TYPE_P (t)
> -	   && t != ptr_type_node)
> +     For this reason go to canonical type of the unqalified pointer type.
> +     Until GCC 6 this code set all pointers sets to have alias set of
> +     ptr_type_node but that is a bad idea, because it prevents disabiguations
> +     in between pointers.  For Firefox this accounts about 20% of all
> +     disambiguations in the program.  */
> +  else if (POINTER_TYPE_P (t) && t != ptr_type_node && !in_lto_p)
> +    {
> +      tree p;
> +      auto_vec <bool, 8> reference;
> +
> +      /* Unnest all pointers and references.
> +         We also want to make pointer to array equivalent to pointer to its
> +         element. So skip all array types, too.  */
> +      for (p = t; POINTER_TYPE_P (p)
> +	   || (TREE_CODE (p) == ARRAY_TYPE && !TYPE_NONALIASED_COMPONENT (p));
> +	   p = TREE_TYPE (p))
> +	{
> +	  if (TREE_CODE (p) == REFERENCE_TYPE)
> +	    reference.safe_push (true);
> +	  if (TREE_CODE (p) == POINTER_TYPE)
> +	    reference.safe_push (false);
> +	}
> +      p = TYPE_MAIN_VARIANT (p);
> +
> +      /* 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);
> +      else
> +	{
> +	  /* Rebuild pointer type from starting from canonical types using
> +	     unqualified pointers and references only.  This way all such
> +	     pointers will have the same alias set and will conflict with
> +	     each other.
> +
> +	     Most of time we already have pointers or references of a given type.
> +	     If not build new one just to be sure that if someone later (probably
> +	     only middle-end can, as we should assign all alias classes only after
> +	     finishing translation unit) builds the pointer type, the canonical type
> +	     will match.  */
> +	  p = TYPE_CANONICAL (p);
> +	  while (!reference.is_empty ())
> +	    {
> +	      if (reference.pop ())
> +		p = build_reference_type (p);
> +	      else
> +		p = build_pointer_type (p);
> +	      p = TYPE_CANONICAL (TYPE_MAIN_VARIANT (p));
> +	    }
> +          gcc_checking_assert (TYPE_CANONICAL (p) == p);
> +
> +	  /* Assign the alias set to both p and t.
> +	     We can not call get_alias_set (p) here as that would triger
> +	     infinite recursion when p == t.
> +	     In other cases it would just trigger unnecesary legwork of
> +	     rebuilding the pointer again.  */
> +	  if (TYPE_ALIAS_SET_KNOWN_P (p))
> +	    /* Return early; we do not need to record component aliases.  */
> +	    return TYPE_ALIAS_SET (t) = TYPE_ALIAS_SET (p);
> +	  else
> +	    {
> +	      set = new_alias_set ();
> +	      TYPE_ALIAS_SET (p) = set;
> +	    }
> +	}
> +    }
> +  /* In LTO the rules above needs to be part of canonical type machinery.
> +     For now just punt.  */
> +  else if (POINTER_TYPE_P (t) && t != ptr_type_node && in_lto_p)
>      set = get_alias_set (ptr_type_node);
>  
>    /* Otherwise make a new alias set for this type.  */
> @@ -950,7 +997,8 @@ get_alias_set (tree t)
>  
>    /* If this is an aggregate type or a complex type, we must record any
>       component aliasing information.  */
> -  if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
> +  if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE
> +      || TREE_CODE (t) == POINTER_TYPE)
>      record_component_aliases (t);
>  
>    return set;
> @@ -1050,6 +1098,26 @@ record_component_aliases (tree type)
>  
>    switch (TREE_CODE (type))
>      {
> +    /* We want void * to be compatible with any other pointer without really
> +       dropping it to alias set 0. Doing so would make it compatible with
> +       all non-pointer types too.
> +
> +       Make thus ptr_type_node to be a component of every pointer type.
> +       Tus memory operations of type "void *" conflict with operations of type
> +       "T *" without impossing a conflict with memory operations of type "Q *"
> +       in case T and Q do not conflict.
> + 
> +       This is not strictly necessary by the language standards, but avoids
> +       common type punning mistakes.  In addition to that, we need the existence
> +       of such universal pointer to implement Fortran's C_PTR type (which is
> +       defined as type compatible with all C pointers) and we use it in
> +       get_alias_set to give alias set to pointers to types without
> +       canonical types (those are typically nameless incomplete types)
> +       that needs to be also compatible with each other and with pointers to
> +       complete types.  */
> +    case POINTER_TYPE:
> +      record_alias_subset (superset, get_alias_set (ptr_type_node));
> +      break;
>      case RECORD_TYPE:
>      case UNION_TYPE:
>      case QUAL_UNION_TYPE:
> 

Hi.

If I see correctly, ipa-icf-4.C is still broken, I suggest to degrade the number of merge
operations for the test.

Should I apply the patch?

Thanks,
Martin

[-- Attachment #2: 0001-Fix-IPA-ICF-test-case.patch --]
[-- Type: text/x-patch, Size: 941 bytes --]

From aa836d99a793eceaa72cd5ed24ba20b7c03bd12f Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Wed, 10 Jun 2015 12:38:03 +0200
Subject: [PATCH] Fix IPA ICF test case.

gcc/testsuite/ChangeLog:

2015-06-10  Martin Liska  <mliska@suse.cz>

	* g++.dg/ipa/ipa-icf-4.C: Update expected number of merge
	operations.
---
 gcc/testsuite/g++.dg/ipa/ipa-icf-4.C | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/g++.dg/ipa/ipa-icf-4.C b/gcc/testsuite/g++.dg/ipa/ipa-icf-4.C
index b552ef4..a5b7920 100644
--- a/gcc/testsuite/g++.dg/ipa/ipa-icf-4.C
+++ b/gcc/testsuite/g++.dg/ipa/ipa-icf-4.C
@@ -44,4 +44,4 @@ int main()
 }
 
 /* { dg-final { scan-ipa-dump "\(Unified; Variable alias has been created\)|\(Symbol aliases are not supported by target\)" "icf"  } } */
-/* { dg-final { scan-ipa-dump "Equal symbols: \[67\]" "icf"  } } */
+/* { dg-final { scan-ipa-dump "Equal symbols: 2" "icf"  } } */
-- 
2.1.4