rsmith added inline comments.
================
Comment at: lib/Driver/ToolChains/Clang.cpp:2934
+ ABICompatArg->render(Args, CmdArgs);
+
// Add runtime flag for PS4 when PGO or Coverage are enabled.
----------------
probinson wrote:
> ```
> else if (getToolChain().getTriple().isPS4())
> CmdArgs.push_back("-fclang-abi-compat=3.2");
> ```
>
> Which lets us avoid piles of PS4 special cases all over everywhere.
> Sony is historically very conservative about compatibility, and we'll be
> happier defaulting it this way. Setting platform-specific defaults in the
> driver seems to be pretty common already, this is just one more.
I initially thought that made sense too, but I'm now fairly convinced that it
doesn't.
Let's take Darwin as an example. There are some facets of Darwin's platform ABI
that are determined by what old versions of Clang did, and other facets of its
platform ABI that have changed to match changes to the x86_64 psABI and Itanium
C++ ABI. But it's irrelevant where the platform ABI rules come from; the point
is that there is some set of platform ABI rules that you could write down, and
Clang attempts to implement those rules correctly by default.
The flag being added in this patch provides the ability to request that Clang
does something else: that it produces code that is ABI-compatible with what a
prior version of itself did when targeting that platform ABI. In particular, we
fixed the C++ calling convention for certain rare class types in Clang 5 to
conform to (an update to) the Itanium C++ ABI rules, and this patch allows that
to be undone.
It seems to me that the situation for PS4 is exactly the same. There is some
platform ABI that you could write down, and Clang attempts to be compatible
with that by default. And it's irrelevant whether that's the ABI that Clang 3.2
used or the ABI of GCC 2.95; it's just the platform ABI. This is not a "be
compatible with clang 3.2" mode, it is (as far as I can tell) the actual
platform ABI.
Let me repeat an example I gave before: suppose Clang 5 has some (accidental)
ABI change in it for PS4, and we fix that in Clang 6 to once again follow the
platform ABI by doing what Clang 3.2 did. In that case, building with
`-fclang-abi-compat=5` should theoretically reinstate that accidental ABI
change.
Hopefully that should clarify that this does *not* actually let us avoid PS4
special cases anywhere. ABI choices depend on both the platform ABI *and* on
the version of Clang that we're providing compatibility with (if any).
That said, it's clearly not up to me what the PS4 platform ABI is. If you want
to say that the PS4 platform ABI is actually something other than what Clang
3.2 does, but all object code on your system is compiled in a compatibility
mode that diverges from the platform ABI and matches Clang 3.2, then I would
concede that we can remove the PS4 platform special cases elsewhere and set a
default here. But that sounds like a very strange decision to make, and it
creates a horrible problem for the meaning of the `-fclang-abi-compat` flag: if
someone in the future specifies `-fclang-abi-compat=5` when targeting PS4, and
Clang 5 by default set `-fclang-abi-compat=3.2`, then are we targeting what
Clang 5 would have done by default or what Clang 5 would have done when told to
be compatible with itself? As you can see, this default would create a lot of
confusion.
Repository:
rL LLVM
https://reviews.llvm.org/D36501
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
