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

[Alex Bradbury via Phabricator via llvm-commits]

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 can fix them and request them for merging into the LLVM 11 branch).



================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:556
   //
-  // The logic uses the following, in order:
+  // The logic uses in GCC 9.2.0 as the following, in order:
   // 1. Explicit choices using `--with-abi=`
----------------
"The logic used in GCC 9.2.0 i the following, in order:"


================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:566
+  //
+  // In order to make chosing logic more clear, clang uses the following logic,
+  // in order:
----------------
"Clang uses the following logic;"


================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:569
+  // 1. Explicit choices using `-mabi=`
+  // 2. A default based on clang's choosing arch logic. The order of default
+  // arch login is `-march`, `-mcpu` and target triples`.
----------------
"A default based on the architecture as determined by getRISCVArch"


================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:576
 
-  // 2. Choose a default based on `-march=`
+  // 2. Choose a default based on clang's choosing arch logic.
   //
----------------
"Choose a default based on the target architecture"


================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:615
   //
-  // The logic uses the following, in order:
+  // The logic uses in GCC 9.2.0 as the following, in order:
   // 1. Explicit choices using `--with-arch=`
----------------
"The logic used in GCC 9.2.0 is the following, in order:"


================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:627
+  // 1. Explicit choices using `-march=`
+  // 2. Based on `-mcpu` if target cpu has default isa extension feature
+  // 3. A default based on `-mabi`, if provided
----------------
Based on `-mcpu` if the target CPU has a default ISA string


================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.h:29
                        const llvm::Triple &Triple);
+StringRef getMArchFromMabi(StringRef MABI);
+StringRef getMArchFromTriple(const llvm::Triple &Triple);
----------------
These functions aren't defined anywhere - delete?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124