[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.
Herald added subscribers: wangpc, luke, sunshaoce, arichardson.
Herald added a project: All.



Comment at: clang/lib/Driver/ToolChains/RISCVToolchain.cpp:40
+ const llvm::opt::ArgList ) {
+  if (Args.getLastArg(options::OPT_gcc_toolchain))
+return true;

This logic (if `--gcc-toolchain=` is available or `lib/crt0.o` is present, 
select `RISCVToolChain`; otherwise `BareMetal`) is strange.

Can someone shed light on what this `RISCVToolChain` is intended and which 
`crt0.o` is used?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5216-5220
+if (toolchains::RISCVToolChain::hasGCCToolchain(*this, Args))
+  TC =
+  std::make_unique(*this, Target, 
Args);
+else
+  TC = std::make_unique(*this, Target, Args);

abidh wrote:
> jrtc27 wrote:
> > This has broken our use downstream. We construct a normal bare-metal 
> > non-multilib baremetal sysroot, no GCC involved, passed explicitly with 
> > --sysroot. However, BareMetal's findRISCVMultilibs unconditionally appends 
> > multilib paths for anything other than IMAC (always ILP32/LP64) (well, it 
> > still appends the path, just the multilib path is curiously empty), without 
> > filtering out ones that don't exist like Gnu.cpp. I assume the empty string 
> > for IMAC is a hack to allow most normal uses of bare-metal toolchains to 
> > work, but it breaks if you want hard float support (or fewer extensions).
> > 
> > I suspect this is best fixed by filtering out non-existent directories from 
> > findRISCVMultilibs (and at the same time, fixing IMAC to have a non-empty 
> > path, since RISCVToolChain has one for it, and we no longer need the hacky, 
> > insufficient workaround)?
> IIRC the empty path was probably there to keep the non-multilib toolchain 
> working. I don't mind if we remove the empty path for IMAC and all multilibs 
> have non-empty paths.
> 
> I am not sure on filtering out non-existing folder though. It is a bit 
> different from GNU.cpp in that we may not be using a pre-built sysroot and 
> use the just-built compiler for building the runtime too. Also I am wondering 
> what should happen if user wants a multilib whose runtime bits are not 
> present in the toolchain. 
Well we need to detect multilib vs non-multilib sysroots *somehow*, and 
everywhere else that gets done based on file or directory existence. If you 
want a multilib but don't have it, falling back on the non-multilib path and 
failing (either with incompatibility errors or missing files) seems fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5216-5220
+if (toolchains::RISCVToolChain::hasGCCToolchain(*this, Args))
+  TC =
+  std::make_unique(*this, Target, 
Args);
+else
+  TC = std::make_unique(*this, Target, Args);

jrtc27 wrote:
> This has broken our use downstream. We construct a normal bare-metal 
> non-multilib baremetal sysroot, no GCC involved, passed explicitly with 
> --sysroot. However, BareMetal's findRISCVMultilibs unconditionally appends 
> multilib paths for anything other than IMAC (always ILP32/LP64) (well, it 
> still appends the path, just the multilib path is curiously empty), without 
> filtering out ones that don't exist like Gnu.cpp. I assume the empty string 
> for IMAC is a hack to allow most normal uses of bare-metal toolchains to 
> work, but it breaks if you want hard float support (or fewer extensions).
> 
> I suspect this is best fixed by filtering out non-existent directories from 
> findRISCVMultilibs (and at the same time, fixing IMAC to have a non-empty 
> path, since RISCVToolChain has one for it, and we no longer need the hacky, 
> insufficient workaround)?
IIRC the empty path was probably there to keep the non-multilib toolchain 
working. I don't mind if we remove the empty path for IMAC and all multilibs 
have non-empty paths.

I am not sure on filtering out non-existing folder though. It is a bit 
different from GNU.cpp in that we may not be using a pre-built sysroot and use 
the just-built compiler for building the runtime too. Also I am wondering what 
should happen if user wants a multilib whose runtime bits are not present in 
the toolchain. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5216-5220
+if (toolchains::RISCVToolChain::hasGCCToolchain(*this, Args))
+  TC =
+  std::make_unique(*this, Target, 
Args);
+else
+  TC = std::make_unique(*this, Target, Args);

This has broken our use downstream. We construct a normal bare-metal 
non-multilib baremetal sysroot, no GCC involved, passed explicitly with 
--sysroot. However, BareMetal's findRISCVMultilibs unconditionally appends 
multilib paths for anything other than IMAC (always ILP32/LP64) (well, it still 
appends the path, just the multilib path is curiously empty), without filtering 
out ones that don't exist like Gnu.cpp. I assume the empty string for IMAC is a 
hack to allow most normal uses of bare-metal toolchains to work, but it breaks 
if you want hard float support (or fewer extensions).

I suspect this is best fixed by filtering out non-existent directories from 
findRISCVMultilibs (and at the same time, fixing IMAC to have a non-empty path, 
since RISCVToolChain has one for it, and we no longer need the hacky, 
insufficient workaround)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh added a comment.

In D91442#2444399 , @MaskRay wrote:

> This change has been failing on my machine for the past two weeks.
>
> `riscv32-toolchain-extra.c` and `riscv64-toolchain-extra.c` are not hermit. I 
> have installed `/usr/lib/gcc-cross/riscv64-linux-gnu/10` 
> (gcc-10-riscv64-linux-gnu and its dependencies) on my machine and the test 
> will find some directories relative to 
> `/usr/lib/gcc-cross/riscv64-linux-gnu/10`. I think `--sysroot` is required 
> for such tests.
>
> `%T` is a deprecated lit feature. Please do something like `rm -rf %t; mkdir 
> %t` instead.

Hi MaskRay,
Thanks for reporting this issue. I don't fully understand how test is finding 
directories relative to some other location. But I have opened 
https://reviews.llvm.org/D93023 to replace %T with %t as you suggested. Please 
have a look and see if this fixes your issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

This change has been failing on my machine for the past two weeks.

`riscv32-toolchain-extra.c` and `riscv64-toolchain-extra.c` are not hermit. I 
have installed `/usr/lib/gcc-cross/riscv64-linux-gnu/10` 
(gcc-10-riscv64-linux-gnu and its dependencies) on my machine and the test will 
find some directories relative to `/usr/lib/gcc-cross/riscv64-linux-gnu/10`.

`%T` is a deprecated lit feature. Please do something like `rm -rf %t; mkdir 
%t` instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG45ba2392d7e0: [clang][Driver] Handle risvc in Baremetal.cpp. 
(authored by abidh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91442

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.h
  clang/test/Driver/baremetal.cpp
  clang/test/Driver/riscv-gnutools.c
  clang/test/Driver/riscv32-toolchain-extra.c
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain-extra.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -1,11 +1,15 @@
 // A basic clang -cc1 command-line, and simple environment check.
 // REQUIRES: platform-linker
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv64"
 
 // Test interaction with -fuse-ld=lld, if lld is available.
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld 2>&1 | FileCheck -check-prefix=LLD %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=LLD %s
 // LLD: {{(error: invalid linker name in argument '-fuse-ld=lld')|(ld.lld)}}
 
 // In the below tests, --rtlib=platform is used so that the driver ignores
@@ -133,6 +137,7 @@
 
 // Check that --rtlib can be used to override the used runtime library
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv64-unknown-elf --rtlib=libgcc 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-RTLIB-LIBGCC-LP64 %s
 // C-RV64-RTLIB-LIBGCC-LP64: "{{.*}}crt0.o"
@@ -141,6 +146,7 @@
 // C-RV64-RTLIB-LIBGCC-LP64: "{{.*}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv64-unknown-elf --rtlib=compiler-rt 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-RTLIB-COMPILERRT-LP64 %s
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}crt0.o"
Index: clang/test/Driver/riscv64-toolchain-extra.c
===
--- clang/test/Driver/riscv64-toolchain-extra.c
+++ clang/test/Driver/riscv64-toolchain-extra.c
@@ -22,6 +22,10 @@
 // RUN:-target riscv64-unknown-elf --rtlib=platform 2>&1 \
 // RUN:| FileCheck -check-prefix=C-RV64-BAREMETAL-LP64-NOGCC %s
 
+// RUN: %T/testroot-riscv64-baremetal-nogcc/bin/clang %s -### -no-canonical-prefixes \
+// RUN:-target riscv64-unknown-elf --rtlib=platform 2>&1 \
+// RUN:| FileCheck -check-prefix=C-RV64-BAREMETAL-LP64-NOGCC %s
+
 // C-RV64-BAREMETAL-LP64-NOGCC: "-internal-isystem" "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/../riscv64-unknown-elf/include"
 // C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/riscv64-unknown-elf-ld"
 // C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/../riscv64-unknown-elf/lib/crt0.o"
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -1,11 +1,15 @@
 // A basic clang -cc1 command-line, and simple environment check.
 // REQUIRES: platform-linker
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv32"
 
 // Test interaction with -fuse-ld=lld, if lld is available.
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld 2>&1 | FileCheck -check-prefix=LLD %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fuse-ld=lld 2>&1 \
+// RUN:   | FileCheck -check-prefix=LLD %s
 // LLD: {{(error: invalid linker name in argument '-fuse-ld=lld')|(ld.lld)}}
 
 // In the below tests, --rtlib=platform is used so that the driver ignores
@@ -177,6 +181,7 @@
 
 // Check that --rtlib can be used to override the used runtime library
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv32-unknown-elf --rtlib=libgcc 

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Nope, no more comments from me. Thanks for updating based on my comments :)


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh added a comment.

