[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Zakk Chen via Phabricator via llvm-commits]

khchen accepted this revision.
khchen added a comment.
This revision is now accepted and ready to land.

LGTM. Other than that last comments.



================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:77
+  // Create compressed hsignature table from SemaRecords.
+  void init(const std::vector<SemaRecord> &SemaRecords);
+
----------------
please use ArrayRef


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:110
+  void createRVVIntrinsics(std::vector<std::unique_ptr<RVVIntrinsic>> &Out,
+                           std::vector<SemaRecord> *SemaRecords);
+  /// Create all intrinsics record and SemaSignatureTable from SemaRecords.
----------------
maybe SemaRecords could have default argument as nullptr.


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:114
+                                SemaSignatureTable &SST,
+                                const std::vector<SemaRecord> &SemaRecords);
+
----------------
please use ArrayRef


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:254
+  if (Signature.empty())
+    return 0;
+
----------------
Does it mean empty Signature always at 0?
If yes,  maybe we could check the table from Index = 1 in below loop?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111617/new/

https://reviews.llvm.org/D111617