Re: Teach same_types_for_tbaa to structurally compare arrays, pointers and vectors

[Richard Biener <rguenther@suse.de>]

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

On Mon, 27 May 2019, Jan Hubicka wrote:

> > The way you do it above seeing struct X ****p will end up comparing
> > 'struct X' but that doesn't really have any say on whether we
> > can apply TBAA to the outermost pointer type which, if used as a base,
> > cannot be subsetted by components anyway.
> 
> We remove pointers in pairs so seeing
> struct X ****p
> struct Y ****q
> we will end up saying that these pointers are same if struct X and Y are
> same (which we will do by pointer type) or if we can not decide (one of
> them is void).
> 
> Non-LTO code returns 0 in the second case, but I think that could be
> considered unsafe when mixing K&R and ansi-C code.
> > 
> > So - why's anything besides treating all structurally equivalent
> > non-composite types as the same sensible here (and not waste of time)?
> 
> I think you are right that with current implementation it should not
> make difference.  I want eventually to disambiguate
> 
> struct foo {struct bar *a;} ptr1;
> struct foobar **ptr2;
> 
> ptr1->a and *ptr2;
> 
> Here we will currently punt on comparing different pointer types.
> With structural compare we will end up to base&offset because we would
> consider struct foobar * and struct bar * as same types (they are both
> incomplete in LTO now).

But *ptr2 has no access 'path' so we shouldn't even consider it here.

That is, when the innermost reference (for *ptr2 that is a reference
to type foobar *) is of non-aggregate type there's no paths to
disambiguate.  That is same_types_for_tbaa shouldn't even be asked
for non-aggregate types...

> Same_types_for_tbaa does not need to form equivalences and if we unwind
> pointers&arrays to types mathing odr_comparable_p (i.e. we have two
> accesses originating from C++ code), we can disambiguate incompete
> pointers based on mangled name of types they point to.  I have
> incremental patch for that (which futher improves disambiguations to
> about 8000).
> > 
> > I realize this is mostly my mess but if we try to "enhance" things
> > we need to make it clearer what we want...
> > 
> > Maybe even commonize more code paths (the path TBAA stuff is really
> > replicated in at least two places IIRC).
> 
> Yep, I am trying to understand what we need to do here and clean things
> up.
> 
> I guess we could handle few independent issues
> 1) if it is safe to return 1 for types that compare as equivalent as
>    pointers (currently we return -1 if canonical types are not defined).
>    This seems to handle a most common case
> 2) decide what to do about pointers
> 3) decide what to do about arrays
> 4) decide what to do about ODR 
> 5) see how much we can merge with alias set & canonical type
> computation.
> 
> I would propose first just add
>  if (type1 == type2)
>     reutrn 1;

That works for me.

> and I will do bit deeper look at the pointers next and produce also some
> testcases.

Please also see if there are testcases that do anything meaningful
and FAIL after instead of

  /* Do access-path based disambiguation.  */
  if (ref1 && ref2
      && (handled_component_p (ref1) || handled_component_p (ref2)))

doing

  /* Do access-path based disambiguation.  */
  if (ref1 && ref2
      && (handled_component_p (ref1) && handled_component_p (ref2)))

Thanks.
Richard.

> 
> Honza
> > 
> > Richard.
> > 
> > > Honza
> > > > 
> > > > > +      else
> > > > > +	{
> > > > > +	  if (POINTER_TYPE_P (type1) != POINTER_TYPE_P (type2))
> > > > > +	    return 0;
> > > > > +	  return in_ptr ? 1 : -1;
> > > > > +	}
> > > > > +
> > > > > +      if (type1 == type2)
> > > > > +        return in_array ? -1 : 1;
> > > > > +    }
> > > > >  
> > > > >    /* Compare the canonical types.  */
> > > > >    if (TYPE_CANONICAL (type1) == TYPE_CANONICAL (type2))
> > > > > -    return 1;
> > > > > +    return in_array ? -1 : 1;
> > > > >  
> > > > >    /* ??? Array types are not properly unified in all cases as we have
> > > > >       spurious changes in the index types for example.  Removing this
> > > > >       causes all sorts of problems with the Fortran frontend.  */
> > > > >    if (TREE_CODE (type1) == ARRAY_TYPE
> > > > >        && TREE_CODE (type2) == ARRAY_TYPE)
> > > > > -    return -1;
> > > > > +    return in_ptr ? 1 : -1;
> > > > >  
> > > > >    /* ??? In Ada, an lvalue of an unconstrained type can be used to access an
> > > > >       object of one of its constrained subtypes, e.g. when a function with an
> > > > > @@ -770,7 +858,7 @@ same_type_for_tbaa (tree type1, tree typ
> > > > >       not (e.g. type and subtypes can have different modes).  So, in the end,
> > > > >       they are only guaranteed to have the same alias set.  */
> > > > >    if (get_alias_set (type1) == get_alias_set (type2))
> > > > > -    return -1;
> > > > > +    return in_ptr ? 1 : -1;
> > > > >  
> > > > >    /* The types are known to be not equal.  */
> > > > >    return 0;
> > > > > 
> > > > 
> > > > -- 
> > > > Richard Biener <rguenther@suse.de>
> > > > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> > > > GF: Felix ImendĂśrffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NĂźrnberg)
> > > 
> > > 
> > 
> > -- 
> > Richard Biener <rguenther@suse.de>
> > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> > GF: Felix ImendĂśrffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NĂźrnberg)
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix ImendĂśrffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NĂźrnberg)