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

Wed Apr 19 12:33:50 PDT 2017

dberlin added a comment.

In https://reviews.llvm.org/D31885#730909, @rjmccall wrote:

> In https://reviews.llvm.org/D31885#730853, @hfinkel wrote:
>
> > > 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.
> >
> > Okay, so the problem right now is that we're **not** conservative regarding union type punning, and while BasicAA sometimes fixes this for us, it doesn't always, and can't always, and so now we're miscompiling code.
>
>
> I agree that BasicAA "can't always", but I'm not sure it "can't often enough".  Again, it seems to me that the low-level problem is just that the AA interface isn't designed for what we're trying to do with TBAA.  "MayAlias" is the default answer for everything that can't be proven one way or the other, and "MustAlias" demands that the memory be actually known to overlap.  If there were an intermediate "LikelyAlias" for pointers that are known to be related but BasicAA just doesn't know for certain whether the accesses overlap, then TBAA would turn itself off in those cases as long as at least a basic value-propagation pass has been run.  That would put us on much firmer ground to say "it's reasonable for the compiler to assume that these things don't alias, and if you're going to use type-punning like this, you just need to rewrite your code to make it more obvious that the pointers are related".  When you're given a language rule as weak as this, especially one that's so frequently violated, that's the only kind of argument which is going to have any traction with users.
>
> John.

In https://reviews.llvm.org/D31885#730909, @rjmccall wrote:

> In https://reviews.llvm.org/D31885#730853, @hfinkel wrote:
>
> > > 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.
> >
> > Okay, so the problem right now is that we're **not** conservative regarding union type punning, and while BasicAA sometimes fixes this for us, it doesn't always, and can't always, and so now we're miscompiling code.
>
>
> I agree that BasicAA "can't always", but I'm not sure it "can't often enough".  Again, it seems to me that the low-level problem is just that the AA interface isn't designed for what we're trying to do with TBAA.  "MayAlias" is the default answer for everything that can't be proven one way or the other, and "MustAlias" demands that the memory be actually known to overlap.  If there were an intermediate "LikelyAlias" for pointers that are known to be related but BasicAA just doesn't know for certain whether the accesses overlap, then TBAA would turn itself off in those cases as long as at least a basic value-propagation pass has been run.  That would put us on much firmer ground to say "it's reasonable for the compiler to assume that these things don't alias, and if you're going to use type-punning like this, you just need to rewrite your code to make it more obvious that the pointers are related".  When you're given a language rule as weak as this, especially one that's so frequently violated, that's the only kind of argument which is going to have any traction with users.
>
> John.

Just so i understand: Ignoring everything else (we can't actually make likelyalias work, i think the code in the bugs makes that very clear), you also believe we should effectively pessimize every other language that generates correct TBAA info for LLVM  and will now no longer get optimized well because we've decided not to have clang emit TBAA metadata only in the cases where  it actually wants TBAA applied?

Repository:
  rL LLVM

https://reviews.llvm.org/D31885