Hi @lenary,
Do you any more comments on the patch?


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh updated this revision to Diff 306180.
abidh added a comment.
Herald added a subscriber: MaskRay.

This update contains following changes.

1. Address issue raised by lenary. I have introduced a static function that 
checks for the presence of gcc toolchain first by --gcc-toolchain argument or 
in the same prefix where clang is installed. If found, RISCVToolChain is 
instantiated. I have added tests in riscv64-toolchain-extra.c and 
riscv32-toolchain-extra.c to check this case.

2. Fixed some formatting things pointed out by jrtc27.

3. In the initial revision, jroelofs commented that other place where Baremetal 
is intantiated should have the similar check. I put that in 2nd version of the 
patch. But I think it does quite make sense in the current patch so I have 
removed it.

4. Rebased baremetal.cpp tests after recent changes and fixed some typos.


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

https://reviews.llvm.org/D91442

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.h
  clang/test/Driver/baremetal.cpp
  clang/test/Driver/riscv-gnutools.c
  clang/test/Driver/riscv32-toolchain-extra.c
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain-extra.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -1,11 +1,15 @@
 // A basic clang -cc1 command-line, and simple environment check.
 // REQUIRES: platform-linker
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv64"
 
 // Test interaction with -fuse-ld=lld, if lld is available.
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld 2>&1 | FileCheck -check-prefix=LLD %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=LLD %s
 // LLD: {{(error: invalid linker name in argument '-fuse-ld=lld')|(ld.lld)}}
 
 // In the below tests, --rtlib=platform is used so that the driver ignores
