[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith closed this revision.
rsmith added a comment.

Committed as 311823.


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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

Sorry it took so long to wrap my head around this, but it has been about 20 
years since I've had to accommodate an intentional ABI breakage, and that's 
actually what you want to do.  Remembering that case, I have a model for this 
one, and I understand what you are doing now.

Be that as it may, `-fclang-abi-compat` is meaningless on PS4, because every 
time there's a case for checking ClangABICompat, PS4 will want to do it the 3.2 
way regardless.  And we'll just make those checks in all those places.  At 
least it will be relatively easy to audit, and I'm sure our licensees will be 
happy to ignore the new flag.

So, I am convinced, and the patch LGTM.


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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Paul Robinson via Phabricator via cfe-commits
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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Paul Robinson via Phabricator via cfe-commits
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.

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


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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In https://reviews.llvm.org/D36501#852864, @rjmccall wrote:

> The only thing I would say here is that, as a platform vendor, I would like 
> the ability to opt in to ABI fixes that aren't in my base-line version.  In 
> particular, we're generally more aggressive about taking C++ ABI fixes on 
> Darwin when there's a strong argument that the impact is likely to be low, 
> like for some of these indirect-ABI cases that only make a difference for 
> classes with e.g. only deleted copy/move constructors but a trivial 
> destructor.


I don't think the idea of a base-line version matches with the direction of the 
patch, and I've removed the part of the patch that has a base-line Clang 
version for PS4. I think that if your platform ABI dictates that you do thing X 
differently from, say, the x86_64 psABI or the Itanium C++ ABI, then that's 
simply part of your platform ABI, and not a clang compatibility feature, 
regardless of whether the historical reason your platform ABI made that choice 
was due to clang compatibility.


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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 112733.

Repository:
  rL LLVM

https://reviews.llvm.org/D36501

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/ABIInfo.h
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/CodeGenTypes.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/clang-abi-compat.cpp
  test/CodeGenCXX/uncopyable-args.cpp
  test/Driver/flags.c
  test/Frontend/clang-abi-compat.cpp

Index: test/Frontend/clang-abi-compat.cpp
===
--- test/Frontend/clang-abi-compat.cpp
+++ test/Frontend/clang-abi-compat.cpp
@@ -0,0 +1,15 @@
+// RUN: not %clang_cc1 -fclang-abi-compat=banana %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=2.9 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=8 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=3.10 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=4.1 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=04 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=4. %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=4.00 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// INVALID: error: invalid value '{{.*}}' in '-fclang-abi-compat={{.*}}'
+//
+// RUN: %clang_cc1 -fclang-abi-compat=3.0 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=3.9 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=4 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=4.0 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=latest %s -fsyntax-only
Index: test/Driver/flags.c
===
--- test/Driver/flags.c
+++ test/Driver/flags.c
@@ -24,3 +24,6 @@
 
 // RUN: %clang -target armv7-apple-darwin10 -### -S -mno-implicit-float -mimplicit-float %s 2>&1 | FileCheck -check-prefix=TEST8 %s
 // TEST8-NOT: "-no-implicit-float"
+
+// RUN: %clang -target x86_64-linux-gnu -### -c -fclang-abi-compat=3.2 %s 2>&1 | FileCheck -check-prefix=TEST9 %s
+// TEST9: "-fclang-abi-compat=3.2"
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=NEWABI
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -fclang-abi-compat=4.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=OLDABI
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-scei-ps4 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=OLDABI
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=18 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-18
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=19 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-19
 
@@ -56,8 +58,10 @@
 // CHECK-LABEL: define void @_ZN9move_ctor3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
 // CHECK-NOT: call
-// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// NEWABI: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// OLDABI: call void @_ZN9move_ctor3fooENS_1AE(i8* %{{.*}})
+// NEWABI-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// OLDABI-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(i8*)
 
 // WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
 }
@@ -76,8 +80,10 @@
 // CHECK-LABEL: define void @_ZN11all_deleted3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
 // CHECK-NOT: call
-// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// NEWABI: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// OLDABI: call void @_ZN11all_deleted3fooENS_1AE(i8* %{{.*}})
+// NEWABI-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// OLDABI-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(i8*)
 
 // WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
 }
@@ -95,8 

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

