[PATCH] D31885: Remove TBAA information from LValues representing union members

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D31885#730799, @dberlin wrote:

> In https://reviews.llvm.org/D31885#730766, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D31885#730539, @dberlin wrote:
> >
> > > In https://reviews.llvm.org/D31885#730072, @rjmccall wrote:
> > >
> > > > Thanks for CC'ing me.  There are two problems here.
> > > >
> > > > The second is that our alias-analysis philosophy says that the fact that two accesses "sufficiently obviously" can alias should always override TBAA.
> > >
> > >
> > > I'm not sure i agree.
> > >  We have made explicit exceptions for parts of the standard that are nonsensical (and they've been slowly updated in newer standards to be less nonsensical).  Outside of that, i don't believe that is our philosophy, nor would i believe it should be.
> > >  There is no "do what i mean" here we can define that makes sense.
> >
> >
> > There was a deliberate decision to make TBAA conservative about type punning in LLVM because, in practice, the strict form of TBAA you think we should follow has proven to be a failed optimization paradigm: it just leads users to globally disable TBAA, which is obviously a huge loss.   This was never motivated purely by the nonsensicality of the standardization of unions.
>
>
> I think you have completely missed my point.  i have not said we should be super strict,. We already aren't around unions. I"m fine with that. I'm saying "there is no DWIM like you are advocating we can reasonably define overall" and " whatever we do come up with, we can't do it in the backend".   I don't believe a philosophy of "we come up with a reasonable interpretation and write it down" is a "failed paradigm" (it's also definitely not going to be DWIM).    Compilers already are pretty nice about TBAA relative to what the standard says, as they must be.  People who want to abide turn it on, those who don't, don't.  Most actually *do* turn it on, contrary to your argument that it is a failed paradigm.


We have it on by default.  That has been acceptable to users precisely because we are conservative about punning.  In contrast, GCC adopted very strict rules about punning that led basically everyone to turn off strict aliasing.  That is what I am describing as a failed paradigm.

> You seem to be advocating for "if it's obvious they should alias, they should alias", but that is not amenable to any real implementation either, and we don't get whatever we thought was the right answer to this question right now anyway!

That's our current implementation.  That's why TBAA delegates to BasicAA and allows it to override its judgement.  That is not an accident.

> So let me be concrete: How, precisely, do you propose we fix these bugs, such that they work 100% of the time, and do not require the backend to understand language rules?

I'm not saying the backend should magically understand language rules.  Clearly language rules should be abstractly represented in IR in a way that can be interpreted.  Case in point, we need to be representing the aliasability of vector types and their elements correctly in TBAA metadata.

I'm saying that it's sometimes okay for frontends to rely on an operational understanding of how the backend uses that information, along with major exceptions and overrides to its analysis.  For example, readonly/readnone have a basically behavioral semantics that doesn't actually constrain the operations carried out by the callee.  This kind of analysis is appropriate for situations like this where we want to be generally aggressive as long as any resulting miscompiles are reasonably defensible, which in practice means not miscompiling sufficiently obvious instances of type-punning.

Your proposal to we simply drop TBAA from all accesses related to unions is extremely conservative.  It means that an access through a union has to be treated as potentially aliasing every other visible access, at least in terms of their types.  That level of conservatism is not necessary in this case if we hew to the original understanding of LLVM's TBAA that punning is only allowed when it is sufficiently obvious.  If you are interested in making a strict definition of "sufficiently obvious", and then arguing that it cannot possibly work, we can have that conversation; but right now I'm not seeing anything that says that our approach is basically broken, other you repeatedly asserting it because you think TBAA should be capable of completely standing alone.

My proposal is that (1) we should correctly encode the aliasability of vector types in TBAA metadata and, separately, (2) we should also ensure that TBAA is correctly overridden by BasicAA in this case, which may require introducing an intermediate level of response between MayAlias and MustAlias.

John.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885