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

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added inline comments.


================
Comment at: clang/lib/Parse/ParsePragma.cpp:3963
+        << PP.getSpelling(Tok) << "riscv" << /*Expected=*/true << "'intrinsic'";
+    return;
+  }
----------------
aaron.ballman wrote:
> It's fine to warn on this, but then you need to eat tokens until the end of directive is found so that parsing recovery is correct. e.g.,
> ```
> #pragma clang riscv int i = 12;
> ```
> See `HandlePragmaAttribute()` for an example (though you'll look for `eod` instead of `eof`).
Seems like it already work correctly, and I saw other HandlePragma also just return? I add a testcase to make sure it work.


================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:100
+    switch (Type->getElementBitwidth()) {
+    case 64:
+      QT = Context.DoubleTy;
----------------
aaron.ballman wrote:
> I almost hate to ask, but... `long double`? Any of the 16-bit float types?
Have 16 bit floating below, but we don't support long double in our intrinsic for now, add an assertion to make sure.


================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:121
+  // Transform the type to a pointer as the last step, if necessary.
+  if (Type->isPointer())
+    QT = Context.getPointerType(QT);
----------------
aaron.ballman wrote:
> Double-checking -- do you have to care about references as well?
We don't have any references type in argument type, so we don't care about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617