[PATCH] D12269: Add a pass to lift aggregate into allocas so SROA can get rid of them.

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

I indicated previously that I wasn't sure this is the correct approach. I wanted to clarify that.

The problem I see with the approach of lowering things as loads and stores of (perhaps excessively large) integers is that I think it creates a bad canonicalization problem. Let's consider what happens when we lower as i128 loads and stores of an aggregate. Later on, we may inline and end up access the low and high 64 bits as f64 types. But in order to do that, we would have to use shift and trunc instructions on the integer values. As a consequence of these shift and truncs, we would have integer operations on the bits and avoid properly canonicalizing loads and stores or the basic domain used. =/ I'm moderately worried about this.

The crux of the problem is that we would introduce *strongly typed* operations like 'shl', 'lshr' to extract the narrow regions from the former aggregate. The extractvalue (and insertvalue) instructions quite usefully can attach the type of the extracted or inserted value rather than any particular integer type. As a consequence, I think that we should model this by extracting each value from the FCA, and storing it separately.

For function arguments, this logic doesn't really seem to apply. There, I feel like frontends should be able to fully decompose things or fuse things into proper primitive types.

The only remaining reason why aggregates really exist are to support return types. I actually think that's OK, they're not a terrible syntax and mechanism for modeling multiple return types. I would particularly like it if the only thing you could reasonably do is extractvalue the independent values. This would make aggregates just the LLVM mechanism for doing multiple return types with proper SSA form. I think that's fine. Maybe some day we would even want to replace it with TokenType, for now aggregates provide a nice layer of type checking and sanity checking for returns.

Now, some may wonder what about my vehement arguments about bitfields using large integer types for loads and stores and the memory model implications. Fundamentally, I think these are different concerns. There are two core differences. First, bitfields are *defined* as a single memory location by the frontend language, and so it is reasonable for us to go to some lengths to operate on them as such. I don't think that aggregates are actually so defined in any source languages I'm aware of (C, C++, ObjC, Go, Swift, CUDA, to name a few). But as an example, if a frontend wants to *specifically define* a load or store of an aggregate as an atomic load or store of N bits of memory, I'd rather the frontend choose to express that in an N-bit integer operation with the IR rather than a transform pass doing it.

The second, and perhaps overriding reason why I think all of the arguments line up a different way for bitfields is that bitfields quite fundamentally *are* integers. We lose nothing by modeling them as such and create no canonicalization problems.

Does this make sense to folks?

(Again, sorry for the delay writing this up. A decent chunk of it was taking quite a few hours to carefully think through all the consequences and convince myself that they were OK.)

-Chandler


http://reviews.llvm.org/D12269