[PATCH] D72317: [mlir][VectorOps] Implement insert_strided_slice conversion

[Andy Davis via Phabricator via llvm-commits]

andydavis1 accepted this revision.
andydavis1 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp:112
+// Helper that returns a subset of `arrayAttr` as a vector of int64_t.
+static SmallVector<int64_t, 4> getI64SubArray(ArrayAttr arrayAttr,
+                                              unsigned dropFront = 1,
----------------
I think I have a helper function like this in VectorOps.cpp  Need to unify these at some point.


================
Comment at: mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp:501
+// have different ranks. In this case:
+//   1. the proper subvector is extracted from the destination vector
+//   2. a new InsertStridedSlice op is created to insert the source in the
----------------
At first a little surprised to see that InsertStridedSliceOp would be extracting a vector first, but I'm guessing this is because of the rank difference. If this is the case, maybe comment that we need to extract a subvector of the same rank as the source?


================
Comment at: mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp:544
+                       /*dropFront=*/rankRest));
+    rewriter.replaceOp(op, inserted);
+    return matchSuccess();
----------------
replaceOpWithNewOp


================
Comment at: mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp:607
+        // be called multiple times by the PatternRewrite infrastructure (to
+        // avoid infinite recursion, but in this case we know what we are
+        // doing).
----------------
We know this terminates because the rank is reduced at each recursive step?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72317