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

[dyung]

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.

[rsmith]

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?

[dyung]

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.

[LiuChen3]

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

[rsmith]

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.

clang/include/clang/Basic/LangOptions.h

enum class ClangABI {
     Ver7,
 
     /// Attempt to be ABI-compatible with code generated by Clang 9.0.x
     /// (SVN r351319). This causes vectors of __int128 to be passed in memory
     /// instead of passing in multiple scalar registers on x86_64 on Linux and
     /// NetBSD.
     Ver9,
 
+    /// Attempt to be ABI-compatible with code generated by Clang 11.0.x
+    /// (git  2e10b7a39b93). This causes clang to pass unions with a 256-bit
+    /// vector member on the stack instead of using registers.
+    Ver11,
+
     /// Conform to the underlying platform's C and C++ ABIs as closely
     /// as we can.
     Latest
   };
 
   enum class CoreFoundationABI {
     /// No interoperability ABI has been specified
     Unspecified,

clang/lib/CodeGen/TargetInfo.cpp

if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
           postMerge(Size, Lo, Hi);
           return;
         }
       }
     }
 
     // Classify the fields one at a time, merging the results.
     unsigned idx = 0;
-    bool IsUnion = RT->isUnionType();
+    bool UseClang11Compat = getContext().getLangOpts().getClangABICompat() <=
+                                LangOptions::ClangABI::Ver11 ||
+                            getContext().getTargetInfo().getTriple().isPS4();
+    bool IsUnion = RT->isUnionType() && !UseClang11Compat;
+
     for (RecordDecl::field_iterator i = RD->field_begin(), e = RD->field_end();
            i != e; ++i, ++idx) {
       uint64_t Offset = OffsetBase + Layout.getFieldOffset(idx);
       bool BitField = i->isBitField();
 
       // Ignore padding bit-fields.
       if (BitField && i->isUnnamedBitfield())
         continue;

clang/lib/Frontend/CompilerInvocation.cpp

if (!VerParts.first.startswith("0") &&
       else if (Major <= 4)
         Opts.setClangABICompat(LangOptions::ClangABI::Ver4);
       else if (Major <= 6)
         Opts.setClangABICompat(LangOptions::ClangABI::Ver6);
       else if (Major <= 7)
         Opts.setClangABICompat(LangOptions::ClangABI::Ver7);
       else if (Major <= 9)
         Opts.setClangABICompat(LangOptions::ClangABI::Ver9);
+      else if (Major <= 11)
+        Opts.setClangABICompat(LangOptions::ClangABI::Ver11);
     } else if (Ver != "latest") {
       Diags.Report(diag::err_drv_invalid_value)
           << A->getAsString(Args) << A->getValue();
     }
   }
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
   Opts.BuildingPCHWithObjectFile = Args.hasArg(OPT_building_pch_with_obj);

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

 // RUN: %clang_cc1 -w -ffreestanding -triple x86_64-linux-gnu -target-feature +avx -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=AVX
 // RUN: %clang_cc1 -w -ffreestanding -triple x86_64-linux-gnu -target-feature +avx512f -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=AVX512
+
+// Test Clang 11 and earlier behavior
+// RUN: %clang_cc1 -w -ffreestanding -triple x86_64-linux-gnu -target-feature +avx -fclang-abi-compat=10.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=AVX --check-prefix=CHECK-LEGACY
+// RUN: %clang_cc1 -w -ffreestanding -triple x86_64-linux-gnu -target-feature +avx512f -fclang-abi-compat=11.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-LEGACY --check-prefix=AVX512-LEGACY
+// RUN: %clang_cc1 -w -ffreestanding -triple x86_64-scei-ps4 -target-feature +avx -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-LEGACY --check-prefix=AVX-LEGACY
+
 // This tests verifies that a union parameter should pass by a vector regitster whose first eightbyte is SSE and the other eightbytes are SSEUP.
 
 typedef int __m256 __attribute__ ((__vector_size__ (32)));
 typedef int __m512 __attribute__ ((__vector_size__ (64)));
 
 union M256 {
   double d;
   __m256 m;
 };
 
 union M512 {
   double d;
   __m512 m;
 };
 
 extern void foo1(union M256 A);
 extern void foo2(union M512 A);
 union M256 m1;
 union M512 m2;
-// CHECK-LABEL: define void @test()
-// CHECK:       call void @foo1(<4 x double>
-// AVX:         call void @foo2(%union.M512* byval(%union.M512) align 64
-// AVX512:      call void @foo2(<8 x double>
+// CHECK-LABEL:   define void @test()
+// CHECK:         call void @foo1(<4 x double>
+// CHECK-LEGACY:  call void @foo1(%union.M256* byval(%union.M256) align 32
+// AVX:           call void @foo2(%union.M512* byval(%union.M512) align 64
+// AVX512:        call void @foo2(<8 x double>
+// AVX512-LEGACY: call void @foo2(%union.M512* byval(%union.M512) align 64
 void test() {
   foo1(m1);
   foo2(m2);
 }