[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen added inline comments.


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:899
+    // (operand) in ProtoSeq. ProtoSeq[0] is output operand.
+    SmallVector<std::string, 8> ProtoSeq;
+    const StringRef Primaries("evwqom0ztc");
----------------
craig.topper wrote:
> I think this is something like
> 
> ```
> while (!Prototypes.empty()) {
>  auto Idx = Prototypes.find_first_of(Primaries);
>  assert(Idx != StringRef::npos);
>  ProtoSeq.push_back(Prototypes.slice(0, Idx+1).str());
>  Prototypes = Prototypes.drop_front(Idx+1);
> }
> ```
> 
> Which might be easier to understand.
Thanks, it's clear.


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1016
+  SmallVector<std::string> ExtVector;
+  // D implies F
+  if (Extents & RISCV_Extension::F) {
----------------
craig.topper wrote:
> I don't understand this. It says D implies F but we're checking for F. So that seems like F implies D.
Remove extension implying rule. 
I thought we don't need to consider implying rule because it's clang's responsibility. 
The original thinking was when predecessor "only" defines `__riscv_d`, and floating instruction also need to supported. But in fact,  when enabling __riscv_d predecessor will also define `__riscv_f`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016