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:
> 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?
`-fclang-abi-compat` requests that we generate code that is ABI-compatible with
a prior Clang version. The use case (for *any* platform) is that you have some
code built with an earlier version of Clang that had a bug in its
implementation of the platform ABI, and you want to generate more code that is
ABI-compatible with it.
`-fclang-abi-compat=4`, for instance, has an effect on Darwin: it turns off the
recent bug fix to the calling convention for passing
trivially-copyable-but-not-trivially-moveable C++ class types. (But it does
nothing on PS4, because -- with this patch -- Clang 5 does not change that
aspect of the ABI for PS4.)
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
