[PATCH] D96843: [Clang][RISCV] Add vsetvl and vsetvlmax.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: clang/include/clang/Basic/riscv_vector.td:165
+  // and it will have to be provided manually. See IntrinsicTypes below.
+  bit HasManualCodegen = true;
+
----------------
Why do we need a flag? Can we just make ManualCodegen and ManualCodegenMask default to empty rather than a code sequence that we should never use? Then we can just detect them not being empty in place of the flag.


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:924
+    bool HasAutoDef = HeaderCodeStr.empty();
+    if (HeaderCodeStr.size()) {
+      HeaderCode += HeaderCodeStr.str();
----------------
!HeaderCodeStr.empty()


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1052
   uint8_t PrevExt = (*Defs.begin())->getRISCV_Extensions();
-  bool NeedEndif = emitExtDefStr(PrevExt, OS);
+  bool NeedEndif =
+      (*Defs.begin())->hasAutoDef() ? emitExtDefStr(PrevExt, OS) : false;
----------------
It doesn't make sense to me to skip the emitExtDefStr for !hasAutoDef. The intrinsics without a definition are still sorted with the others. If they happen to have non-zero extensions and on the boundary of the different extension regions in the sorted list we still need to manage the #if/#endif for that boundary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96843