[PATCH] D74880: [mlir] [VectorOps] Multi-dim reductions for lowering vector.contract

[Andy Davis via Phabricator via llvm-commits]

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

Looks good, thanks Aart!  One question, is the unrolling here deterministic in that the same op will always produce the same sequence of reductions after unrolling?  Context is numerical stability across test runs....



================
Comment at: mlir/lib/Dialect/VectorOps/VectorTransforms.cpp:1031
+        rewriter.getArrayAttr(adjustIter(op.iterator_types(), iterIndex));
+    // Unroll into a series of lower dimensional vector.contract ops.
+    // By feeding the initial accumulator into the first contraction,
----------------
Maybe not in this change, but it would be great if this unrolling code could reuse the unrolling code already in this file: mlir::vector::unrollSingleResultOpMatchingType(


================
Comment at: mlir/lib/Dialect/VectorOps/VectorTransforms.cpp:1070
+      int64_t idx = it.index();
+      if (idx == index) {
+        continue;
----------------
s/idx/id.index()  don't see 'idx' used elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74880