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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
