[PATCH] D63473: Support -fclang-abi-compat=8.0 to keep old ABI behavior

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

> 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.



================
Comment at: docs/ReleaseNotes.rst:146
+- 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
----------------
Instead of "historically", please spell out exactly what versions, i.e. versions prior to Clang 9.


================
Comment at: docs/ReleaseNotes.rst:151
+  NetBSD). You can switch back to old API behavior with flag:
+  -fclang-abi-compat=8.0.
+
----------------
Just refer to the major version: ``-fclang-abi-compat=8``


================
Comment at: include/clang/Basic/LangOptions.h:142
+    /// Attempt to be ABI-compatible with code generated by Clang 8.0.x
+    /// (SVN r363116). This causes __m64 to be passed in MMX register 
+    /// instead of integer register.
----------------
Isn't the comment backwards? Setting Ver8 causes __m64 *not* to be passed in an MMX register?


================
Comment at: lib/CodeGen/TargetInfo.cpp:1091
   bool isPassInMMXRegABI() const {
+    // Clang <= 8.0 did not do this for compatiblity with old behavior.
+    if (getContext().getLangOpts().getClangABICompat() <=
----------------
What about Clang 8.0.1? :-) Probably better to say "Clang 8 and previous versions did not do this."


================
Comment at: test/CodeGen/x86_32-m64.c:6
 // 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
 
----------------
RKSimon wrote:
> Use ABI80 instead of OLDABI?
Please pass -fclang-abi-compat=8 instead to match how the flag is used otherwhere.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63473/new/

https://reviews.llvm.org/D63473