This is an archive of the discontinued LLVM Phabricator instance.

[x86-64 ABI] Fix for PR23082: an assertion failure when passing/returning a wrapper union in a full YMM register.
ClosedPublic

Authored by andreadb on Jun 2 2015, 11:04 AM.

Details

Summary

This patch fixes PR23082.

For the purpose of ABI classification, vectors are classified based on their 'Width'. The 'Width' is computed starting from the size of the element type multiplied by the number of packed elements in the vector. However, if the 'Width' is not a power-of-2, it gets implicitly rounded to the next power-of-2.

By construction, ABI class "SSEUp" is only used for vector types (or aggregate data structures that wrap vector type).
Also, by construction, the Width of a vector type of ABI class 'SSEUp' can only be 16 or 32 bytes.

So, when method 'GetByteVectorType' is called, the size of type 'Ty' can only be either 16 or 32 bytes. At this point, the Frontend could just return an IR vector type that matches in size the size of QualType 'Ty'. For example, the Frontend could return <2 x double> for the case where the Size is 128 bits (this used to be the default before r229408); alternatively, a vector type <4 x double> for the case where Size is 256 bits).

What currently happens is that the Frontend tries to be friendly and select the "best" IR vector type for the QualType 'Ty'. In particular, if 'Ty' is a wrapper structure, it keeps unwrapping it until it finds a vector type VTy. That VTy would then be our "preferred IR type". However, method 'isSingleElementStructure' (which is used to unwrap structures) doesn't know how to 'look through unions'. So, if end up with a nesting of wrapper structs/union we might end up triggering the assertion added at revision 230971.

In my experiments, the assertion failure reported as PR23082 only occurs if we a nest of wrapper structures with at least two union types. In all other cases, calling method 'isSingleElementStructure' is always okay.

I decided to address the problem in a simple way:
if we fail to find the "preferred type" for a single element structure 'Ty', then we just return a potentially "less friendly" vector type which would still be valid according to the ABI. So, rather than asserting on a valid type in input, we return a vector type which is <2 x double> if size is 16 bytes, or <4 x double> if size is 32 bytes.
An alternative approach consists in teaching 'isSingleElementStructure' how to handle unions. However, I decided to go for the safest approach (since 'isSingleElementStructure' is also used by other ABI, and not only by the x86-64 ABI) and conservatively return a vector type which is always known to be okay for the ABI.

Please let me know what you think.

Thanks,
Andrea

Diff Detail

Repository
rL LLVM

Event Timeline

andreadb updated this revision to Diff 26986.Jun 2 2015, 11:04 AM
andreadb retitled this revision from to [x86-64 ABI] Fix for PR23082: an assertion failure when passing/returning a wrapper union in a full YMM register..
andreadb updated this object.
andreadb edited the test plan for this revision. (Show Details)
andreadb added reviewers: rnk, hfinkel, spatel, bkramer.
andreadb added a subscriber: Unknown Object (MLST).
bkramer accepted this revision.Jun 2 2015, 11:09 AM
bkramer edited edge metadata.

LGTM, thanks!

This revision is now accepted and ready to land.Jun 2 2015, 11:09 AM
This revision was automatically updated to reflect the committed changes.

Thanks Benjamin!
Committed revision 238861.