[PATCH] D75699: [mlir] [VectorOps] Merge VectorReduction/VectorReductionV2 into one Op

Thu Mar 5 12:07:09 PST 2020

rriddle added inline comments.

================
Comment at: mlir/lib/Dialect/VectorOps/VectorOps.cpp:98
+
+static ParseResult parseReductionOp(OpAsmParser &parser,
+                                    OperationState &result) {
----------------
rriddle wrote:
> aartbik wrote:
> > rriddle wrote:
> > > nicolasvasilache wrote:
> > > > Re assembly format, file a bug for @rriddle linking to this diff?
> > > > Another way, if variadic is more tricky to impl automatically (because of say combinatorial effects with variadic operands), could be to allow multiple declarative forms and a simple convention on optional arg ordering?
> > > It already does: https://mlir.llvm.org/docs/OpDefinitions/#optional-groups
> > Ah, very interesting! The customer parsing/printing here has a minor advantage of printing less type information (since most of it is inferred), but that is good to keep in mind. Thanks.
> I think the only problem you will run into is with inferring the type of the optional operand.
The assembly format can already infer types in a majority of cases. I don't think this explicit case is handled right now, though it should be easy to support. (i.e. inferring the type of an optional operand)

https://mlir.llvm.org/docs/OpDefinitions/#type-inferrence

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75699