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

Hal Finkel via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 19 11:54:12 PDT 2017


hfinkel added a comment.

> 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.  Removing the TBAA from union accesses would make us conservative. In the long term, I think we agree that we need a better way of actually representing unions in our TBAA representation. The question is what to do in the short term. Maybe there's some less drastic short-term solution that addresses these test cases?

> (And the nonsensicality of the standard very much continues. The code that we've all agreed is obviously being miscompiled here is still a strict violation of the "improved" union rules in C++, and if we decide to treat it as ill-formed we're going to break a massive amount of code (and vector code in particular heavily uses unions), because trust me, nobody is reliably placement-newing union fields when they're non-trivial. C++'s new rules are unworkable in part because they rely on C++-specific features that cannot be easily adopted in headers which need to be language-neutral.)

I think we all agree that the vector types need to have their scalar (element) types as parents in the current TBAA representation. That's a separate change that we should just make. That, unfortunately, only fixes a subset of the union-related miscompiles.

@rjmccall, if you agree that not adding TBAA to union accesses is reasonable at this stage, what do you think of this implementation of that goal?


Repository:
  rL LLVM

https://reviews.llvm.org/D31885





More information about the cfe-commits mailing list