craig.topper added inline comments.
================
Comment at: clang/include/clang/Basic/riscv_vector.td:90
+// equivalent integer vector type with EEW and corresponding ELMUL (elmul
=
+// (eew/sew) * lmul). Fore example, vector type is __rvv_float16m4
+// (SEW=16, LMUL=4) and Log2EEW is 3 (EEW=8), and then equivalent vector
----------------
Fore -> For
================
Comment at: clang/include/clang/Basic/riscv_vector.td:93
+// type is __rvv_uint8m2_t (elmul=(8/16)*4 = 2). Ignore to define a new
+// builtins if its qeuivalent type has illegal lmul.
//
----------------
equivalent*
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:95
+ const std::string &getShortStr() {
+ if (ShortStr.empty())
+ initShortStr();
----------------
Why would it be empty? Don't we call initShortStr in the constructor? This at
least needs a comment explaining why it is different than the other get*Str
functions.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:621
+ // Extract and compute complex type transformer. It can only appear one time.
+ const Regex ComplexEx("\\((.*:.*)\\).*");
+ SmallVector<StringRef> Matches;
----------------
Do we really need a regex here? Can't we just check that it starts with '(' and
then do a find for the ')'?
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:626
+ "only allow one complex type transformer");
+ Transformer.consume_front(Twine("(" + Matches[1] + ")").str());
+ auto UpdateAndCheckComplexProto = [&]() {
----------------
Can we do
```
Transformer = Transformer.drop_front(Matches[1].size() + 2);
```
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1011
+ size_t Idx = 0;
+ // Bypass complex prototype because it could have primitive type
+ // char.
----------------
I'd write this as "Skip over complex prototype because it could contain
primitive type character."
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1026
+ parsePrototypes(Prototypes, SuffixStrs,
+ [&](StringRef Proto, SmallVector<std::string> &Result) {
+ auto T = computeType(Type, Log2LMUL, Proto);
----------------
You don't need to pass SuffixStrs into parsePrototypes you can just capture it
in the lambda and use SuffixStrs.push_back() in the lambda.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1066
+ parsePrototypes(Prototypes, ProtoSeq,
+ [](StringRef Proto, SmallVector<std::string> &Result) {
+ Result.push_back(Proto.str());
----------------
Just capture ProtoSeq in the lambda capture list and use ProtoSeq.push_back in
the body.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98848/new/
https://reviews.llvm.org/D98848
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
