[PATCH] D73233: [mlir] Add option to use custom base class for dialect in LLVMIRIntrinsicGen.

[Alex Zinenko via Phabricator via llvm-commits]

ftynse requested changes to this revision.
ftynse added a comment.
This revision now requires changes to proceed.

Looks good in general, I am suggesting to push it one step further.



================
Comment at: mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp:215
+  os << "def LLVM_" << intr.getProperRecordName() << " : " << opBaseClass
+     << "<\"intr." << intr.getOperationName() << "\", [";
   mlir::interleaveComma(traits, os);
----------------
I wold consider dropping the "intr." part here, and rather defining a new LLVM_IntrOp that derives LLVM_Op in Tablegen and using your new functionality to obtain `llvm.intr.*` names. Other sets of intrinsics don't necessarily follow this naming convention (e.g. we have `nvvm.barrier0`, not `nvvm.intr.barrier0` and the intrinsic/operation makes sense only for the "core" LLVM parts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73233