[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu abandoned this revision. zixuan-wu added a comment. Herald added subscribers: jobnoorman, luke. As this issue is solved at https://reviews.llvm.org/D148124, abandon this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129824/new/ https://reviews.llvm.org/D129824

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu added a comment. Ping.. Do we still prefer to specify target triple explicitly to make a check? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129824/new/ https://reviews.llvm.org/D129824 ___ cfe-commits mailing list

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu added a comment. AFAIK, --target is clang-specific and transparent for compiler user in RV side. -m is also undefined or less used by RV user. It just uses -march to specify 32 or 64 mode and extensions. Is the convention specified in RV Spec? CHANGES SINCE LAST ACTION

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

MaskRay added a comment. In D129824#3794229 , @jrtc27 wrote: > In D129824#3794221 , @MaskRay wrote: > >> Both D54214 and this look like a >> surprising behavior to me. Do we

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

jrtc27 added a comment. In D129824#3794221 , @MaskRay wrote: > Both D54214 and this look like a surprising > behavior to me. Do we still have time to go back the state before D54214 >

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

MaskRay added a comment. Both D54214 and this look like a surprising behavior to me. Do we still have time to go back the state before D54214 and make mismatching --target & -march= an error? > clang -target=riscv32--

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu marked an inline comment as not done. zixuan-wu added a comment. ping... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129824/new/ https://reviews.llvm.org/D129824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu added inline comments. Comment at: clang/test/Driver/riscv-arch.c:410 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s +// RUN: %clang --target=riscv32-unknown-elf -mcpu=sifive-s21 -### %s \ +// RUN: -fsyntax-only 2>&1 | FileCheck

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

jrtc27 added inline comments. Comment at: clang/test/Driver/riscv-arch.c:410 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s +// RUN: %clang --target=riscv32-unknown-elf -mcpu=sifive-s21 -### %s \ +// RUN: -fsyntax-only 2>&1 | FileCheck

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu added a comment. In D129824#3706654 , @asb wrote: > @zixuan-wu we discussed this a bit in the last community sync call > . > One aspect that makes this patch a

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

asb added a comment. @zixuan-wu we discussed this a bit in the last community sync call . One aspect that makes this patch a little hard to review is the lack of a clear patch description summarising the behaviour

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu added a comment. In D129824#3699911 , @kito-cheng wrote: > Just realized the problem is trying to fixed the default value of `-mabi=`, > currently `clang -target riscv32-elf -march=rv64gc > -mabi=lp64d`/`riscv32-elf-clang -march=rv64gc

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

pcwang-thead added a comment. In D129824#3699911 , @kito-cheng wrote: > Just realized the problem is trying to fixed the default value of `-mabi=`, > currently `clang -target riscv32-elf -march=rv64gc > -mabi=lp64d`/`riscv32-elf-clang -march=rv64gc

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

kito-cheng added a comment. Just realized the problem is trying to fixed the default value of `-mabi=`, currently `clang -target riscv32-elf -march=rv64gc -mabi=lp64d`/`riscv32-elf-clang -march=rv64gc -mabi=lp64d` is work, and match the behavior of GCC did, `riscv32-elf-gcc -march=rv64gc

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

jrtc27 added a comment. If you still want to pursue this, discussion belongs at https://github.com/riscv-non-isa/riscv-toolchain-conventions, not here, since it's an interface shared by Clang and GCC and the two should be consistent CHANGES SINCE LAST ACTION

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu added a comment. In D129824#3670586 , @reames wrote: > This was very briefly discussed at today's sync up call. We were running > short on time, so we didn't get a chance to talk through it, but there did > seem to be a consensus that

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

reames added a comment. This was very briefly discussed at today's sync up call. We were running short on time, so we didn't get a chance to talk through it, but there did seem to be a consensus that discussion on the interface implications was needed. This should hopefully be on the agenda

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu updated this revision to Diff 445690. zixuan-wu added a comment. Remove unnecessary include header. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129824/new/ https://reviews.llvm.org/D129824 Files: clang/lib/Driver/Driver.cpp clang/test/Driver/riscv-abi.c

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu added inline comments. Comment at: clang/lib/Driver/Driver.cpp:660 + +if (RVArch == llvm::Triple::riscv64 && +ArchName.startswith_insensitive("rv32")) eopXD wrote: > Do we need to throw error (or warning) when these two (`RVArch` and >

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

eopXD added inline comments. Comment at: clang/lib/Driver/Driver.cpp:660 + +if (RVArch == llvm::Triple::riscv64 && +ArchName.startswith_insensitive("rv32")) Do we need to throw error (or warning) when these two (`RVArch` and `ArchName`) don't match?

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu added a comment. In D129824#3653895 , @jrtc27 wrote: > Does GCC allow this or not? Because this strikes me as a bad idea at first > sight… GCC can deduce from -mcpu but not -mabi. > riscv64-unknown-linux-gnu-gcc a.c -mabi=ilp32 > cc1: error:

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

jrtc27 added a comment. Does GCC allow this or not? Because this strikes me as a bad idea at first sight… Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129824/new/ https://reviews.llvm.org/D129824 ___

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

zixuan-wu created this revision. zixuan-wu added reviewers: simoncook, lenary, asb. Herald added subscribers: sunshaoce, VincentWu, luke957, StephenFan, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult,