[PATCH] D73468: [mlir] LLVM dialect: Generate conversions between EnumAttrCase and LLVM API

[Lei Zhang via Phabricator via llvm-commits]

antiagainst accepted this revision.
antiagainst added a comment.
This revision is now accepted and ready to land.

In D73468#1849667 <https://reviews.llvm.org/D73468#1849667>, @ftynse wrote:

> This is a follow-up to that.


Oh, sorry, wasn't checking carefully. :)



================
Comment at: mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td:66
 // Case of the LLVM enum attribute backed by I64Attr with customized string
 // representation that corresponds to what is visible in the textual IR form.
+class LLVM_EnumAttrCase<string cppSym, string irSym, string llvmSym, int val> :
----------------
We've have quite a few `*Sym` here. What about adding some documentation for them (potentially with a good example)?


================
Comment at: mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td:76
 // corresponding to what is visible in the textual IR form.
-class LLVM_EnumAttr<string name, string description,
+class LLVM_EnumAttr<string name, string llvmName, string description,
                     list<LLVM_EnumAttrCase> cases> :
----------------
Same here.


================
Comment at: mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp:277
+    genEnumToLLVMConversion("gen-enum-to-llvmir-conversions",
+                            "Generate conversions of EnumAttrs to LLVM",
+                            emitEnumConversionDefs);
----------------
"to LLVM IR" to be specific. LLVM can mean both the C++ API and the IR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73468