@@ -133,6 +137,7 @@
 
 // Check that --rtlib can be used to override the used runtime library
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv64-unknown-elf --rtlib=libgcc 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-RTLIB-LIBGCC-LP64 %s
 // C-RV64-RTLIB-LIBGCC-LP64: "{{.*}}crt0.o"
@@ -141,6 +146,7 @@
 // C-RV64-RTLIB-LIBGCC-LP64: "{{.*}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv64-unknown-elf --rtlib=compiler-rt 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-RTLIB-COMPILERRT-LP64 %s
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}crt0.o"
Index: clang/test/Driver/riscv64-toolchain-extra.c
===
--- clang/test/Driver/riscv64-toolchain-extra.c
+++ clang/test/Driver/riscv64-toolchain-extra.c
@@ -22,6 +22,10 @@
 // RUN:-target riscv64-unknown-elf --rtlib=platform 2>&1 \
 // RUN:| FileCheck -check-prefix=C-RV64-BAREMETAL-LP64-NOGCC %s
 
+// RUN: %T/testroot-riscv64-baremetal-nogcc/bin/clang %s -### -no-canonical-prefixes \
+// RUN:-target riscv64-unknown-elf --rtlib=platform 2>&1 \
+// RUN:| FileCheck -check-prefix=C-RV64-BAREMETAL-LP64-NOGCC %s
+
 // C-RV64-BAREMETAL-LP64-NOGCC: "-internal-isystem" "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/../riscv64-unknown-elf/include"
 // C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/riscv64-unknown-elf-ld"
 // C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/../riscv64-unknown-elf/lib/crt0.o"
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -1,11 +1,15 @@
 // A basic clang -cc1 command-line, and simple environment check.
 // REQUIRES: platform-linker
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv32"
 
 // Test interaction with -fuse-ld=lld, if lld is available.
