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

[Kito Cheng via Phabricator via llvm-commits]

kito-cheng added inline comments.


================
Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:114
   bool operator>(const PrototypeDescriptor &PD) const {
-    return !(PD.PT <= PT && PD.VTM <= VTM && PD.TM <= TM);
+    if (PD.PT != PT)
+      return PD.PT > PT;
----------------
craig.topper wrote:
> This can be written as
> 
> `return std::tie(PD.PT, PD.VTM, PD.TM) > std::tie(PT, VTM, TM);`
> 
> Though it's still surprising that PD is on the left. This is operator> but the implementation looks more like operator<.
Rewrite as `operator>` and updated the use site, thank!


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:254
+  if (Signature.empty())
+    return 0;
+
----------------
khchen wrote:
> Does it mean empty Signature always at 0?
> If yes,  maybe we could check the table from Index = 1 in below loop?
Actually empty signature could be indicate into any index, we have hold length when we emit the index. 

Add comment to mention that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617