This is an archive of the discontinued LLVM Phabricator instance.

Add option to use older clang ABI behavior when passing certain union types as function arguments
ClosedPublic

Authored by dyung on Oct 19 2020, 3:28 PM.

Details

Summary

Recently commit D78699 (commit 26cfb6e562f1), fixed clang's behavior with respect to passing a union type through a register to correctly follow the ABI. However, this is an ABI breaking change with earlier versions of the clang compiler, so we should add an -fclang-abi-compat option to address this. Additionally, the PS4 ABI requires the older behavior, so that is added as well.

This change adds a Ver11 value to the ClangABI enum that when it is set (or the target is the PS4 triple), we skip the ABI fix introduced in D78699.

Diff Detail

Event Timeline

dyung created this revision.Oct 19 2020, 3:28 PM
dyung requested review of this revision.Oct 19 2020, 3:28 PM
rsmith accepted this revision.Oct 19 2020, 6:00 PM

This seems fine to me; thanks for the fix.

For future reference, please upload diffs with full context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).

clang/test/CodeGen/X86/avx-union.c
7

There are no added CHECK lines for --check-prefix=AVX-LEGACY. Is that intentional, or is there a AVX-LEGACY: line missing from the test below?

This revision is now accepted and ready to land.Oct 19 2020, 6:00 PM
dyung added a comment.Oct 19 2020, 6:02 PM

Sorry for the lack of context, I made my diffs using a new method this time, but I'll make sure it has the full context next time. Thanks!

clang/test/CodeGen/X86/avx-union.c
7

It was intentional since the AVX-LEGACY was the same. I debated putting it in to make it clearer, but decided to leave it out thinking it might cause more confusing to see two identical CHECK lines.

Adding an option compatible with old abi is good method. Looks good for me.

rsmith added inline comments.Oct 19 2020, 10:20 PM
clang/test/CodeGen/X86/avx-union.c
7

If there are no check lines corresponding to this check-prefix, it'd be better to not list it to avoid confusing future readers.