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

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 14 22:14:29 PDT 2020


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





More information about the llvm-commits mailing list