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

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added a comment.

In D71124#2010759 <https://reviews.llvm.org/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 the outcome of that discussion?


Sorry, I don't remember the outcome of discussion...
I thought it make sense if both mcpu and march are specified, and march extensions overrides the mcpu extension
This also allows users to specific superset or subset of march.



================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:501-516
-  // 3. Choose a default based on the triple
-  //
-  // We deviate from GCC's defaults here:
-  // - On `riscv{XLEN}-unknown-elf` we use the integer calling convention only.
-  // - On all other OSs we use the double floating point calling convention.
-  if (Triple.getArch() == llvm::Triple::riscv32) {
-    if (Triple.getOS() == llvm::Triple::UnknownOS)
----------------
lenary wrote:
> What's the justification for removing this code?
I changed the logic in how to get the default march by calling `getRISCVArch(Args, Triple);`
`getRISCVArch` can also get the default march based on the triple, so we don't need above code now.



================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:310
+  case llvm::Triple::riscv64:
+    if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
+      return A->getValue();
----------------
lenary wrote:
> Should we be doing more validation here?
I think we don't need to validation here, we have validation in RISCVTargetInfo::isValidCPUName. You can see it work in test/Driver/riscv-cpus.c#L22.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124