[PATCH] D46767: Force the PS4 clang ABI version to 6, and add a warning if this is attempted to be overridden

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 11 17:31:59 PDT 2018


rsmith added a comment.

Everything old is new again. This was discussed when `-fclang-abi-compat` was introduced; see https://reviews.llvm.org/D36501 for the argument why this patch is the wrong way of modeling an ABI. Forcing the `ClangABICompat` language option as a way of "tricking" Clang into producing the PS4 ABI is a hack. The various ABI changes that `-fclang-abi-compat=` controls are simply part of the PS4 ABI, and that knowledge should idealistically be carried by the CodeGen (etc) code that knows about PS4, rather than by imagining that there is some other PS4 ABI that Clang would produces at version `Latest`, and that we're asking for a compatibility version of it.

That said, pragmatically speaking, that approach isn't working out well. We have systematically forgotten to special-case PS4 when adding ABI compatibility features, so I think I'm convinced that this hack is better than the status quo. This will go wrong if we ever release (or have ever released) a Clang version that fails to properly implement the PS4 ABI. In such a case, `-fclang-abi-compat` should be usable to request that we emulate past ABI bugs, but would actually have no effect on PS4. I think it's OK to cross that bridge if/when we come to it.

However, we should not issue a warning for use of the flag. Remember that the flag means "please be ABI-compatible with Clang version X.Y". Because all versions of Clang that target PS4 use the same ABI, the flag is a no-op on that target (at least for now, until we accidentally introduce an ABI break). So we should not be warning on it, just silently accepting it and doing what it says -- which for now is nothing.



================
Comment at: lib/Frontend/CompilerInvocation.cpp:2762
+    }
+    Opts.setClangABICompat(LangOptions::ClangABI::Ver6);
+  }
----------------
Is `Ver6` really the right setting? When determining whether to pass objects of class type indirect, PS4 uses the Clang <= 4 rule, when determining how to pass a vector of 1 long long, it uses the Clang <= 3.8 rule, and so on. In SemaDeclCXX.cpp (in `paramCanBeDestoyedInCallee`), I found this comment:

      // The PS4 platform ABI follows the behavior of Clang 3.2.

So I *suspect* you should actually be setting this to `Ver3_8` (which is the oldest version we provide a compatibility flag for).


Repository:
  rC Clang

https://reviews.llvm.org/D46767





More information about the cfe-commits mailing list