[PATCH] D132444: [clang] Allow using -rtlib=platform to switching to the default rtlib on all targets

2022-08-24 Thread Martin Storsjö via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0a4c6c9f98a6: [clang] Allow using -rtlib=platform to 
switching to the default rtlib on all… (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132444/new/

https://reviews.llvm.org/D132444

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/msvc-compiler-rt.c
  clang/test/Driver/rtlib-darwin.c


Index: clang/test/Driver/rtlib-darwin.c
===
--- /dev/null
+++ clang/test/Driver/rtlib-darwin.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-apple-darwin 
-resource-dir=%S/Inputs/resource_dir --rtlib=compiler-rt -### %s 2>&1 | 
FileCheck %s -check-prefix DARWIN-COMPILER-RT
+// RUN: %clang -target x86_64-apple-darwin 
-resource-dir=%S/Inputs/resource_dir --rtlib=platform -### %s 2>&1 | FileCheck 
%s -check-prefix DARWIN-COMPILER-RT
+// RUN: not %clang %s -target x86_64-apple-darwin --rtlib=libgcc 2>&1 | 
FileCheck %s -check-prefix CHECK-ERROR
+
+// DARWIN-COMPILER-RT: "{{.*}}clang_rt.osx{{.*}}"
+// CHECK-ERROR: unsupported runtime library 'libgcc' for platform 'darwin'
Index: clang/test/Driver/msvc-compiler-rt.c
===
--- clang/test/Driver/msvc-compiler-rt.c
+++ clang/test/Driver/msvc-compiler-rt.c
@@ -1,5 +1,7 @@
 // RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt -### %s 2>&1 
| FileCheck %s -check-prefix MSVC-COMPILER-RT
+// RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt 
--rtlib=platform -### %s 2>&1 | FileCheck %s -check-prefix MSVC-DEFAULT
 // RUN: not %clang %s -target x86_64-pc-windows-msvc --rtlib=libgcc 2>&1 | 
FileCheck %s -check-prefix CHECK-ERROR
 
 // MSVC-COMPILER-RT: "{{.*}}clang_rt.builtins{{.*}}"
+// MSVC-DEFAULT-NOT: "{{.*}}clang_rt.builtins{{.*}}"
 // CHECK-ERROR: unsupported runtime library 'libgcc' for platform 'MSVC'
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1385,7 +1385,7 @@
 const ArgList ) const {
   if (Arg* A = Args.getLastArg(options::OPT_rtlib_EQ)) {
 StringRef Value = A->getValue();
-if (Value != "compiler-rt")
+if (Value != "compiler-rt" && Value != "platform")
   getDriver().Diag(clang::diag::err_drv_unsupported_rtlib_for_platform)
   << Value << "darwin";
   }
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1602,9 +1602,10 @@
 if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
   // Issue error diagnostic if libgcc is explicitly specified
   // through command line as --rtlib option argument.
-  if (Args.hasArg(options::OPT_rtlib_EQ)) {
+  Arg *A = Args.getLastArg(options::OPT_rtlib_EQ);
+  if (A && A->getValue() != StringRef("platform")) {
 TC.getDriver().Diag(diag::err_drv_unsupported_rtlib_for_platform)
-<< Args.getLastArg(options::OPT_rtlib_EQ)->getValue() << "MSVC";
+<< A->getValue() << "MSVC";
   }
 } else
   AddLibgcc(TC, D, CmdArgs, Args);


Index: clang/test/Driver/rtlib-darwin.c
===
--- /dev/null
+++ clang/test/Driver/rtlib-darwin.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-apple-darwin -resource-dir=%S/Inputs/resource_dir --rtlib=compiler-rt -### %s 2>&1 | FileCheck %s -check-prefix DARWIN-COMPILER-RT
+// RUN: %clang -target x86_64-apple-darwin -resource-dir=%S/Inputs/resource_dir --rtlib=platform -### %s 2>&1 | FileCheck %s -check-prefix DARWIN-COMPILER-RT
+// RUN: not %clang %s -target x86_64-apple-darwin --rtlib=libgcc 2>&1 | FileCheck %s -check-prefix CHECK-ERROR
+
+// DARWIN-COMPILER-RT: "{{.*}}clang_rt.osx{{.*}}"
+// CHECK-ERROR: unsupported runtime library 'libgcc' for platform 'darwin'
Index: clang/test/Driver/msvc-compiler-rt.c
===
--- clang/test/Driver/msvc-compiler-rt.c
+++ clang/test/Driver/msvc-compiler-rt.c
@@ -1,5 +1,7 @@
 // RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt -### %s 2>&1 | FileCheck %s -check-prefix MSVC-COMPILER-RT
+// RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt --rtlib=platform -### %s 2>&1 | FileCheck %s -check-prefix MSVC-DEFAULT
 // RUN: not %clang %s -target x86_64-pc-windows-msvc --rtlib=libgcc 2>&1 | FileCheck %s -check-prefix CHECK-ERROR
 
 // MSVC-COMPILER-RT: "{{.*}}clang_rt.builtins{{.*}}"
+// MSVC-DEFAULT-NOT: "{{.*}}clang_rt.builtins{{.*}}"
 // CHECK-ERROR: unsupported runtime library 'libgcc' for platform 'MSVC'
Index: clang/lib/Driver/ToolChains/Darwin.cpp

[PATCH] D132444: [clang] Allow using -rtlib=platform to switching to the default rtlib on all targets

2022-08-24 Thread Petr Hosek via Phabricator via cfe-commits 
phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM

In D132444#3743051 , @mstorsjo wrote:

> In D132444#3743020 , @thakis wrote:
>
>> Makes sense to me. Maybe @hans has an opinion too.
>>
>> WDYT about accepting the same string for -fuse-ld= to mean "platform linker" 
>> there as well?
>
> Sounds reasonable to me - requiring the user to set an option to the empty 
> string is kinda awkward.

We currently support `-fuse-ld=` and `-fuse-ld=ld`, see 
https://github.com/llvm/llvm-project/blob/e29f9f7572d75c25cf06b080dce6bda9ebcf5008/clang/lib/Driver/ToolChain.cpp#L644.
 I think that `-fuse-ld=platform` would be less confusing (to distinguish from 
`ld` as the linker name) and it's also more consistent with other flags.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132444/new/

https://reviews.llvm.org/D132444

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132444: [clang] Allow using -rtlib=platform to switching to the default rtlib on all targets

2022-08-23 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo added a comment.

In D132444#3743020 , @thakis wrote:

> Makes sense to me. Maybe @hans has an opinion too.
>
> WDYT about accepting the same string for -fuse-ld= to mean "platform linker" 
> there as well?

Sounds reasonable to me - requiring the user to set an option to the empty 
string is kinda awkward.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132444/new/

https://reviews.llvm.org/D132444

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132444: [clang] Allow using -rtlib=platform to switching to the default rtlib on all targets

2022-08-23 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Makes sense to me. Maybe @hans has an opinion too.

WDYT about accepting the same string for -fuse-ld= to mean "platform linker" 
there as well?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132444/new/

https://reviews.llvm.org/D132444

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132444: [clang] Allow using -rtlib=platform to switching to the default rtlib on all targets

2022-08-23 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo added a comment.

In D132444#3742305 , @mstorsjo wrote:

> In D132444#3742295 , @thakis wrote:
>
>> Do we have precedent for "platform" for this? For fuse-ld=, one is supposed 
>> to use `-fuse-ld=` (without anything after the `=`) to get the default ld. 
>> That's not great (...but it can't collide with actual linker names, i 
>> suppose).
>>
>> Using "platform" (or any other self-descriptive name) for this seems easier 
>> to understand than passing an empty value. But it'd be nice if we could use 
>> this consistently in our various flags.
>
> Yes, `"platform"` is an existing option handled in a bunch of places already 
> - see e.g. 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChain.cpp#L832-L838.
>  It's just that these cases hadn't been updated to take it into account.

