probinson 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.
----------------
rsmith wrote:
> 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.
On the other hand, if all the places that check ClangABICompat also check for
PS4 and Darwin, then specifying `-fclang-abi-compat` while targeting PS4 or
Darwin has no effect, and also no diagnostic. Which seems to make
`-fclang-abi-compat` totally pointless. Is there a non-PS4/Darwin use-case for
this flag?
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
