khchen added a comment.
Do we need to have some tests in `clang/test/PCH/` for new #pragma?
================
Comment at: clang/lib/Sema/SemaLookup.cpp:932
+ if (DeclareRVVBuiltins) {
+ if (GetRVVBuiltinInfo(*this, R, II, PP)) {
+ return true;
----------------
Don’t Use Braces on Simple Single-Statement Bodies.
================
Comment at: clang/lib/Support/RISCVVIntrinsicUtils.cpp:884
RVVIntrinsic::getSuffixStr(BasicType Type, int Log2LMUL,
- const llvm::SmallVector<TypeProfile> &TypeProfiles)
{
+ const llvm::ArrayRef<TypeProfile> &TypeProfiles) {
SmallVector<std::string> SuffixStrs;
----------------
maybe this changed should be in another NFC patch.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:372
StringRef Name = R->getValueAsString("Name");
- StringRef SuffixProto = R->getValueAsString("Suffix");
+ StringRef Suffix = R->getValueAsString("Suffix");
StringRef MangledName = R->getValueAsString("MangledName");
----------------
maybe all renaming stuffs should be in another NFC patch.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:480
+ // They are handled by riscv_vector.h
+ if (Name == "vsetvli" || Name == "vsetvlimax")
+ continue;
----------------
I feel little tricky to checking the name here. what do you mean they are
handled by riscv_vector.h?
do you mean they have `vsetvl_macro:RVVHeader`?
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:611
+ for (const auto &SR : SemaRecords) {
+ // Output *MUST* sync with RVVIntrinsicRecord in SemaRVVLookup.cpp.
+ OS << "{"
----------------
I'm thinking is it possible to have an unittest or test to make sure we won't
screw up in the future implementation?
Is it possible to have unittest to test implement really have `sync` correctly?
Is it easy to debug the mismatch problem during implementation without any new
test added?
We will add a new implementation (really cool speed up and meaningful
improvement), but unfortunately we don't have any tests, that make me a little
hesitating...
What do you think?
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
