[PATCH] D74370: [mlir] [VectorOps] Implement vector.reduce operation

[Aart Bik via Phabricator via llvm-commits]

aartbik marked 4 inline comments as done.
aartbik added inline comments.


================
Comment at: mlir/include/mlir/Dialect/VectorOps/VectorOps.td:210
+  let assemblyFormat = [{
+    $kind `,` $vector attr-dict `:` type($vector) `into` type($dest)
+  }];
----------------
nicolasvasilache wrote:
> if we had better dependent type support, would we want to drop the `into f32` part of the op?
Yes, at the moment that would make sense to get more concise syntax. In the longer run, we may want to add mixed type versions (e.g. i32 = reduce_max_index f32) where having the type may be more useful.


================
Comment at: mlir/test/Dialect/VectorOps/invalid.mlir:998
+  // expected-error at +1 {{'vector.reduction' op unknown reduction kind: joho}}
+  %0 = vector.reduction "joho", %arg0 : vector<16xf32> into f32
+}
----------------
nicolasvasilache wrote:
> what's a "joho" ? :)
Ha! Joho has been my "scratch" word since my very first computer (files names joho are always temporary, invalid syntax is shown with a joho keyword!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74370