Support -fclang-abi-compat=8.0 to keep old ABI behavior

[wxiao3]

Summary

The System V i386 bug fix (https://reviews.llvm.org/D59744) makes it impossible
for 32-bit Linux Chromium to write an assembly function that works with both
trunk clang and clang 8.0.0, which makes it difficult to update compilers
independent of changing the code (more details:
https://bugs.chromium.org/p/chromium/issues/detail?id=974542#c5).

This patch aims to provide a solution for such situation.

[compnerd]

Could you please add a test to ensure that Darwin defaults to the old behaviour?

include/clang/Basic/LangOptions.h

142

Referencing the SVN revision is better IMO as the infrastructure can change, but, the reference to the commit is useful as that can be mapped.

[wxiao3]

Yes, there is a test to ensure that Darwin defaults to the old behaviour in "test/CodeGen/x86_32-m64.c".

[RKSimon]

test/CodeGen/x86_32-m64.c

6

Use ABI80 instead of OLDABI?

[hans]

The System V i386 bug fix (https://reviews.llvm.org/D59744) makes it impossible

Please refer to the svn revision instead of the code review.

for 32-bit Linux Chromium to write an assembly function that works with both
trunk clang and clang 8.0.0, which makes it difficult to update compilers
independent of changing the code (more details:
https://bugs.chromium.org/p/chromium/issues/detail?id=974542#c5).

We still don't understand the Chromium problem so we should probably wait until that's done, and then the commit message should be more general. (For Chromium, we will not want to lock to an old ABI version but rather do whatever fixing is required.)

This patch aims to provide a solution for such situation.

docs/ReleaseNotes.rst

146

Instead of "historically", please spell out exactly what versions, i.e. versions prior to Clang 9.

151

Just refer to the major version: `-fclang-abi-compat=8`

include/clang/Basic/LangOptions.h

142

Isn't the comment backwards? Setting Ver8 causes __m64 *not* to be passed in an MMX register?

lib/CodeGen/TargetInfo.cpp

1091

What about Clang 8.0.1? :-) Probably better to say "Clang 8 and previous versions did not do this."

test/CodeGen/x86_32-m64.c

6

Please pass -fclang-abi-compat=8 instead to match how the flag is used otherwhere.

[thakis]

hans and compnerd got this, no need for so many reviewers :)

docs/ReleaseNotes.rst

 OpenCL C Language Changes in Clang
 ----------------------------------
 
 ...
 
 ABI Changes in Clang
 --------------------
 
+- The System V i386 psABI requires __m64 to be passed in MMX registers.
+  Clang historically had a bug where it failed to apply this rule, and
+  some platforms (e.g. Darwin, PS4, and FreeBSD) have opted to maintain
+  compatibility with the old Clang behavior, so we only apply it on
+  platforms that have specifically requested it (currently just Linux and
+  NetBSD). You can switch back to old API behavior with flag:
+  -fclang-abi-compat=8.0.
+
 - ...
 
 OpenMP Support in Clang
 -----------------------
 
 - Added emission of the debug information for NVPTX target devices.
 
 CUDA Support in Clang

include/clang/Basic/LangOptions.h

enum class ClangABI {
     Ver6,
 
     /// Attempt to be ABI-compatible with code generated by Clang 7.0.x
     /// (SVN r338536). This causes alignof (C++) and _Alignof (C11) to be
     /// compatible with __alignof (i.e., return the preferred alignment)
     /// rather than returning the required alignment.
     Ver7,
 
+    /// Attempt to be ABI-compatible with code generated by Clang 8.0.x
+    /// (https://reviews.llvm.org/D59744). This causes __m64 to be passed in
+    /// MMX register instead of integer register.
+    Ver8,
+
     /// 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,

lib/CodeGen/TargetInfo.cpp

public:
   }
 
   bool isSwiftErrorInRegister() const override {
     // x86-32 lowering does not support passing swifterror in a register.
     return false;
   }
 
   bool isPassInMMXRegABI() const {
+    // Clang <= 8.0 did not do this for compatiblity with old behavior.
+    if (getContext().getLangOpts().getClangABICompat() <=
+        LangOptions::ClangABI::Ver8)
+      return false;
+
     // The System V i386 psABI requires __m64 to be passed in MMX registers.
     // Clang historically had a bug where it failed to apply this rule, and
     // some platforms (e.g. Darwin, PS4, and FreeBSD) have opted to maintain
     // compatibility with the old Clang behavior, so we only apply it on
     // platforms that have specifically requested it (currently just Linux and
     // NetBSD).
     const llvm::Triple &T = getTarget().getTriple();
     if (IsMMXEnabled && (T.isOSLinux() || T.isOSNetBSD()))

lib/Frontend/CompilerInvocation.cpp

if (!VerParts.first.startswith("0") &&
       if (Major == 3 && Minor <= 8)
         Opts.setClangABICompat(LangOptions::ClangABI::Ver3_8);
       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 <= 8)
+        Opts.setClangABICompat(LangOptions::ClangABI::Ver8);
     } 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);

test/CodeGen/x86_32-m64.c

 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-netbsd -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,NETBSD
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -fclang-abi-compat=8.0 -o - %s | FileCheck %s --check-prefixes=CHECK,OLDABI
 
 #include <mmintrin.h>
 __m64 m64;
 void callee(__m64 __m1, __m64 __m2);
 __m64 caller(__m64 __m1, __m64 __m2)
 {
 // LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
 // LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
 // LINUX: ret x86_mmx
 // NETBSD-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
 // NETBSD: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
 // NETBSD: ret x86_mmx
 // DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
 // DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
 // DARWIN: ret i64
 // IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
 // IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
 // IAMCU: ret <1 x i64>
 // WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
 // WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
 // WIN32: ret <1 x i64>
+// OLDABI: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// OLDABI: ret <1 x i64>
   callee(__m2, __m1);
   return m64;
 }