[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1754 + + int RC = llvm::sys::ExecuteAndWait(GCCPath, GCCArgs, None, Redirects); + Even if ExecuteAndWait is accepted, you may consider a pipe. The current implementation leaves tempo

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. In D97916#2625170 , @luismarques wrote: > This patch seems to be in pretty good shape now. > > One thing that might be useful (important?) t

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng updated this revision to Diff 331448. kito-cheng added a comment. ChangeLog: - Add more verbose message during reading GCC multilib configuration Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97916/new/ https://reviews.llvm.org/D97916

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng added a comment. > This still doesn't report that the multilib configuration came from GCC when > it succeeds, does it? I suppose that's not a deal-breaker, but it would be > nice to have. Would it be difficult to implement? There is some order issue is those function are executed be

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

luismarques added a comment. This still doesn't report that the multilib configuration came from GCC when it succeeds, does it? I suppose that's not a deal-breaker, but it would be nice to have. Would it be difficult to implement? Regarding the Windows test issue, aren't there other test cases

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng updated this revision to Diff 331024. kito-cheng added a comment. Address jrtc27's and luismarques comment ChangeLogs - Move all new test to clang/test/Driver/riscv-toolchain-gcc-multilib.c - I don't found good way to test that on windows, since ExecuteAndWait only allow to execute

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

luismarques added a comment. This patch seems to be in pretty good shape now. One thing that might be useful (important?) to add is additional diagnostics when run in verbose mode. Currently `clang -v` will indicate that it found the GCC installation and will list the multilibs but there will b

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

jrtc27 added inline comments. Comment at: clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/bin/riscv32-unknown-elf-gcc:1 +#!/usr/bin/env python + Python 3 Comment at: clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/bin/riscv32-unknown-elf-

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

khchen added a comment. Overall LGTM. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1604 + const Arg *A = Args.getLastArg(clang::driver::options::OPT_gcc_toolchain); + if (A) { +GCCPath = findGCCPath(D, A->getValue()); khchen wrote: > if (const Arg *A =

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng updated this revision to Diff 328407. kito-cheng added a comment. Reupload Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97916/new/ https://reviews.llvm.org/D97916 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/incl

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng updated this revision to Diff 328406. kito-cheng marked 2 inline comments as done. kito-cheng added a comment. - Fix build issue. - Address Jim Lin's and Zakk's comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97916/new/ https://rev

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng marked 5 inline comments as done. kito-cheng added a comment. Here is another solution for flexible multi-lib configuration I consider before: - Add an option called `-fmultilib-config=` - Using same or similar syntax with GCC's `--with-multilib-generator` But the problem is it's real

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

khchen added a comment. I didn't check why I got error in Failed Tests (1): Clang :: Driver/riscv64-toolchain.c Could you please double check it? Thanks! Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1599 +static std::string getGCCPath(const Driver &D, const ArgList &Ar

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

Jim added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1744 + // which multi-lib should be used. + return ScanGCCMultilibConfig(D, TargetTriple, Path, Args, MultilibOutput, + Result); scanGCCMultilibConfig ? R

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

Jim added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1703 +} + } + Ms.emplace_back(Multilib); Do you have plan to support other kind of options to build multilib? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng marked 3 inline comments as done. kito-cheng added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:727 +{ + /* Default code model is small(medlow). */ + StringRef CodeModel; MaskRay wrote: > `//` Thanks, I guess some time my

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng updated this revision to Diff 328062. kito-cheng added a comment. Address MaskRay's comment and apply clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97916/new/ https://reviews.llvm.org/D97916 Files: clang/include/clang/Ba

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng added inline comments. Comment at: clang/test/Driver/riscv64-toolchain.c:158 +// RUN: %clang %s \ +// RUN: -target riscv64-unknown-elf \ +// RUN: --gcc-toolchain=%S/Inputs/multilib_riscv64_elf_sdk \ MaskRay wrote: > Hmm. I happened to have posted

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:727 +{ + /* Default code model is small(medlow). */ + StringRef CodeModel; `//` Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1595 +return GCCPath.st

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng updated this revision to Diff 328040. kito-cheng added a comment. Minor clean up Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97916/new/ https://reviews.llvm.org/D97916 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clan

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

kito-cheng created this revision. Herald added subscribers: vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, simoncook, johnrusso,