-// RUN: %clang %s -### -no-canonical-prefixes -target 

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh added a comment.

In D91442#2399772 , @lenary wrote:

> I recall that there is also https://reviews.llvm.org/D68407 which iirc hoped 
> to address using `RISCVToolchain` with Compiler-rt and libunwind - presumably 
> it is not a complete solution, but it might be we want to use some checking 
> of the GCCInstallation (like `RISCVToolChain::GetDefaultRuntimeLibType` does) 
> to choose which toolchain to instantiate? I'm not sure if we can satisfy that 
> ordering though.

But that function is not static and you need to instantiate the RISCVToolChain 
to use things like GCCInstallation.isValid().


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh added a comment.

In D91442#2399753 , @jrtc27 wrote:

> In D91442#2399750 , @abidh wrote:
>
>> In D91442#2399402 , @lenary wrote:
>>
>>> In D91442#2399341 , @abidh wrote:
>>>
 In D91442#2399200 , @lenary wrote:

> I'm worried about this change - I *think* it doesn't cover the existing 
> behaviour of a baremetal GCC toolchain being installed into the same 
> prefix as clang, and clang automatically picking up that baremetal gcc 
> toolchain. What should we expect to do here? This is especially an issue 
> if you're trying to make a relocatable toolchain tarball, where 
> specifying `--gcc-toolchain` automatically is difficult.

 Would it be possible to use a relative path with --gcc-toolchain then this 
 can be checked in either RISCVToolChain.cpp or GNU.cpp and adjusted 
 accordingly?
>>>
>>> The GCC toolchain, when given a relative path, already interprets it 
>>> relative to the working directory the compiler was invoked from, not the 
>>> directory the compiler is located in, iirc.
>>
>> I dont think path is made absolute relative using working directory. What is 
>> happening is that if (!VFS.exists(Prefix)) call in the following line will 
>> succeed if the path existed relative to working directory. This seems 
>> accidental to me.
>>
>> https://github.com/llvm/llvm-project/blob/5a9f3867046c4e1c97760e22a505f4d1d788417e/clang/lib/Driver/ToolChains/Gnu.cpp#L1947
>>
>> I tried a small change to turn a relative --gcc-toolchain into absolute 
>> using the compiler installed path and it works ok.  If this is something 
>> that works for you then I can post a patch.
>
> Relative paths should be relative to the current working directory. That is 
> what everyone expects and is what every other argument does (and see `=` and 
> `$SYSROOT` for how GCC and compatible drivers get around that in cases like 
> -I).

The constructor of Driver class in clang resolves the default sysroot relative 
to install directory.  That gave me the idea that perhaps similar thing could 
be done with gcc-toolchain too. I dont have a strong opinion on it either way. 
I am just trying to find a way where current RISCVToolChain based toolchain 
continue working while I can instantiate Baremetal toolchain.


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I recall that there is also https://reviews.llvm.org/D68407 which iirc hoped to 
address using `RISCVToolchain` with Compiler-rt and libunwind - presumably it 
is not a complete solution, but it might be we want to use some checking of the 
GCCInstallation (like `RISCVToolChain::GetDefaultRuntimeLibType` does) to 
choose which toolchain to instantiate? I'm not sure if we can satisfy that 
ordering though.


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D91442#2399753 , @jrtc27 wrote:

> In D91442#2399750 , @abidh wrote:
>
>> In D91442#2399402 , @lenary wrote:
>>
>>> In D91442#2399341 , @abidh wrote:
>>>
 In D91442#2399200 , @lenary wrote:

> I'm worried about this change - I *think* it doesn't cover the existing 
> behaviour of a baremetal GCC toolchain being installed into the same 
> prefix as clang, and clang automatically picking up that baremetal gcc 
> toolchain. What should we expect to do here? This is especially an issue 
> if you're trying to make a relocatable toolchain tarball, where 
> specifying `--gcc-toolchain` automatically is difficult.

 Would it be possible to use a relative path with --gcc-toolchain then this 
 can be checked in either RISCVToolChain.cpp or GNU.cpp and adjusted 
 accordingly?
>>>
>>> The GCC toolchain, when given a relative path, already interprets it 
>>> relative to the working directory the compiler was invoked from, not the 
>>> directory the compiler is located in, iirc.
>>
>> I dont think path is made absolute relative using working directory. What is 
>> happening is that if (!VFS.exists(Prefix)) call in the following line will 
>> succeed if the path existed relative to working directory. This seems 
>> accidental to me.
>>
>> https://github.com/llvm/llvm-project/blob/5a9f3867046c4e1c97760e22a505f4d1d788417e/clang/lib/Driver/ToolChains/Gnu.cpp#L1947
>>
>> I tried a small change to turn a relative --gcc-toolchain into absolute 
>> using the compiler installed path and it works ok.  If this is something 
>> that works for you then I can post a patch.
>
> Relative paths should be relative to the current working directory. That is 
> what everyone expects and is what every other argument does (and see `=` and 
> `$SYSROOT` for how GCC and compatible drivers get around that in cases like 
> -I).

And how is that accidental? It looks to me like the correct behaviour naturally 
falls out just as it normally does in that by default anything involving a 
relative path automatically interprets it relative to the current working 
directory.


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D91442#2399750 , @abidh wrote:

> In D91442#2399402 , @lenary wrote:
>
>> In D91442#2399341 , @abidh wrote:
>>
>>> In D91442#2399200 , @lenary wrote:
>>>
 I'm worried about this change - I *think* it doesn't cover the existing 
 behaviour of a baremetal GCC toolchain being installed into the same 
 prefix as clang, and clang automatically picking up that baremetal gcc 
 toolchain. What should we expect to do here? This is especially an issue 
 if you're trying to make a relocatable toolchain tarball, where specifying 
 `--gcc-toolchain` automatically is difficult.
>>>
>>> Would it be possible to use a relative path with --gcc-toolchain then this 
>>> can be checked in either RISCVToolChain.cpp or GNU.cpp and adjusted 
>>> accordingly?
>>
>> The GCC toolchain, when given a relative path, already interprets it 
>> relative to the working directory the compiler was invoked from, not the 
>> directory the compiler is located in, iirc.
>
> I dont think path is made absolute relative using working directory. What is 
> happening is that if (!VFS.exists(Prefix)) call in the following line will 
> succeed if the path existed relative to working directory. This seems 
> accidental to me.
>
> https://github.com/llvm/llvm-project/blob/5a9f3867046c4e1c97760e22a505f4d1d788417e/clang/lib/Driver/ToolChains/Gnu.cpp#L1947
>
> I tried a small change to turn a relative --gcc-toolchain into absolute using 
> the compiler installed path and it works ok.  If this is something that works 
> for you then I can post a patch.

Relative paths should be relative to the current working directory. That is 
what everyone expects and is what every other argument does (and see `=` and 
`$SYSROOT` for how GCC and compatible drivers get around that in cases like -I).


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh added a comment.

In D91442#2399402 , @lenary wrote:

> In D91442#2399341 , @abidh wrote:
>
>> In D91442#2399200 , @lenary wrote:
>>
>>> I'm worried about this change - I *think* it doesn't cover the existing 
>>> behaviour of a baremetal GCC toolchain being installed into the same prefix 
>>> as clang, and clang automatically picking up that baremetal gcc toolchain. 
>>> What should we expect to do here? This is especially an issue if you're 
>>> trying to make a relocatable toolchain tarball, where specifying 
>>> `--gcc-toolchain` automatically is difficult.
>>
>> Would it be possible to use a relative path with --gcc-toolchain then this 
>> can be checked in either RISCVToolChain.cpp or GNU.cpp and adjusted 
>> accordingly?
>
> The GCC toolchain, when given a relative path, already interprets it relative 
> to the working directory the compiler was invoked from, not the directory the 
> compiler is located in, iirc.

I dont think path is made absolute relative using working directory. What is 
happening is that if (!VFS.exists(Prefix)) call in the following line will 
succeed if the path existed relative to working directory. This seems 
accidental to me.

https://github.com/llvm/llvm-project/blob/5a9f3867046c4e1c97760e22a505f4d1d788417e/clang/lib/Driver/ToolChains/Gnu.cpp#L1947

I tried a small change to turn a relative --gcc-toolchain into absolute using 
the compiler installed path and it works ok.  If this is something that works 
for you then I can post a patch.


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D91442#2399341 , @abidh wrote:

> In D91442#2399200 , @lenary wrote:
>
>> I'm worried about this change - I *think* it doesn't cover the existing 
>> behaviour of a baremetal GCC toolchain being installed into the same prefix 
>> as clang, and clang automatically picking up that baremetal gcc toolchain. 
>> What should we expect to do here? This is especially an issue if you're 
>> trying to make a relocatable toolchain tarball, where specifying 
>> `--gcc-toolchain` automatically is difficult.
>
> Would it be possible to use a relative path with --gcc-toolchain then this 
> can be checked in either RISCVToolChain.cpp or GNU.cpp and adjusted 
> accordingly?

The GCC toolchain, when given a relative path, already interprets it relative 
to the working directory the compiler was invoked from, not the directory the 
compiler is located in, iirc.


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh added a comment.

In D91442#2399200 , @lenary wrote:

> I'm worried about this change - I *think* it doesn't cover the existing 
> behaviour of a baremetal GCC toolchain being installed into the same prefix 
> as clang, and clang automatically picking up that baremetal gcc toolchain. 
> What should we expect to do here? This is especially an issue if you're 
> trying to make a relocatable toolchain tarball, where specifying 
> `--gcc-toolchain` automatically is difficult.

Would it be possible to use a relative path with --gcc-toolchain then this can 
be checked in either RISCVToolChain.cpp or GNU.cpp and adjusted accordingly?


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I'm worried about this change - I *think* it doesn't cover the existing 
behaviour of a baremetal GCC toolchain being installed into the same prefix as 
clang, and clang automatically picking up that baremetal gcc toolchain. What 
should we expect to do here? This is especially an issue if you're trying to 
make a relocatable toolchain tarball, where specifying `--gcc-toolchain` 
automatically is difficult.


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/test/Driver/riscv64-toolchain.c:5
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 | FileCheck 
-check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv64"

(repeated below and in riscv32-toolchain.c)


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh marked an inline comment as done.
abidh added a comment.

In D91442#2394501 , @jroelofs wrote:

> Seems reasonable.
>
> I could see someone wanting to use `--gcc-toolchain` to point at the 
> baremetal toolchain for their target, but that's unlikely to work out of the 
> box anyway. I'd love to know where the Generic_GCC toolchain is used in 
> practice, since that's always seemed quite fragile to me.

Thanks for the review. I have handled the comment in updated diff. I will give 
it a few more days in case anyone else wants to comment before landing.


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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh updated this revision to Diff 305573.
abidh added a comment.

Made the condition consistent in both places where Baremetal toolchain is 
instantiated as suggested in review.


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

