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

[Marcello Maggioni via Phabricator via llvm-commits]

kariddi marked an inline comment as done.
kariddi added inline comments.


================
Comment at: mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp:215
+  os << "def LLVM_" << intr.getProperRecordName() << " : " << opBaseClass
+     << "<\"intr." << intr.getOperationName() << "\", [";
   mlir::interleaveComma(traits, os);
----------------
ftynse wrote:
> 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.
Hi Alex, thanks for the comment, I agree, that's a good idea! I'm almost done with it, but I noticed that the strings produced actually look like "llvm.intr.llvm.is.constant" or "nvvm.intr.nvvm.barrier0"

For example this is the current code generated:
LLVM_Op<"intr.nvvm.barrier0", ...

Do you think we should drop the nvvm after the "intr." as well (as it is redundant with respect to the dialect usually)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73233