frasercrmck added a comment.
Just nits from me.
================
Comment at: clang/include/clang/Sema/RISCVIntrinsicManager.h:9
+//
+// This file defines the RISCVIntrinsicManager, which handle RISC-V vector
+// intrinsic functions.
----------------
`handles`
================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:49
+ // Index of RISCVIntrinsicManagerImpl::IntrinsicList.
+ SmallVector<size_t, 8> Indexs;
+};
----------------
`Indices` (or `Indexes`)?
================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:218
+ for (int Log2LMUL = -3; Log2LMUL <= 3; Log2LMUL++) {
+ if (!(Record.Log2LMULMask & (1 << (Log2LMUL + 3)))) {
+ continue;
----------------
Drop curly braces
================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:185-186
+
Record.OverloadedSuffixSize);
+ for (int TypeRangeMaskShift = 0;
+ TypeRangeMaskShift <= static_cast<int>(BasicType::MaxOffset);
+ ++TypeRangeMaskShift) {
----------------
aaron.ballman wrote:
> Given that we're bit twiddling with this, I'd feel more comfortable if this
> was `unsigned int` rather than `int` (same for `BaseTypeI`).
+1
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:47
+
+ // Required extensions for this intrinsic.
+ unsigned RequiredExtension;
----------------
Comment is plural, variable is singular.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:111
+ std::vector<SemaRecord> *SemaRecords = nullptr);
+ /// Create all intrinsics record and SemaSignatureTable from SemaRecords.
+ void createRVVIntrinsicRecord(std::vector<RVVIntrinsicRecord> &Out,
----------------
all records, plural?
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:578
+
+ if (SemaRecords) {
+ // Create SemaRecord
----------------
`if (!SemaRecords) continue;`? Might make things a little more readable.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:637
+ Out.emplace_back(Record);
+ Out.back().Name = SR.Name.c_str();
+ Out.back().OverloadedName = SR.OverloadedName.c_str();
----------------
I assume the compiler's able to avoid recomputing `Out.back()` multiple times?
We could take a reference to `Out.back()` and use that, just in case?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111617/new/
https://reviews.llvm.org/D111617
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
