-fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

-fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Jan Hubicka]

Hi,
this patch makes the type system to be unchanged by flag_strict_aliasing.
This is needed to prevent optimization loss in flag_strict_aliasing code where
some !flag_strict_aliasing code put alias set 0 into a type (this happens
in all cases I modified in my original patch). It is also necessary to validate
ipa-icf and operand_equal_p transformations to be safe for code transitions
!flag_strict_aliasing->flag_strict_aliasing that I wasn to do in the inliner.

This patch goes the opposite way than my previous attempt (and is short unlike
the explanation ;).  Instead of adding extra parameter to get_alias_set it
makes get_alias_set do ignore flag_strict_aliasing.  To make sure that no TBAA
is used when !flag_strict_aliasing I can simply disable alias_set_subset_of and
alias_sets_conflict_p which are the only way TBAA oracle can disambiguate
items.

Next there are cases where optimizations are disabled to keep TBAA right.  
I audited the code and found only function.c (that uses object_must_conflict
for packing) and ipa-icf/fold-const.  This patch updates objects_must_conflict_p, fold-const
already check flag_strict_aliasing and I did not update ipa-icf because I would
have to disable non-strict-aliasing path in the followup patch.

I checked that there is no code difference with -fno-strict-aliasing -fno-ipa-icf
with this patch on tramp3d and dealII


Bootstrapped/regtested x86_64-linux and also lto-bootstraped. Looks OK?

	* alias.c (alias_set_subset_of, alias_sets_conflict_p,
	objects_must_conflict_p): Short circuit for !flag_strict_aliasing
	(get_alias_set): Remove flag_strict_aliasing check.
	(new_alias_set): Likewise.