Then again, the quoted code comment says that `"platform"` only is meant to be 
used for overriding `CLANG_DEFAULT_RTLIB` in tests, but I don't see why one 
can't use it for overriding `CLANG_DEFAULT_RTLIB` (or an earlier `-rtlib` 
option on the command line) in real world uses too. (I had a user wanting to 
use my builds of clang for MSVC use cases, where it failed due to my Clang 
defaulting to `-rtlib=compiler-rt`, with no way of overriding it back to 
default, see https://github.com/mstorsjo/llvm-mingw/issues/267)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132444/new/

https://reviews.llvm.org/D132444

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132444: [clang] Allow using -rtlib=platform to switching to the default rtlib on all targets

2022-08-23 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo added a comment.

In D132444#3742295 , @thakis wrote:

> Do we have precedent for "platform" for this? For fuse-ld=, one is supposed 
> to use `-fuse-ld=` (without anything after the `=`) to get the default ld. 
> That's not great (...but it can't collide with actual linker names, i 
> suppose).
>
> Using "platform" (or any other self-descriptive name) for this seems easier 
> to understand than passing an empty value. But it'd be nice if we could use 
> this consistently in our various flags.

Yes, `"platform"` is an existing option handled in a bunch of places already - 
see e.g. 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChain.cpp#L832-L838.
 It's just that these cases hadn't been updated to take it into account.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132444/new/

https://reviews.llvm.org/D132444

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132444: [clang] Allow using -rtlib=platform to switching to the default rtlib on all targets

2022-08-23 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Do we have precedent for "platform" for this? For fuse-ld=, one is supposed to 
use `-fuse-ld=` (without anything after the `=`) to get the default ld. That's 
not great (...but it can't collide with actual linker names, i suppose).

Using "platform" (or any other self-descriptive name) for this seems easier to 
understand than passing an empty value. But it'd be nice if we could use this 
consistently in our various flags.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132444/new/

https://reviews.llvm.org/D132444

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132444: [clang] Allow using -rtlib=platform to switching to the default rtlib on all targets

2022-08-23 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo created this revision.
mstorsjo added reviewers: phosek, thakis, hans.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: clang.

Normally, passing -rtlib=platform overrides any earlier -rtlib
options, and overrides any hardcoded CLANG_DEFAULT_RTLIB option.
However, some targets, MSVC and Darwin, have custom logic for
disallowing specific -rtlib= option values; amend these checks for
allowing the -rtlib=platform option.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132444

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/msvc-compiler-rt.c
  clang/test/Driver/rtlib-darwin.c


Index: clang/test/Driver/rtlib-darwin.c
===
--- /dev/null
+++ clang/test/Driver/rtlib-darwin.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-apple-darwin 
-resource-dir=%S/Inputs/resource_dir --rtlib=compiler-rt -### %s 2>&1 | 
FileCheck %s -check-prefix DARWIN-COMPILER-RT
+// RUN: %clang -target x86_64-apple-darwin 
-resource-dir=%S/Inputs/resource_dir --rtlib=platform -### %s 2>&1 | FileCheck 
%s -check-prefix DARWIN-COMPILER-RT
+// RUN: not %clang %s -target x86_64-apple-darwin --rtlib=libgcc 2>&1 | 
FileCheck %s -check-prefix CHECK-ERROR
+
+// DARWIN-COMPILER-RT: "{{.*}}clang_rt.osx{{.*}}"
+// CHECK-ERROR: unsupported runtime library 'libgcc' for platform 'darwin'
Index: clang/test/Driver/msvc-compiler-rt.c
===
--- clang/test/Driver/msvc-compiler-rt.c
+++ clang/test/Driver/msvc-compiler-rt.c
@@ -1,5 +1,7 @@
 // RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt -### %s 2>&1 
| FileCheck %s -check-prefix MSVC-COMPILER-RT
+// RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt 
--rtlib=platform -### %s 2>&1 | FileCheck %s -check-prefix MSVC-DEFAULT
 // RUN: not %clang %s -target x86_64-pc-windows-msvc --rtlib=libgcc 2>&1 | 
FileCheck %s -check-prefix CHECK-ERROR
 
 // MSVC-COMPILER-RT: "{{.*}}clang_rt.builtins{{.*}}"
+// MSVC-DEFAULT-NOT: "{{.*}}clang_rt.builtins{{.*}}"
 // CHECK-ERROR: unsupported runtime library 'libgcc' for platform 'MSVC'
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1385,7 +1385,7 @@
 const ArgList ) const {
   if (Arg* A = Args.getLastArg(options::OPT_rtlib_EQ)) {
 StringRef Value = A->getValue();
-if (Value != "compiler-rt")
+if (Value != "compiler-rt" && Value != "platform")
   getDriver().Diag(clang::diag::err_drv_unsupported_rtlib_for_platform)
   << Value << "darwin";
   }
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1584,9 +1584,10 @@
 if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
   // Issue error diagnostic if libgcc is explicitly specified
   // through command line as --rtlib option argument.
-  if (Args.hasArg(options::OPT_rtlib_EQ)) {
+  Arg *A = Args.getLastArg(options::OPT_rtlib_EQ);
+  if (A && A->getValue() != StringRef("platform")) {
 TC.getDriver().Diag(diag::err_drv_unsupported_rtlib_for_platform)
-<< Args.getLastArg(options::OPT_rtlib_EQ)->getValue() << "MSVC";
+<< A->getValue() << "MSVC";
   }
 } else
   AddLibgcc(TC, D, CmdArgs, Args);


Index: clang/test/Driver/rtlib-darwin.c
===
--- /dev/null
+++ clang/test/Driver/rtlib-darwin.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-apple-darwin -resource-dir=%S/Inputs/resource_dir --rtlib=compiler-rt -### %s 2>&1 | FileCheck %s -check-prefix DARWIN-COMPILER-RT
+// RUN: %clang -target x86_64-apple-darwin -resource-dir=%S/Inputs/resource_dir --rtlib=platform -### %s 2>&1 | FileCheck %s -check-prefix DARWIN-COMPILER-RT
+// RUN: not %clang %s -target x86_64-apple-darwin --rtlib=libgcc 2>&1 | FileCheck %s -check-prefix CHECK-ERROR
+
+// DARWIN-COMPILER-RT: "{{.*}}clang_rt.osx{{.*}}"
+// CHECK-ERROR: unsupported runtime library 'libgcc' for platform 'darwin'
Index: clang/test/Driver/msvc-compiler-rt.c
===
--- clang/test/Driver/msvc-compiler-rt.c
+++ clang/test/Driver/msvc-compiler-rt.c
@@ -1,5 +1,7 @@
 // RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt -### %s 2>&1 | FileCheck %s -check-prefix MSVC-COMPILER-RT
+// RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt --rtlib=platform -### %s 2>&1 | FileCheck %s -check-prefix MSVC-DEFAULT
 // RUN: not %clang %s -target x86_64-pc-windows-msvc --rtlib=libgcc 2>&1 | FileCheck %s -check-prefix CHECK-ERROR