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

Mon May 14 11:10:43 PDT 2018

probinson added a comment.

In https://reviews.llvm.org/D46767#1096746, @rsmith wrote:

> Everything old is new again.

If only that were true about my brain. :-P

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

Muchas gracias for the reminder of the previous discussion; it's quite true that on our side we have not done our due diligence in making sure that upstream Clang fully supports the PS4 ABI, and that `-fclang-abi-compat` is the wrong way to do this.  It needs to become part of my team's consciousness and collective memory that these sorts of expedient hacks are the wrong approach.

> This will go wrong if we ever release (or have ever released) a Clang version that fails to properly implement the PS4 ABI.

Yeah, we crossed that bridge years ago, but nobody has been brave enough to try to deliver that patch upstream.  Actually I think there are two, but as they typically don't cause any merge conflicts they're not at top-of-mind for anybody, even me.

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

Got it.  I'll take an action to straighten this one out.

Repository:
  rC Clang

https://reviews.llvm.org/D46767