Index: alias.c
===================================================================
--- alias.c	(revision 231081)
+++ alias.c	(working copy)
@@ -405,6 +405,10 @@ alias_set_subset_of (alias_set_type set1
 {
   alias_set_entry *ase2;
 
+  /* Disable TBAA oracle with !flag_strict_aliasing.  */
+  if (!flag_strict_aliasing)
+    return true;
+
   /* Everything is a subset of the "aliases everything" set.  */
   if (set2 == 0)
     return true;
@@ -466,6 +470,10 @@ alias_sets_conflict_p (alias_set_type se
   alias_set_entry *ase1;
   alias_set_entry *ase2;
 
+  /* Disable TBAA oracle with !flag_strict_aliasing.  */
+  if (!flag_strict_aliasing)
+    return true;
+
   /* The easy case.  */
   if (alias_sets_must_conflict_p (set1, set2))
     return 1;
@@ -561,6 +569,9 @@ objects_must_conflict_p (tree t1, tree t
 {
   alias_set_type set1, set2;
 
+  if (!flag_strict_aliasing)
+    return 1;
+
   /* If neither has a type specified, we don't know if they'll conflict
      because we may be using them to store objects of various types, for
      example the argument and local variables areas of inlined functions.  */
@@ -816,10 +827,12 @@ get_alias_set (tree t)
 {
   alias_set_type set;
 
-  /* If we're not doing any alias analysis, just assume everything
-     aliases everything else.  Also return 0 if this or its type is
-     an error.  */
-  if (! flag_strict_aliasing || t == error_mark_node
+  /* We can not give up with -fno-strict-aliasing because we need to build
+     proper type representation for possible functions which are build with
+     -fstirct-aliasing.  */
+
+  /* return 0 if this or its type is an error.  */
+  if (t == error_mark_node
       || (! TYPE_P (t)
 	  && (TREE_TYPE (t) == 0 || TREE_TYPE (t) == error_mark_node)))
     return 0;
@@ -1085,15 +1098,10 @@ get_alias_set (tree t)
 alias_set_type
 new_alias_set (void)
 {
-  if (flag_strict_aliasing)
-    {
-      if (alias_sets == 0)
-	vec_safe_push (alias_sets, (alias_set_entry *) NULL);
-      vec_safe_push (alias_sets, (alias_set_entry *) NULL);
-      return alias_sets->length () - 1;
-    }
-  else
-    return 0;
+  if (alias_sets == 0)
+    vec_safe_push (alias_sets, (alias_set_entry *) NULL);
+  vec_safe_push (alias_sets, (alias_set_entry *) NULL);
+  return alias_sets->length () - 1;
 }
 
 /* Indicate that things in SUBSET can alias things in SUPERSET, but that

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Richard Biener]

On Wed, 2 Dec 2015, Jan Hubicka wrote:

> Hi,
> this patch makes the type system to be unchanged by flag_strict_aliasing.
> This is needed to prevent optimization loss in flag_strict_aliasing code where
> some !flag_strict_aliasing code put alias set 0 into a type (this happens
> in all cases I modified in my original patch). It is also necessary to validate
> ipa-icf and operand_equal_p transformations to be safe for code transitions
> !flag_strict_aliasing->flag_strict_aliasing that I wasn to do in the inliner.
> 
> This patch goes the opposite way than my previous attempt (and is short unlike
> the explanation ;).  Instead of adding extra parameter to get_alias_set it
> makes get_alias_set do ignore flag_strict_aliasing.  To make sure that no TBAA
> is used when !flag_strict_aliasing I can simply disable alias_set_subset_of and
> alias_sets_conflict_p which are the only way TBAA oracle can disambiguate
> items.
> 
> Next there are cases where optimizations are disabled to keep TBAA right.  
> I audited the code and found only function.c (that uses object_must_conflict
> for packing) and ipa-icf/fold-const.  This patch updates objects_must_conflict_p, fold-const
> already check flag_strict_aliasing and I did not update ipa-icf because I would
> have to disable non-strict-aliasing path in the followup patch.
> 
> I checked that there is no code difference with -fno-strict-aliasing -fno-ipa-icf
> with this patch on tramp3d and dealII
> 
> 
> Bootstrapped/regtested x86_64-linux and also lto-bootstraped. Looks OK?
> 
> 	* alias.c (alias_set_subset_of, alias_sets_conflict_p,
> 	objects_must_conflict_p): Short circuit for !flag_strict_aliasing
> 	(get_alias_set): Remove flag_strict_aliasing check.
> 	(new_alias_set): Likewise.
> Index: alias.c
> ===================================================================
> --- alias.c	(revision 231081)
> +++ alias.c	(working copy)
> @@ -405,6 +405,10 @@ alias_set_subset_of (alias_set_type set1
>  {
>    alias_set_entry *ase2;
>  
> +  /* Disable TBAA oracle with !flag_strict_aliasing.  */
> +  if (!flag_strict_aliasing)
> +    return true;
> +
>    /* Everything is a subset of the "aliases everything" set.  */
>    if (set2 == 0)
>      return true;
> @@ -466,6 +470,10 @@ alias_sets_conflict_p (alias_set_type se
>    alias_set_entry *ase1;
>    alias_set_entry *ase2;
>  
> +  /* Disable TBAA oracle with !flag_strict_aliasing.  */
> +  if (!flag_strict_aliasing)
> +    return true;
> +
>    /* The easy case.  */
>    if (alias_sets_must_conflict_p (set1, set2))
>      return 1;
> @@ -561,6 +569,9 @@ objects_must_conflict_p (tree t1, tree t
>  {
>    alias_set_type set1, set2;
>  
> +  if (!flag_strict_aliasing)
> +    return 1;
> +

Rather than adjusting this function please adjust 
alias_sets_must_conflict_p.

Otherwise this looks ok and indeed much nicer.

Thanks,
Richard.

>    /* If neither has a type specified, we don't know if they'll conflict
>       because we may be using them to store objects of various types, for
>       example the argument and local variables areas of inlined functions.  */
> @@ -816,10 +827,12 @@ get_alias_set (tree t)
>  {
>    alias_set_type set;
>  
> -  /* If we're not doing any alias analysis, just assume everything
> -     aliases everything else.  Also return 0 if this or its type is
> -     an error.  */
> -  if (! flag_strict_aliasing || t == error_mark_node
> +  /* We can not give up with -fno-strict-aliasing because we need to build
> +     proper type representation for possible functions which are build with
> +     -fstirct-aliasing.  */
> +
> +  /* return 0 if this or its type is an error.  */
> +  if (t == error_mark_node
>        || (! TYPE_P (t)
>  	  && (TREE_TYPE (t) == 0 || TREE_TYPE (t) == error_mark_node)))
>      return 0;
> @@ -1085,15 +1098,10 @@ get_alias_set (tree t)
>  alias_set_type
>  new_alias_set (void)
>  {
> -  if (flag_strict_aliasing)
> -    {
> -      if (alias_sets == 0)
> -	vec_safe_push (alias_sets, (alias_set_entry *) NULL);
> -      vec_safe_push (alias_sets, (alias_set_entry *) NULL);
> -      return alias_sets->length () - 1;
> -    }
> -  else
> -    return 0;
> +  if (alias_sets == 0)
> +    vec_safe_push (alias_sets, (alias_set_entry *) NULL);
> +  vec_safe_push (alias_sets, (alias_set_entry *) NULL);
> +  return alias_sets->length () - 1;
>  }
>  
>  /* Indicate that things in SUBSET can alias things in SUPERSET, but that

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Jan Hubicka]

> 
> Rather than adjusting this function please adjust 
> alias_sets_must_conflict_p.
> 
> Otherwise this looks ok and indeed much nicer.

OK, will update it.  Will hold this patch until we resolve what we want to do
with the debug dumps. I do not seem to be able to reproduce any -fcompare-debug
issues with that, so perhaps we already strip the alias sets and the whoe
-fstrict-alias-set fiddling is unnecesary.

Honza

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Eric Botcazou]

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

> Bootstrapped/regtested x86_64-linux and also lto-bootstraped. Looks OK?
> 
> 	* alias.c (alias_set_subset_of, alias_sets_conflict_p,
> 	objects_must_conflict_p): Short circuit for !flag_strict_aliasing
> 	(get_alias_set): Remove flag_strict_aliasing check.
> 	(new_alias_set): Likewise.

Not clear whether it's this patch specifically or another one in the series, 
but the compiler now hangs on simple Ada code it used to compile instantly.

A couple of testcases is attached.  It looks like the compiler is now stuck in 
get_alias_set endlessly pushing references onto a vector.

-- 
Eric Botcazou

[-- Attachment #2: access1.ads --]
[-- Type: text/x-adasrc, Size: 88 bytes --]

package Access1 is

   type R;
   type S is access R;
   type R is new S;

end Access1;

[-- Attachment #3: access2.ads --]
[-- Type: text/x-adasrc, Size: 141 bytes --]

package Access2 is

    type Priv;
    type Inc is access Priv;
    type Priv is access Inc;
    C : constant Priv := new Inc;

end Access2;

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Jan Hubicka]

> > Bootstrapped/regtested x86_64-linux and also lto-bootstraped. Looks OK?
> > 
> > 	* alias.c (alias_set_subset_of, alias_sets_conflict_p,
> > 	objects_must_conflict_p): Short circuit for !flag_strict_aliasing
> > 	(get_alias_set): Remove flag_strict_aliasing check.
> > 	(new_alias_set): Likewise.
> 
> Not clear whether it's this patch specifically or another one in the series, 
> but the compiler now hangs on simple Ada code it used to compile instantly.
> 
> A couple of testcases is attached.  It looks like the compiler is now stuck in 
> get_alias_set endlessly pushing references onto a vector.
uhm, sorry. I will take a look.

Honza
> 
> -- 
> Eric Botcazou

> package Access1 is
> 
>    type R;
>    type S is access R;
>    type R is new S;
> 
> end Access1;

> package Access2 is
> 
>     type Priv;
>     type Inc is access Priv;
>     type Priv is access Inc;
>     C : constant Priv := new Inc;
> 
> end Access2;

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Jan Hubicka]

> > > Bootstrapped/regtested x86_64-linux and also lto-bootstraped. Looks OK?
> > > 
> > > 	* alias.c (alias_set_subset_of, alias_sets_conflict_p,
> > > 	objects_must_conflict_p): Short circuit for !flag_strict_aliasing
> > > 	(get_alias_set): Remove flag_strict_aliasing check.
> > > 	(new_alias_set): Likewise.
> > 
> > Not clear whether it's this patch specifically or another one in the series, 
> > but the compiler now hangs on simple Ada code it used to compile instantly.
> > 
> > A couple of testcases is attached.  It looks like the compiler is now stuck in 
> > get_alias_set endlessly pushing references onto a vector.
> uhm, sorry. I will take a look.
The problem is with the type:
(gdb) p debug_tree (p)
 <pointer_type 0x7ffff6af02a0 type <pointer_type 0x7ffff6af02a0>
    sizes-gimplified public visited unsigned DI
    size <integer_cst 0x7ffff6ad7bb8 type <integer_type 0x7ffff6adb2a0 bitsizetype> constant visited 64>
    unit size <integer_cst 0x7ffff6ad7bd0 type <integer_type 0x7ffff6adb1f8 sizetype> constant visited 8>
    align 64 symtab 0 alias set -1 canonical type 0x7ffff6af02a0
    pointer_to_this <pointer_type 0x7ffff6af02a0>>

it is a recursive pointer to itself. Does this make sense in Ada?  If so we
will need to add a recursion guard into the loop and put the alias set into
voidptr_alias_set.  It more looks like a frontend bug to me - I can not think
of a use for this beast.

Honza

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Eric Botcazou]

> The problem is with the type:
> (gdb) p debug_tree (p)
>  <pointer_type 0x7ffff6af02a0 type <pointer_type 0x7ffff6af02a0>
>     sizes-gimplified public visited unsigned DI
>     size <integer_cst 0x7ffff6ad7bb8 type <integer_type 0x7ffff6adb2a0
> bitsizetype> constant visited 64> unit size <integer_cst 0x7ffff6ad7bd0
> type <integer_type 0x7ffff6adb1f8 sizetype> constant visited 8> align 64
> symtab 0 alias set -1 canonical type 0x7ffff6af02a0
>     pointer_to_this <pointer_type 0x7ffff6af02a0>>
> 
> it is a recursive pointer to itself. Does this make sense in Ada?  If so we
> will need to add a recursion guard into the loop and put the alias set into
> voidptr_alias_set.  It more looks like a frontend bug to me - I can not
> think of a use for this beast.

This one (access1) is admittedly border line and we can probably kludge around 
it in the front-end, but what about access2 and more generally pointer cycles?

-- 
Eric Botcazou

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Richard Biener]

On Tue, 8 Dec 2015, Eric Botcazou wrote:

> > The problem is with the type:
> > (gdb) p debug_tree (p)
> >  <pointer_type 0x7ffff6af02a0 type <pointer_type 0x7ffff6af02a0>
> >     sizes-gimplified public visited unsigned DI
> >     size <integer_cst 0x7ffff6ad7bb8 type <integer_type 0x7ffff6adb2a0
> > bitsizetype> constant visited 64> unit size <integer_cst 0x7ffff6ad7bd0
> > type <integer_type 0x7ffff6adb1f8 sizetype> constant visited 8> align 64
> > symtab 0 alias set -1 canonical type 0x7ffff6af02a0
> >     pointer_to_this <pointer_type 0x7ffff6af02a0>>
> > 
> > it is a recursive pointer to itself. Does this make sense in Ada?  If so we
> > will need to add a recursion guard into the loop and put the alias set into
> > voidptr_alias_set.  It more looks like a frontend bug to me - I can not
> > think of a use for this beast.
> 
> This one (access1) is admittedly border line and we can probably kludge around 
> it in the front-end, but what about access2 and more generally pointer cycles?

Usually cycles happen through structure members and it might be that
all other frontends have the pointed-to type incomplete.  But the
above recursion shouldn't apply for the structure case.

Not sure how your other examples look like.

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Eric Botcazou]

> Usually cycles happen through structure members and it might be that
> all other frontends have the pointed-to type incomplete.  But the
> above recursion shouldn't apply for the structure case.

All types are equal in Ada and can be forward declared; the language specifies 
that their "elaboration" can be delayed until a "freeze" point (in particular, 
you cannot declare an object of the type until after it), from which all the 
incomplete references must be resolved to the final type.

> Not sure how your other examples look like.

Pure pointer cycles of any length.

-- 
Eric Botcazou

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Jan Hubicka]

> > Usually cycles happen through structure members and it might be that
> > all other frontends have the pointed-to type incomplete.  But the
> > above recursion shouldn't apply for the structure case.
> 
> All types are equal in Ada and can be forward declared; the language specifies 
> that their "elaboration" can be delayed until a "freeze" point (in particular, 
> you cannot declare an object of the type until after it), from which all the 
> incomplete references must be resolved to the final type.
> 
> > Not sure how your other examples look like.
> 
> Pure pointer cycles of any length.

OK, the code already pre-allocates the vector to be 8 elements.  What about
simply punting when reaching this depth? I do not think real world program have
more than 8 nested pointers often enough for this to matter.
They will then get same alias set as void *.

Honza
> 
> -- 
> Eric Botcazou

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Eric Botcazou]

> OK, the code already pre-allocates the vector to be 8 elements.  What about
> simply punting when reaching this depth? I do not think real world program
> have more than 8 nested pointers often enough for this to matter.
> They will then get same alias set as void *.

Fine with me, but the len<8 cases need to be dealt with as well.

-- 
Eric Botcazou

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Jan Hubicka]

Hi
this patch implements the trik for punting if we get too many nested pointers.
This fixes the ada tstcases. Curiously enough I would like to replace safe_push
by quick_push but doing so I get weird error about freeing non-heap object
in the auto_vec desructor...

Bootstraping/regtesting x86_64-linux. Ok if it passes?

Honza

	* alias.c (get_alias_set): Punt after getting 8 nested pointers.

Index: alias.c
===================================================================
--- alias.c	(revision 231439)
+++ alias.c	(working copy)
@@ -990,6 +990,14 @@ get_alias_set (tree t)
 	   || TREE_CODE (p) == VECTOR_TYPE;
 	   p = TREE_TYPE (p))
 	{
+	  /* Ada supports recusive pointers.  Instead of doing recrusion check
+	     just give up once the preallocated space of 8 elements is up.
+	     In this case just punt to void * alias set.  */
+	  if (reference.length () == 8)
+	    {
+	      p = ptr_type_node;
+	      break;
+	    }
 	  if (TREE_CODE (p) == REFERENCE_TYPE)
 	    /* In LTO we want languages that use references to be compatible
  	       with languages that use pointers.  */

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Arnaud Charlet]

> Hi
> this patch implements the trik for punting if we get too many nested
> pointers.
> This fixes the ada tstcases. Curiously enough I would like to replace
> safe_push
> by quick_push but doing so I get weird error about freeing non-heap object
> in the auto_vec desructor...
> 
> Bootstraping/regtesting x86_64-linux. Ok if it passes?
> 
> Honza
> 
> 	* alias.c (get_alias_set): Punt after getting 8 nested pointers.
> 
> Index: alias.c
> ===================================================================
> 
> --- alias.c	(revision 231439)
> +++ alias.c	(working copy)
> @@ -990,6 +990,14 @@ get_alias_set (tree t)
>  	   || TREE_CODE (p) == VECTOR_TYPE;
>  	   p = TREE_TYPE (p))
>  	{
> +	  /* Ada supports recusive pointers.  Instead of doing recrusion check

typo above: recrusion -> recursion

> +	     just give up once the preallocated space of 8 elements is up.
> +	     In this case just punt to void * alias set.  */
> +	  if (reference.length () == 8)

We don't use magic numbers in general, can you please replace by a named
constant instead?

> +	    {
> +	      p = ptr_type_node;
> +	      break;
> +	    }
>  	  if (TREE_CODE (p) == REFERENCE_TYPE)
>  	    /* In LTO we want languages that use references to be compatible
>   	       with languages that use pointers.  */

I'll let others comment on the general idea.

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Richard Biener]

On Wed, 9 Dec 2015, Jan Hubicka wrote:

> Hi
> this patch implements the trik for punting if we get too many nested pointers.
> This fixes the ada tstcases. Curiously enough I would like to replace safe_push
> by quick_push but doing so I get weird error about freeing non-heap object
> in the auto_vec desructor...

That's odd but a sign of you doing sth wrong ;)  Like somehow
clobbering m_using_auto_storage of the vec.

But of course if you use a maximum length of 8 there
is no need to use a vec<>, just use a fixed bool[8] array
(or an integer mask).

> Bootstraping/regtesting x86_64-linux. Ok if it passes?

Ok.

Thanks,
Richard.

> Honza
> 
> 	* alias.c (get_alias_set): Punt after getting 8 nested pointers.
> 
> Index: alias.c
> ===================================================================
> --- alias.c	(revision 231439)
> +++ alias.c	(working copy)
> @@ -990,6 +990,14 @@ get_alias_set (tree t)
>  	   || TREE_CODE (p) == VECTOR_TYPE;
>  	   p = TREE_TYPE (p))
>  	{
> +	  /* Ada supports recusive pointers.  Instead of doing recrusion check
> +	     just give up once the preallocated space of 8 elements is up.
> +	     In this case just punt to void * alias set.  */
> +	  if (reference.length () == 8)
> +	    {
> +	      p = ptr_type_node;
> +	      break;
> +	    }
>  	  if (TREE_CODE (p) == REFERENCE_TYPE)
>  	    /* In LTO we want languages that use references to be compatible
>   	       with languages that use pointers.  */

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Richard Biener]

On Wed, 9 Dec 2015, Arnaud Charlet wrote:

> > Hi
> > this patch implements the trik for punting if we get too many nested
> > pointers.
> > This fixes the ada tstcases. Curiously enough I would like to replace
> > safe_push
> > by quick_push but doing so I get weird error about freeing non-heap object
> > in the auto_vec desructor...
> > 
> > Bootstraping/regtesting x86_64-linux. Ok if it passes?
> > 
> > Honza
> > 
> > 	* alias.c (get_alias_set): Punt after getting 8 nested pointers.
> > 
> > Index: alias.c
> > ===================================================================
> > 
> > --- alias.c	(revision 231439)
> > +++ alias.c	(working copy)
> > @@ -990,6 +990,14 @@ get_alias_set (tree t)
> >  	   || TREE_CODE (p) == VECTOR_TYPE;
> >  	   p = TREE_TYPE (p))
> >  	{
> > +	  /* Ada supports recusive pointers.  Instead of doing recrusion check
> 
> typo above: recrusion -> recursion
> 
> > +	     just give up once the preallocated space of 8 elements is up.
> > +	     In this case just punt to void * alias set.  */
> > +	  if (reference.length () == 8)
> 
> We don't use magic numbers in general, can you please replace by a named
> constant instead?

Instead of a magic constant you could also use sth like TREE_VISITED
(or a pointer-map).  Of course that's a lot more expensive.

> > +	    {
> > +	      p = ptr_type_node;
> > +	      break;
> > +	    }
> >  	  if (TREE_CODE (p) == REFERENCE_TYPE)
> >  	    /* In LTO we want languages that use references to be compatible
> >   	       with languages that use pointers.  */
> 
> I'll let others comment on the general idea.
> 
> 

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

Re: -fstrict-aliasing fixes 5/6: make type system independent of flag_strict_aliasing

[Jan Hubicka]

> On Wed, 9 Dec 2015, Arnaud Charlet wrote:
> 
> > > Hi
> > > this patch implements the trik for punting if we get too many nested
> > > pointers.
> > > This fixes the ada tstcases. Curiously enough I would like to replace
> > > safe_push
> > > by quick_push but doing so I get weird error about freeing non-heap object
> > > in the auto_vec desructor...
> > > 
> > > Bootstraping/regtesting x86_64-linux. Ok if it passes?
> > > 
> > > Honza
> > > 
> > > 	* alias.c (get_alias_set): Punt after getting 8 nested pointers.
> > > 
> > > Index: alias.c
> > > ===================================================================
> > > 
> > > --- alias.c	(revision 231439)
> > > +++ alias.c	(working copy)
> > > @@ -990,6 +990,14 @@ get_alias_set (tree t)
> > >  	   || TREE_CODE (p) == VECTOR_TYPE;
> > >  	   p = TREE_TYPE (p))
> > >  	{
> > > +	  /* Ada supports recusive pointers.  Instead of doing recrusion check
> > 
> > typo above: recrusion -> recursion

Thanks, updated in my copy.
> > 
> > > +	     just give up once the preallocated space of 8 elements is up.
> > > +	     In this case just punt to void * alias set.  */
> > > +	  if (reference.length () == 8)
> > 
> > We don't use magic numbers in general, can you please replace by a named
> > constant instead?
> 
> Instead of a magic constant you could also use sth like TREE_VISITED
> (or a pointer-map).  Of course that's a lot more expensive.

Yep, I think in this specific case a magic constant is resonable - it makes the
code a lot easier and I simply can't think of sane use for more than 8
pointers.  Of course it is not particularly time sensitive thing, so I can make
a version with pointer map.

I do not think I can safely use TREE_VISITED here since get_alias_set is
called from random parts of the compiler.

Honza
> 
> > > +	    {
> > > +	      p = ptr_type_node;
> > > +	      break;
> > > +	    }
> > >  	  if (TREE_CODE (p) == REFERENCE_TYPE)
> > >  	    /* In LTO we want languages that use references to be compatible
> > >   	       with languages that use pointers.  */
> > 
> > I'll let others comment on the general idea.
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)