https://reviews.llvm.org/D91442

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/test/Driver/baremetal.cpp
  clang/test/Driver/riscv-gnutools.c
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -1,11 +1,13 @@
 // A basic clang -cc1 command-line, and simple environment check.
 // REQUIRES: platform-linker
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv64"
 
 // Test interaction with -fuse-ld=lld, if lld is available.
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld 2>&1 | FileCheck -check-prefix=LLD %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 | FileCheck -check-prefix=LLD %s
 // LLD: {{(error: invalid linker name in argument '-fuse-ld=lld')|(ld.lld)}}
 
 // In the below tests, --rtlib=platform is used so that the driver ignores
@@ -133,6 +135,7 @@
 
 // Check that --rtlib can be used to override the used runtime library
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv64-unknown-elf --rtlib=libgcc 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-RTLIB-LIBGCC-LP64 %s
 // C-RV64-RTLIB-LIBGCC-LP64: "{{.*}}crt0.o"
@@ -141,6 +144,7 @@
 // C-RV64-RTLIB-LIBGCC-LP64: "{{.*}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv64-unknown-elf --rtlib=compiler-rt 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-RTLIB-COMPILERRT-LP64 %s
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}crt0.o"
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -1,11 +1,13 @@
 // A basic clang -cc1 command-line, and simple environment check.
 // REQUIRES: platform-linker
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv32"
 
 // Test interaction with -fuse-ld=lld, if lld is available.
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld 2>&1 | FileCheck -check-prefix=LLD %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fuse-ld=lld 2>&1 | FileCheck -check-prefix=LLD %s
 // LLD: {{(error: invalid linker name in argument '-fuse-ld=lld')|(ld.lld)}}
 
 // In the below tests, --rtlib=platform is used so that the driver ignores
@@ -177,6 +179,7 @@
 
 // Check that --rtlib can be used to override the used runtime library
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv32-unknown-elf --rtlib=libgcc 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-RTLIB-LIBGCC-ILP32 %s
 // C-RV32-RTLIB-LIBGCC-ILP32: "{{.*}}crt0.o"
@@ -185,6 +188,7 @@
 // C-RV32-RTLIB-LIBGCC-ILP32: "{{.*}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv32-unknown-elf --rtlib=compiler-rt 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-RTLIB-COMPILERRT-ILP32 %s
 // C-RV32-RTLIB-COMPILERRT-ILP32: "{{.*}}crt0.o"
Index: clang/test/Driver/riscv-gnutools.c
===
--- clang/test/Driver/riscv-gnutools.c
+++ clang/test/Driver/riscv-gnutools.c
@@ -5,15 +5,15 @@
 // 32-bit checks
 
 // Check default on riscv32-unknown-elf
-// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as %s -### -c \
+// RUN: %clang -target riscv32-unknown-elf --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fno-integrated-as %s -### -c \
 // RUN: 2>&1 | FileCheck -check-prefix=CHECK-RV32IMAC-ILP32 %s
 
 // Check default on riscv32-unknown-linux-gnu
-// RUN: %clang -target riscv32-unknown-linux-gnu -fno-integrated-as %s -### -c \
+// RUN: %clang -target riscv32-unknown-linux-gnu --gcc-toolchain=%S/Inputs/basic_riscv32_tree 

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh added a comment.

In D91442#2394513 , @jrtc27 wrote:

> We use pure LLVM toolchains so improving support for that out of the box is 
> good in my books. However, I do worry this is going to cause friction for a 
> lot of people using LLVM for RISC-V; my understanding is that most use LLVM 
> with a GNU sysroot and binutils, and so this looks like it will break their 
> setups. Is there anything that can be done to automatically detect such 
> cases? What does Arm do here?

That was one of my concern too. Unlike riscv, Arm does not have an explicit 
check so it falls in the default where it picks Baremetal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

We use pure LLVM toolchains so improving support for that out of the box is 
good in my books. However, I do worry this is going to cause friction for a lot 
of people using LLVM for RISC-V; my understanding is that most use LLVM with a 
GNU sysroot and binutils, and so this looks like it will break their setups. Is 
there anything that can be done to automatically detect such cases? What does 
Arm do here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs accepted this revision.
jroelofs added a comment.
This revision is now accepted and ready to land.

Seems reasonable.

I could see someone wanting to use `--gcc-toolchain` to point at the baremetal 
toolchain for their target, but that's unlikely to work out of the box anyway. 
I'd love to know where the Generic_GCC toolchain is used in practice, since 
that's always seemed quite fragile to me.




Comment at: clang/lib/Driver/Driver.cpp:5150
   case llvm::Triple::riscv64:
-TC = std::make_unique(*this, Target, Args);
+if (!Args.getLastArg(clang::driver::options::OPT_gcc_toolchain) &&
+toolchains::BareMetal::handlesTarget(Target))

for consistency and principle-of-least-surprise, maybe we should have the same 
check in the other place where `toolchains::BareMetal` is created?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91442

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

[Hafiz Abid Qadeer via Phabricator via cfe-commits]

abidh created this revision.
abidh added reviewers: asb, jroelofs, manojgupta, clang.
Herald added subscribers: cfe-commits, frasercrmck, luismarques, apazos, 
sameer.abuasal, pzheng, s.egerton, lenary, Jim, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, 
sabuasal, simoncook, johnrusso, rbar, kristof.beyls.
Herald added a project: clang.
abidh requested review of this revision.

I am working on a baremetal riscv toolchain using LLVM runtime and
LLD linker. Baremetal.cpp provides most of the things needed for such
toolchain. So I have modified it to also handle riscv64/32-unknown-elf
 targets alongside arm-none-eabi.

Currently, targets like riscv64-unknown-elf are handled by RISCVToolChain
which mostly expects a gcc toolchain to be present. If you dont
want the dependency on gcc-toolchain/libgloss or want to use LLD, then
RISCVToolChain is not a good fit.

So in the toolchain selection code, I have made this dependency of
RISCVToolChain on gcc toolchain explicit. It is created if gcc-toolchain
option is present. Otherwise Baremetal toolchain is created. I will be
happy to hear if there is a better way to choose between these two
toolchains.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91442

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/test/Driver/baremetal.cpp
  clang/test/Driver/riscv-gnutools.c
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -1,11 +1,13 @@
 // A basic clang -cc1 command-line, and simple environment check.
 // REQUIRES: platform-linker
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv64"
 
 // Test interaction with -fuse-ld=lld, if lld is available.
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld 2>&1 | FileCheck -check-prefix=LLD %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 | FileCheck -check-prefix=LLD %s
 // LLD: {{(error: invalid linker name in argument '-fuse-ld=lld')|(ld.lld)}}
 
 // In the below tests, --rtlib=platform is used so that the driver ignores
@@ -133,6 +135,7 @@
 
 // Check that --rtlib can be used to override the used runtime library
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv64-unknown-elf --rtlib=libgcc 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-RTLIB-LIBGCC-LP64 %s
 // C-RV64-RTLIB-LIBGCC-LP64: "{{.*}}crt0.o"
@@ -141,6 +144,7 @@
 // C-RV64-RTLIB-LIBGCC-LP64: "{{.*}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv64-unknown-elf --rtlib=compiler-rt 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-RTLIB-COMPILERRT-LP64 %s
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}crt0.o"
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -1,11 +1,13 @@
 // A basic clang -cc1 command-line, and simple environment check.
 // REQUIRES: platform-linker
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv32"
 
 // Test interaction with -fuse-ld=lld, if lld is available.
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld 2>&1 | FileCheck -check-prefix=LLD %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv32_tree -fuse-ld=lld 2>&1 | FileCheck -check-prefix=LLD %s
 // LLD: {{(error: invalid linker name in argument '-fuse-ld=lld')|(ld.lld)}}
 
 // In the below tests, --rtlib=platform is used so that the driver ignores
@@ -177,6 +179,7 @@
 
 // Check that --rtlib can be used to override the used runtime library
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv32-unknown-elf --rtlib=libgcc 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-RTLIB-LIBGCC-ILP32 %s
 // C-RV32-RTLIB-LIBGCC-ILP32: "{{.*}}crt0.o"
@@ -185,6 +188,7 @@
 // C-RV32-RTLIB-LIBGCC-ILP32: "{{.*}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes