[PATCH] D74797: [mlir] [VectorOps] Framework for progressive lowering of 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.

Thanks Aart!



================
Comment at: mlir/lib/Dialect/VectorOps/VectorTransforms.cpp:880
+///               performance of generated code
 class ContractionOpLowering : public OpRewritePattern<vector::ContractionOp> {
 public:
----------------
As discussed in person, let's add a TODO to separate the transpose cases out into a separate vector transpose operation.


================
Comment at: mlir/lib/Dialect/VectorOps/VectorTransforms.cpp:951
+private:
+  Value lowerContract(vector::ContractionOp op, int64_t lhsIndex,
+                      int64_t rhsIndex, PatternRewriter &rewriter) const {
----------------
Add a TODO to consider using the existing vector ContractionOp unrolling in VectorTransforms.cpp mlir::vector::unrollSingleResultOpMatchingType(...).
That transform takes a target vector size and supports any iteration order during unrolling: (i, j, k), (i, k, j), etc...


================
Comment at: mlir/lib/Dialect/VectorOps/VectorTransforms.cpp:1075
+  // TODO(ajcbik): why don't we have a generalized reshaping-load for this?
+  static Value reshapeLoad(Location loc, Value val, VectorType type,
+                           int64_t index, int64_t pos,
----------------
Please add a TODO to see if you can use the vector ReshapeOp for these reshapes in the future. You could emit a ReshapeOp here, then have a separate pattern for lowering the ReshapeOps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74797