[PATCH] D112534: [PoC][RISCV] Use an attribute to declare C intrinsics with different policy.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:18614
+  auto *PolicyAttr = E->getCalleeDecl()->getAttr<RISCVVPolicyAttr>();
+  size_t PolicyValue;
 
----------------
Why size_t? This would be the size_t of the host machine that's building/running the compiler and would have no connection to the architecture being targetted.


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:843
+      if (hasPolicy()) {
+        OS << "  if (PolicyAttr) {\n";
+        OS << "    switch (PolicyAttr->getPolicy()) {\n";
----------------
Do we need to emit this switch for every builtin? Couldn't we assign `PolicyValue` before including the autogenerated file and only builtins that have a policy would add `PolicyValue` it to their `Ops` vector?


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:914
+    // Emit function arguments
+    if (!InputTypes.empty()) {
+      ListSeparator LS;
----------------
Does the `InputTypes.empty()` check provide any value. Looks like it just prevents constructing a `ListSeparator` that wouldn't be used, but I would think that's cheap to construct. I know this was copied from the function above, so my question applies there too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112534