[PATCH] D73983: [mlir][ODS] Add documentation for the declarative assembly format.

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/docs/OpDefinitions.md:603
+  - Represents the type of the given input.
+  - `input` must be either an operand, result, the `operands` directive, or the
+    `results` directive.
----------------
antiagainst wrote:
> Say a bit more about variadic operand/result?
I added, and linked to, "variable" to be a bit more specific. The operand/result here refers to the thing registered on the operation in ODS, so variadic isn't differentiated from non-variadic.


================
Comment at: mlir/docs/OpDefinitions.md:623
+
+1. The output and operation name are never shown as they are fixed and cannot be
+   altered.
----------------
jpienaar wrote:
> It might be good to point out that this is a requirement of parsing custom forms in general and not specific to this.
Several of these technically apply to the custom form in general, i.e. you should really make sure all operands are covered. Is there a specific way that you suggest I point this out?


================
Comment at: mlir/docs/OpDefinitions.md:631
+1. The `attr-dict` directive must always be present.
+1. Must not contain overlapping information; e.g. multiple instances of
+   'attr-dict', types, operands, etc.
----------------
jpienaar wrote:
> Are optional attributes handled? I'm assuming no special handling, only via attr-dict.
Yep. There is no distinction between optional/non-optional attributes. We just elide any attributes that were explicitly present in the format from the attribute dictionary. 


================
Comment at: mlir/docs/OpDefinitions.md:632
+1. Must not contain overlapping information; e.g. multiple instances of
+   'attr-dict', types, operands, etc.
+
----------------
antiagainst wrote:
> Should we also mention what happens when we have both attr-dict and specific attributes using variables?
See response to Jacques.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73983