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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, only minor nits found (feel free to ignore any that you disagree with).



================
Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:346
 
+// RVVRequire should be sync with target features, but only
+// required features used in riscv_vector.td.
----------------



================
Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:362
+
+  // Overloaded intrinsic name, could be empty if it can be computed from Name
+  // e.g. vadd
----------------



================
Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:400
+  // Number of fields, greater than 1 if it's segment load/store.
+  uint8_t NF;
+};
----------------



================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:32
+
+// Function definition of a RVV intrinsic
+struct RVVIntrinsicDef {
----------------



================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:40
+
+  /// Mapping to which clang built-in function, e.g. __builtin_rvv_vadd
+  std::string BuiltinName;
----------------



================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:131
+namespace {
+class RISCVIntrinsicManagerImpl : public clang::sema::RISCVIntrinsicManager {
+private:
----------------
You can drop all uses of `clang::` in this file.


================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:181-187
+    auto ProtoSeq =
+        ProtoSeq2ArrayRef(Record.PrototypeIndex, Record.PrototypeLength);
+    auto ProtoMaskSeq = ProtoSeq2ArrayRef(Record.MaskedPrototypeIndex,
+                                          Record.MaskedPrototypeLength);
+    auto SuffixProto =
+        ProtoSeq2ArrayRef(Record.SuffixIndex, Record.SuffixLength);
+    auto OverloadedSuffixProto = ProtoSeq2ArrayRef(Record.OverloadedSuffixIndex,
----------------
Please don't use `auto` when the type is not spelled out in the initialization.


================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:307-308
+  // Skip return type, and convert RVVType to QualType for arguments.
+  for (size_t i = 1; i < SigLength; ++i)
+    ArgTypes.push_back(RVVType2Qual(Context, Sigs[i]));
+
----------------



================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:63
+  // Number of field, large than 1 if it's segment load/store.
+  unsigned NF;
+};
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617