craig.topper added a comment.
This is a very incomplete review, but I need to go eat dinner
================
Comment at: clang/include/clang/Basic/riscv_vector.td:201
+// gen-riscv-vector-test.
+// gen-riscv-v-tests.sh will define each marco to generate each intrinsic test
+// in different files. It mean adding the new definition also need to update
----------------
marco->macro
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:124
+
+// TODO rafactor Intrinsic class design after support all intrinsic
combination.
+class Intrinsic {
----------------
rafactor->refactor
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1000
+ SmallVector<std::string, 8> ProtoSeq;
+ const StringRef Pirmaries("evwqom0ztc");
+ size_t start = 0;
----------------
Is this supposed to be Primaries or some other word?
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1055
+ RVVTypes Types;
+ for (std::string Proto : PrototypeSeq) {
+ auto T = computeType(BT, LMUL, Proto);
----------------
Can Proto be a const std::string & ?
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1057
+ auto T = computeType(BT, LMUL, Proto);
+ if (!T.hasValue()) {
+ return llvm::None;
----------------
Drop curly braces
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1068
+ StringRef Proto) {
+ TypeString Idx = Twine(BT + utostr(LMUL) + Proto).str();
+ // search first
----------------
Use Twine(LMUL) instead of utostr. That should avoid creating std::string
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1112
+ }
+ if (ExtStrings.size())
+ OS << "#endif\n\n";
----------------
!ExtStrings.empty()
================
Comment at: clang/utils/TestUtils/gen-riscv-v-tests.sh:21
+gen_tests(){
+# op_list have marco name used in riscv_vector.td
+ local op_list="VADD VFADD"
----------------
marco->macro
================
Comment at: clang/utils/TestUtils/gen-riscv-v-tests.sh:22
+# op_list have marco name used in riscv_vector.td
+ local op_list="VADD VFADD"
+ local option="$1"
----------------
It feels a little weird that this list is in the script and not derived from
the td file automatically somehow. Ideally we wouldn't have to update the
script every time a new set of intrinsics is added.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95016/new/
https://reviews.llvm.org/D95016
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
