[PATCH] D80393: [mlir][Vector] Add more vector.contract -> outerproduct lowerings and fix vector.contract type inference.

[Aart Bik via Phabricator via llvm-commits]

aartbik added inline comments.


================
Comment at: mlir/lib/Dialect/Vector/VectorOps.cpp:302
+        }));
+    expected = VectorType::get(expectedShape, resVectorType.getElementType());
+    if (resVectorType != expected || accVectorType != expected)
----------------
Nit: one would expect 'expected' to only be set when the method returns true; likewise, for the scalar case, the parameter is not set (obviously since it is a vector); since it is a local method it is okay to have a very specific interface, but since you both test the return value and expected contents with !, as well as print the expected contents on failure, the API feels a bit convoluted


================
Comment at: mlir/test/Dialect/Vector/vector-contract-transforms.mlir:3
 // RUN: mlir-opt %s -test-vector-contraction-conversion=vector-lower-matrix-intrinsics=1 | FileCheck %s --check-prefix=MATRIX --dump-input-on-failure
-// RUN: mlir-opt %s -test-vector-contraction-conversion=vector-outerproduct=1 | FileCheck %s --check-prefix=OUTERPRODUCT --dump-input-on-failure
+// RUN: mlir-opt %s -test-vector-contraction-conversion=vector-outerproduct=1 -mlir-disable-threading
+//| FileCheck %s --check-prefix=OUTERPRODUCT --dump-input-on-failure
----------------
why did the threading flag get into play here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80393