[PATCH] D77133: [mlir] Add an out-of-tree dialect example

[River Riddle via Phabricator via llvm-commits]

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

Looks great.



================
Comment at: mlir/examples/standalone/include/Standalone/StandaloneDialect.h:17
+
+class StandaloneDialect : public Dialect {
+public:
----------------
Please use the dialect declaration generator instead. i.e. -gen-dialect-decls


================
Comment at: mlir/examples/standalone/include/Standalone/StandaloneOps.td:18
+    let description = [{
+        The `standalone.foo` operation illustrates how to define a new operation
+        in a dialect. It uses an operation trait to declare that it has no side
----------------
This looks like it needs to be wrapped at 80 characters.


================
Comment at: mlir/examples/standalone/lib/Standalone/StandaloneOps.cpp:15
+
+// Parse a 'standalone.foo' operation.
+// op ::= <ssa-id> `=` `standalone.foo` <ssa-use> <attr-dict> `:` <type>
----------------
nit: Please use /// for top-level comments


================
Comment at: mlir/examples/standalone/lib/Standalone/StandaloneOps.cpp:22
+  // Parse the custom assembly format.
+  if (parser.parseOperand(operand) ||
+      parser.parseOptionalAttrDict(result.attributes) ||
----------------
Should you be using the declarative assembly format instead?

https://mlir.llvm.org/docs/OpDefinitions/#declarative-assembly-format


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

https://reviews.llvm.org/D77133