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 <rguent...@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 <rguent...@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 <rguent...@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