The only thing I would say here is that, as a platform vendor, I would like the 
ability to opt in to ABI fixes that aren't in my base-line version.  In 
particular, we're generally more aggressive about taking C++ ABI fixes on 
Darwin when there's a strong argument that the impact is likely to be low, like 
for some of these indirect-ABI cases that only make a difference for classes 
with e.g. only deleted copy/move constructors but a trivial destructor.


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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In https://reviews.llvm.org/D36501#852765, @probinson wrote:

> I would prefer a diagnostic if PS4 && >3.2.  Whether you map "latest" to 3.2 
> or diagnose that also, up to you, I'm okay either way.


On reflection, I don't think doing anything special for PS4 here is really 
correct. (Suppose Clang 5 has some (accidental) ABI change in it for PS4, and 
we fix that in Clang 6 to once again follow the psABI and do what Clang 3.2 
did. In that case, building with `-fclang-abi-compat=5` should theoretically 
reinstate that accidental ABI change.) The old-Clang compatibility mode and the 
platform ABI are separate, and we should not conflate them. New patch on the 
way.


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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

I would prefer a diagnostic if PS4 && >3.2.  Whether you map "latest" to 3.2 or 
diagnose that also, up to you, I'm okay either way.


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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-24 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

I'm not familiar with the details of the ABI changes, but the mechanics of the 
code look good to me.


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


[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-24 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 112634.
rsmith retitled this revision from "add flag to undo ABI change in r310401" to 
"Add flag to request Clang is ABI-compatible with older versions of itself".
rsmith edited the summary of this revision.
Herald added a subscriber: srhines.

Repository:
  rL LLVM

https://reviews.llvm.org/D36501

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/ABIInfo.h
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/CodeGenTypes.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/clang-abi-compat.cpp
  test/CodeGenCXX/uncopyable-args.cpp
  test/Driver/flags.c
  test/Frontend/clang-abi-compat.cpp

Index: test/Frontend/clang-abi-compat.cpp
===
--- test/Frontend/clang-abi-compat.cpp
+++ test/Frontend/clang-abi-compat.cpp
@@ -0,0 +1,15 @@
+// RUN: not %clang_cc1 -fclang-abi-compat=banana %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=2.9 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=8 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=3.10 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=4.1 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=04 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=4. %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: not %clang_cc1 -fclang-abi-compat=4.00 %s -fsyntax-only 2>&1 | FileCheck --check-prefix=INVALID %s
+// INVALID: error: invalid value '{{.*}}' in '-fclang-abi-compat={{.*}}'
+//
+// RUN: %clang_cc1 -fclang-abi-compat=3.0 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=3.9 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=4 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=4.0 %s -fsyntax-only
+// RUN: %clang_cc1 -fclang-abi-compat=latest %s -fsyntax-only
Index: test/Driver/flags.c
===
--- test/Driver/flags.c
+++ test/Driver/flags.c
@@ -24,3 +24,6 @@
 
 // RUN: %clang -target armv7-apple-darwin10 -### -S -mno-implicit-float -mimplicit-float %s 2>&1 | FileCheck -check-prefix=TEST8 %s
 // TEST8-NOT: "-no-implicit-float"
+
+// RUN: %clang -target x86_64-linux-gnu -### -c -fclang-abi-compat=3.2 %s 2>&1 | FileCheck -check-prefix=TEST9 %s
+// TEST9: "-fclang-abi-compat=3.2"
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=NEWABI
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -fclang-abi-compat=4.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=OLDABI
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-scei-ps4 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=OLDABI
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=18 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-18
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=19 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-19
 
@@ -56,8 +58,10 @@
 // CHECK-LABEL: define void @_ZN9move_ctor3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
 // CHECK-NOT: call
-// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// NEWABI: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// OLDABI: call void @_ZN9move_ctor3fooENS_1AE(i8* %{{.*}})
+// NEWABI-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// OLDABI-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(i8*)
 
 // WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
 }
@@ -76,8 +80,10 @@
 // CHECK-LABEL: define void @_ZN11all_deleted3barEv()
 // CHECK: call void @_Z{{.*}}C1Ev(
 // CHECK-NOT: call
-// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// NEWABI: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// OLDABI: call void @_ZN11all_deleted3fooENS_1AE(i8* %{{.*}})
+// NEWABI-LABEL: declare