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

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

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

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,

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

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,

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

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,

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

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

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

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

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

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/

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

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

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

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

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

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

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

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

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

abidh added a comment. In D91442#2399753 , @jrtc27 wrote: > In D91442#2399750 , @abidh wrote: > >> In D91442#2399402 , @lenary wrote: >> >>> In D91442#2399341

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

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

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

jrtc27 added a comment. In D91442#2399753 , @jrtc27 wrote: > In D91442#2399750 , @abidh wrote: > >> In D91442#2399402 , @lenary wrote: >> >>> In D91442#2399341

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

jrtc27 added a comment. In D91442#2399750 , @abidh wrote: > In D91442#2399402 , @lenary wrote: > >> In D91442#2399341 , @abidh wrote: >> >>> In D91442#2399200

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

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 -

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

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

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

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

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

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

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

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"

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

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

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

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:

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

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;

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

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

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

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

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

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,