Re: [PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via cfe-commits]

On Sat, Mar 13, 2021 at 9:36 AM Eric Christopher  wrote:

>
>
> On Fri, Mar 5, 2021 at 1:15 AM Siva Chandra via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> sivachandra added a comment.
>>
>> In D97736#2605535 , @phosek
>> wrote:
>>
>> > Have you considered using an input linker script? We could generate
>> `libc.so` that could look something like:
>> >
>> >   INPUT(libllvmlibc.a /lib/libc.so)
>> >
>> > We would need to pass `--sysroot` to the linker for this to work. The
>> driver could remain completely agnostic of whether you're using LLVM libc
>> or not.
>>
>> Yes, that was also considered. Those downstream users who have the
>> flexibility to do it that way should be able to do it that way. However,
>> not all downstream users or normal clang users will have that liberty [1].
>> Another point to note is that we will have to do this with all libc
>> components like `libc.so`, `libm.so` etc.
>>
>> [1] I think all of this can be done. For example, we can set all this up
>> when building a distribution. However, I am not sure this is worth it when
>> we know this is a transient phase. Soon, when LLVM libc is complete enough,
>> a more appropriate option would be the one which allows choosing a libc as
>> Eric pointed out.
>>
>
> To be clear I'm not a fan of a "pick your libc" option as opposed to just
> naming the compiled llvm libc as perhaps libc.[a,so,etc] similar to other
> platforms. I think we'd need a good reason to diverge here.
>

The reason is that LLVM libc is not a libc until it can be one. This option
is being added so that one can use LLVM libc as a source of alternate
implementations. Once LLVM libc can actually be a full libc, this option is
not required. Also, from that point in time, the LLVM libc binary should be
given the conventional libc. name as you suggest.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Eric Christopher via cfe-commits]

On Fri, Mar 5, 2021 at 1:15 AM Siva Chandra via Phabricator <
revi...@reviews.llvm.org> wrote:

> sivachandra added a comment.
>
> In D97736#2605535 , @phosek
> wrote:
>
> > Have you considered using an input linker script? We could generate
> `libc.so` that could look something like:
> >
> >   INPUT(libllvmlibc.a /lib/libc.so)
> >
> > We would need to pass `--sysroot` to the linker for this to work. The
> driver could remain completely agnostic of whether you're using LLVM libc
> or not.
>
> Yes, that was also considered. Those downstream users who have the
> flexibility to do it that way should be able to do it that way. However,
> not all downstream users or normal clang users will have that liberty [1].
> Another point to note is that we will have to do this with all libc
> components like `libc.so`, `libm.so` etc.
>
> [1] I think all of this can be done. For example, we can set all this up
> when building a distribution. However, I am not sure this is worth it when
> we know this is a transient phase. Soon, when LLVM libc is complete enough,
> a more appropriate option would be the one which allows choosing a libc as
> Eric pointed out.
>

To be clear I'm not a fan of a "pick your libc" option as opposed to just
naming the compiled llvm libc as perhaps libc.[a,so,etc] similar to other
platforms. I think we'd need a good reason to diverge here.

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

In D97736#2605535 , @phosek wrote:

> Have you considered using an input linker script? We could generate `libc.so` 
> that could look something like:
>
>   INPUT(libllvmlibc.a /lib/libc.so)
>
> We would need to pass `--sysroot` to the linker for this to work. The driver 
> could remain completely agnostic of whether you're using LLVM libc or not.

Yes, that was also considered. Those downstream users who have the flexibility 
to do it that way should be able to do it that way. However, not all downstream 
users or normal clang users will have that liberty [1]. Another point to note 
is that we will have to do this with all libc components like `libc.so`, 
`libm.so` etc.

[1] I think all of this can be done. For example, we can set all this up when 
building a distribution. However, I am not sure this is worth it when we know 
this is a transient phase. Soon, when LLVM libc is complete enough, a more 
appropriate option would be the one which allows choosing a libc as Eric 
pointed out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

Have you considered using an input linker script? We could generate `libc.so` 
that could look something like:

  INPUT(libllvmlibc.a /lib/libc.so)

We would need to pass `--sysroot` to the linker for this to work. The driver 
could remain completely agnostic of whether you're using LLVM libc or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 328380.
sivachandra added a comment.

Remove empty '//' in lit test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/linux-ld.c


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -232,7 +232,27 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 
'static-pie'
-//
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lm"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lc"
+
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -670,6 +670,23 @@
 
   Args.AddAllArgs(CmdArgs, options::OPT_T);
 
+  if (Args.hasArg(options::OPT_experimental_link_llvmlibc)) {
+// We should add -lllvmlibc after all other link args have been 
accumulated.
+// This way, we can iterate over all the link args and add -lllvmlibc 
before
+// each libc library. This will also ensure that we prepend -lllvmlibc
+// before any used listed -lm options.
+ArgStringList WithLLVMLibc;
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread
+// functions available.
+WithLLVMLibc.push_back("-lllvmlibc");
+  }
+  WithLLVMLibc.push_back(Arg.data());
+}
+CmdArgs = WithLLVMLibc;
+  }
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3623,6 +3623,7 @@
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group, 
Flags<[NoArgumentUnused]>;


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -232,7 +232,27 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 'static-pie'
-//
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: 

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

In D97736#2605432 , @echristo wrote:

> In addition to the bikeshed below, I'm not a huge fan of this in general. I 
> think we should assume that the libc we link is complete and thus it would 
> just be named "libc." and in a sysroot somewhere. Another option is 
> perhaps looking at the rtlib option, but I'd want to see a bit of a writeup 
> there so we can see what we'd be doing.

I missed responding to the "writeup" part earlier. But, I am not sure what I 
should be writing about. I feel that there is some confusion that this option 
can be used to switch between libcs. As I said in the earlier reply, the idea 
is not to provide a way to switch between libcs. Rather, the goal is to provide 
a way to use LLVM libc along with the system libc with the effect that LLVM 
libc symbols will be preferred by the linker over the system libc symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

In D97736#2605432 , @echristo wrote:

> In addition to the bikeshed below, I'm not a huge fan of this in general. I 
> think we should assume that the libc we link is complete and thus it would 
> just be named "libc." and in a sysroot somewhere. Another option is 
> perhaps looking at the rtlib option, but I'd want to see a bit of a writeup 
> there so we can see what we'd be doing.

By not giving it a `libc.` name, we are actually implying that, "it is not 
the full libc that you expect." It is expected that it will be used along with 
another full libc. When LLVM libc is complete enough, then yes we should be 
giving it the conventional name of `libc.` and the experimental option can 
be thrown out.

In D97736#2605159 , @MaskRay wrote:

> If the end goal is to provide a complete libc, and currently the usage is 
> expected to shadow system libc (this has to be very careful, as I don't know 
> how shadowing some important components like pthread shall work), perhaps the 
> name should convey this point?

Yes. So, we have not done it yet, but the plan is to be able to build LLVM in 
two different modes:

1. The default mode: This mode only builds and packages the ABI independent and 
ABI agnostic parts of LLVM libc. The binaries produced from this mode are to be 
used with the `-experimental-llvm-libc`. The plan is also to make default mode 
work under `LLVM_RUNTIMES_BUILD`.
2. The full libc mode: In this mode, LLVM libc will be built as if it is a full 
libc. A special CMake option needs to be set to build in this mode. The 
binaries produced in this mode will include everything, including ABI sensitive 
pieces. They will be tested on bots but will not be practically useful for many 
months as we are still building out the libc.




Comment at: clang/include/clang/Driver/Options.td:3626
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;

echristo wrote:
> Bikeshedding: Not a huge fan of this name. I'd rather a more general option 
> if we feel the need to select libc from an option rather than a sysroot. 
The idea is not to provide an option to "choose" a libc. The idea is that this 
option will help one use what LLVM libc provides while getting the rest from 
the system libc. I would think that the option to "choose" a libc is not 
related to this as LLVM libc is not complete.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

In addition to the bikeshed below, I'm not a huge fan of this in general. I 
think we should assume that the libc we link is complete and thus it would just 
be named "libc." and in a sysroot somewhere. Another option is perhaps 
looking at the rtlib option, but I'd want to see a bit of a writeup there so we 
can see what we'd be doing.

Thoughts? Other options?

Thanks!

-eric




Comment at: clang/include/clang/Driver/Options.td:3626
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;

Bikeshedding: Not a huge fan of this name. I'd rather a more general option if 
we feel the need to select libc from an option rather than a sysroot. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

If the end goal is to provide a complete libc, and currently the usage is 
expected to shadow system libc (this has to be very careful, as I don't know 
how shadowing some important components like pthread shall work), perhaps the 
name should convey this point?




Comment at: clang/test/Driver/linux-ld.c:235
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 
'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \

Empty `// ` actually makes browsing difficult and we generally don't do this 
for other tests. You can omit them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:680
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread

sivachandra wrote:
> phosek wrote:
> > This wouldn't handle the case where you use `-nolibc path/to/libc.a` in 
> > which case you'd have to manually pass in the `libllvmllibc.a`, but I'm not 
> > sure if that's a case we care about.
> Yeah, its hard to cater to all combinations.  In this case though, assuming 
> `libllvmlibc.a` is available in path, one can add `-lllvmlibc` in the right 
> place. 
Another point to note is that `-nolibc` does not prevent `-lm` from getting 
added for C++ compilations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:680
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread

phosek wrote:
> This wouldn't handle the case where you use `-nolibc path/to/libc.a` in which 
> case you'd have to manually pass in the `libllvmllibc.a`, but I'm not sure if 
> that's a case we care about.
Yeah, its hard to cater to all combinations.  In this case though, assuming 
`libllvmlibc.a` is available in path, one can add `-lllvmlibc` in the right 
place. 



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:684
+WithLLVMLibc.push_back("-lllvmlibc");
+WithLLVMLibc.push_back(Arg.data());
+  } else {

phosek wrote:
> You can move this after the condition and omit the `else` branch.
[Shame cube] Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 328340.
sivachandra added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/linux-ld.c


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -233,6 +233,26 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 
'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+//
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lm"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lc"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -670,6 +670,23 @@
 
   Args.AddAllArgs(CmdArgs, options::OPT_T);
 
+  if (Args.hasArg(options::OPT_experimental_link_llvmlibc)) {
+// We should add -lllvmlibc after all other link args have been 
accumulated.
+// This way, we can iterate over all the link args and add -lllvmlibc 
before
+// each libc library. This will also ensure that we prepend -lllvmlibc
+// before any used listed -lm options.
+ArgStringList WithLLVMLibc;
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread
+// functions available.
+WithLLVMLibc.push_back("-lllvmlibc");
+  }
+  WithLLVMLibc.push_back(Arg.data());
+}
+CmdArgs = WithLLVMLibc;
+  }
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3623,6 +3623,7 @@
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group, 
Flags<[NoArgumentUnused]>;


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -233,6 +233,26 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+//
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// 

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:680
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread

This wouldn't handle the case where you use `-nolibc path/to/libc.a` in which 
case you'd have to manually pass in the `libllvmllibc.a`, but I'm not sure if 
that's a case we care about.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:684
+WithLLVMLibc.push_back("-lllvmlibc");
+WithLLVMLibc.push_back(Arg.data());
+  } else {

You can move this after the condition and omit the `else` branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra created this revision.
sivachandra added a reviewer: phosek.
Herald added subscribers: jansvoboda11, dang.
sivachandra requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The experimental option is named -experimental-link-llvmlibc. Specifying
it will put -lllvmlibc before each system libc library component on the
link command line. This way, even if the user specifies -lm, functions
from LLVM libc will be picked instead of functions from the system libc
math library. Also, sanitizer and other options add -lm at few other
places. By putting -lllvmlibc right before each of them, we ensure that
functions available in LLVM libc are picked up by the linker.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97736

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/linux-ld.c


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -233,6 +233,26 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 
'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+//
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lm"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lc"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -670,6 +670,25 @@
 
   Args.AddAllArgs(CmdArgs, options::OPT_T);
 
+  if (Args.hasArg(options::OPT_experimental_link_llvmlibc)) {
+// We should add -lllvmlibc after all other link args have been 
accumulated.
+// This way, we can iterate over all the link args and add -lllvmlibc 
before
+// each libc library. This will also ensure that we prepend -lllvmlibc
+// before any used listed -lm options.
+ArgStringList WithLLVMLibc;
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread
+// functions available.
+WithLLVMLibc.push_back("-lllvmlibc");
+WithLLVMLibc.push_back(Arg.data());
+  } else {
+WithLLVMLibc.push_back(Arg.data());
+  }
+}
+CmdArgs = WithLLVMLibc;
+  }
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3623,6 +3623,7 @@
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group, 
Flags<[NoArgumentUnused]>;


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -233,6 +233,26 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: