[PATCH] D71124: [RISCV] support clang driver to select cpu

jrtc27 added a comment. In D71124#2423633 , @oceanfish81 wrote: > @khchen , > how should I obtain available (v)CPU features? > Any sample code, that should work on RISC-V? Probably `getauxval(AT_HWCAP)`; both Linux and FreeBSD put the ISA string in the

[PATCH] D71124: [RISCV] support clang driver to select cpu

oceanfish81 added a comment. @khchen , how should I obtain available (v)CPU features? Any sample code, that should work on RISC-V? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen added a comment. > How does the CLI options correspond with the back-end capabilities? `clang -print-supported-cpus` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124

[PATCH] D71124: [RISCV] support clang driver to select cpu

oceanfish81 added a comment. @khchen , what is the recomended way to get the current CPU features? I see that there are some ISA extensions defined here . So RISC-V back-end has another API, for both identifying

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen added a comment. In D71124#2415424 , @oceanfish81 wrote: > I tried to build the following: > >

[PATCH] D71124: [RISCV] support clang driver to select cpu

oceanfish81 added a comment. Herald added subscribers: frasercrmck, dexonsmith, NickHung, pengfei. I tried to build the following: https://gist.github.com/advancedwebdeveloper/09033869445a09a2e297a73387d46c41/raw/bf7dac33e256621c445f9e61f88a832e50d901a3/llvm_cpu_features_investigation.cpp but

[PATCH] D71124: [RISCV] support clang driver to select cpu

This revision was automatically updated to reflect the committed changes. Closed by commit rG294d1eae75bf: [RISCV] Add support for -mcpu option. (authored by Zakk Chen zakk.c...@sifive.com). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/

[PATCH] D71124: [RISCV] support clang driver to select cpu

asb accepted this revision. asb added a comment. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124 ___ cfe-commits mailing list

[PATCH] D71124: [RISCV] support clang driver to select cpu

lenary accepted this revision. lenary added a comment. This revision is now accepted and ready to land. I am happy with this. I think we should get it landed so we can backport it to the LLVM 11 branch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen updated this revision to Diff 278197. khchen added a comment. address @lenary's comment, good catch!! thanks!! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124 Files:

[PATCH] D71124: [RISCV] support clang driver to select cpu

lenary added a comment. Thanks for the fix! Please can you also update `clang/test/Misc/target-invalid-cpu-note.c` for riscv32 and riscv64 - it's likely you'll need the fix in the inline note below. Also, there are a bunch of clang-tidy issues, which I think we can fix now rather than later.

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen updated this revision to Diff 278106. khchen marked 8 inline comments as done. khchen added a comment. address @asb's comment. thanks. Thanks for @lenary bug report. I cannot reproduce the crash in local, but I though the problem is cause by removing "Choose a default based on the

[PATCH] D71124: [RISCV] support clang driver to select cpu

asb added a comment. I've added some suggestions to clarify the code comments. I think before landing it would be good to address the crash Sam pointed out for an invalid -march, but otherwise I think this looks good to me (at least, it seems worth landing this and if further issues crop up we

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen added a comment. Hi @lenary This is normal behavior in current clang implementation, `-mcpu=?` flag does not interact with any flags, it just print out the all of ProcessorModel registered in backend.

[PATCH] D71124: [RISCV] support clang driver to select cpu

lenary added a comment. I've got one major issue (inline below), and I'm confused by some other behaviour: When I run `clang --target=riscv64 -mcpu=?`, the list includes both `generic-rv32` and `generic-rv64`. It doesn't show only the 64-bit cpus. This is not changed by giving a full triple,

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen added a comment. @asb @lenary I thought this path is ready to land? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124 ___ cfe-commits mailing list

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen updated this revision to Diff 277067. khchen added a comment. avoid to check compiler version in testcase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124 Files: clang/lib/Basic/Targets/RISCV.cpp

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen updated this revision to Diff 277042. khchen added a comment. fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124 Files: clang/lib/Basic/Targets/RISCV.cpp clang/lib/Basic/Targets/RISCV.h

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen updated this revision to Diff 276951. khchen added a comment. addess asb's comment. [RISCV][Clang] Support -mcpu option. Summary: 1. gcc uses `-march` and `-mtune` flag to chose arch and pipeline model, but clang does not have `-mtune` flag, we uses `-mcpu` to chose both info. 2. Add

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen planned changes to this revision. khchen marked an inline comment as done. khchen added a comment. BTW, this patch depends on D77030 , which aim to avoid the forcing of any ProcessorModel to have `FeatureRVCHints` feature. But if we decide to keep the

[PATCH] D71124: [RISCV] support clang driver to select cpu

asb added a comment. This has been hanging around for a while, but I think we'd basically agreed this is the right logic. The comments have ended up referring to flags that don't exist on Clang making it a little hard to follow, and I've added a request to slightly expand testing. If you make

[PATCH] D71124: [RISCV] support clang driver to select cpu

lenary added a comment. I realise this is almost certainly something we want to land before the LLVM 11 branch date, as we included schedules in LLVM 10 with no way to use them, and would like users to be able to use them. I'll bring it up on the call tomorrow - I hope this PR implements what

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen updated this revision to Diff 261741. khchen added a comment. address kito's comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124 Files: clang/lib/Basic/Targets/RISCV.cpp

[PATCH] D71124: [RISCV] support clang driver to select cpu

kito-cheng added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:478 // 1. Explicit choices using `--with-abi=` - // 2. A default based on `--with-arch=`, if provided - // 3. A default based on the target triple's arch + // 2. A default based on

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen added a comment. In D71124#2012090 , @kito-cheng wrote: > Another proposal for -mcpu and -mtune: > > Decoupling the -mcpu and -mtune option, -mcpu only accept concrete CPU, and > -mtune for micro-arch/pipeline model, they accept different option

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen added a comment. In D71124#2010759 , @lenary wrote: > This is looking good. > > I remember we discussed this on the LLVM call a few weeks ago - there was a > discussion as to whether we should be prioritising `-march` or `-mcpu` - do > you recall

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen updated this revision to Diff 261165. khchen marked 9 inline comments as done. khchen added a comment. address @lenary's review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124 Files:

[PATCH] D71124: [RISCV] support clang driver to select cpu

kito-cheng added a comment. Another proposal for -mcpu and -mtune: Decoupling the -mcpu and -mtune option, -mcpu only accept concrete CPU, and -mtune for micro-arch/pipeline model, they accept different option set. e.g. -mcpu=sifive-e24 # Imply -march=rv32imafc -mtune=sifive-2-series

[PATCH] D71124: [RISCV] support clang driver to select cpu

lenary added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. This is looking good. I remember we discussed this on the LLVM call a few weeks ago - there was a discussion as to whether we should be prioritising `-march` or `-mcpu` - do you recall the outcome of

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen added a comment. @lenary (Sorry for the very late reply...) In D71124#1829998 , @lenary wrote: > In D71124#1792216 , @khchen wrote: > > > The problem is how `-mcpu` interact with explicitly specified

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen updated this revision to Diff 255377. khchen added reviewers: evandro, HsiangKai. khchen added a comment. Herald added a subscriber: hiraditya. update implementation and handle `-mcpu` with explicitly specified `-march`. If `-mcpu` has default march, explicitly `-march` will overwrite it.

[PATCH] D71124: [RISCV] support clang driver to select cpu

lenary added a comment. In D71124#1792216 , @khchen wrote: > The problem is how `-mcpu` interact with explicitly specified `-march` (or > target features). > > 1. in GCC, -mcpu is only used to chose the pipeline model, I think you mean "in GCC,

[PATCH] D71124: [RISCV] support clang driver to select cpu

evandro added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:164 + +static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"}, +{"rocket-rv32"}}; Strange formatting...

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen marked an inline comment as done. khchen added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:164 + +static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"}, +

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen planned changes to this revision. khchen marked an inline comment as done. khchen added a comment. The problem is how `-mcpu` interact with explicitly specified `-march` (or target features). 1. in GCC, -mcpu is only used to chose the pipeline model, 2. I also read this

[PATCH] D71124: [RISCV] support clang driver to select cpu

lenary added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:164 + +static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"}, +{"rocket-rv32"}}; khchen wrote: > lenary

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen marked 3 inline comments as done. khchen added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:164 + +static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"}, +

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen updated this revision to Diff 233082. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124 Files: clang/lib/Basic/Targets/RISCV.cpp clang/lib/Basic/Targets/RISCV.h

[PATCH] D71124: [RISCV] support clang driver to select cpu

lenary added reviewers: lenary, asb. lenary added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:164 + +static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"}, +{"rocket-rv32"}};

[PATCH] D71124: [RISCV] support clang driver to select cpu

Jim added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.h:46 + bool setCPU(const std::string ) override { +CPU = Name; I think this should test cpu name is valid first. And assign Name to CPU, if it is valid. Repository: rC Clang

[PATCH] D71124: [RISCV] support clang driver to select cpu

khchen created this revision. khchen added a project: clang. Herald added subscribers: cfe-commits, luismarques, apazos, sameer.abuasal, pzheng, s.egerton, lenary, Jim, benna, psnobl, jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, MaskRay, jrtc27,