Re: [PATCH] Fix inconsistency with alias_sets_conflict_p and alias_set_subset_of

[Richard Guenther <rguenther at suse dot de>]

On Thu, 15 Apr 2010, Diego Novillo wrote:

> On 4/15/10 07:24 , Richard Guenther wrote:
> > 
> > This fixes an inconsistency with the abovementioned two functions.
> > The inconsistency is that if alias set A has zero as subset
> > whether B then is a subset of it always or only if its alias set is zero.
> 
> If alias set A has zero as subset, shouldn't it follow that A is
> effectively alias set zero?  IOW, shouldn't alias set zero propagate
> "upwards" as well?

Well, yes.  The code in record_alias_subset is slightly odd:

  if (subset == 0)
    superset_entry->has_zero_child = 1;
  else
    {
      subset_entry = get_alias_set_entry (subset);
      /* 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;

          splay_tree_foreach (subset_entry->children, 
insert_subset_children,
                              superset_entry->children);
        }

      /* Enter the SUBSET itself as a child of the SUPERSET.  */
      splay_tree_insert (superset_entry->children,
                         (splay_tree_key) subset, 0);
    }

so we maintain this has_zero_child set and for other sets we
recursively add the subsets children (but notice that we never
fixup supersets when we later add to one of its subsets
with record_alias_subset!?  Ok, the way we build up the
hierarchy sort-of makes sure this won't happen).

Now if we have has_zero_child we could simply drop all the
supersets children as that information is now irrelevant, but
we do not do this.

Well, it seems to me we do not properly maintain the alias-set
forest.

> Otherwise, we have this weird inversion where a limited alias set is
> "bigger" than the universal alias set.

Yep.  The problem is that C character types use the magic alias-set
zero.  So we'd pessimize every structure containing a char to
effectively have disabled TBAA - the workaround was to have this
weird inversion.

> Perhaps when building the alias set tree, we should detect this and
> collapse?

I'm not sure - it would be the correct thing to do if we decide
that has_zero_child makes the superset equal to alias-set zero
(which we do, at least in the comments ...)

  /* Nonzero if would have a child of zero: this effectively makes this
     alias set the same as alias set zero.  */
  int has_zero_child;

So it's the C frontend that we can blame for the resulting
pessimization?

Richard.