[PATCH] D72808: [mlir] [VectorOps] Lowering of vector.extract_slices to LLVM IR

Mon Jan 20 19:44:48 PST 2020

nicolasvasilache requested changes to this revision.
nicolasvasilache added inline comments.
This revision now requires changes to proceed.

================
Comment at: mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp:1015
+
+    // Compute the strides between slices in each dimension.
+    SmallVector<int64_t, 4> sliceStrides(rank);
----------------
aartbik wrote:
> nicolasvasilache wrote:
> > Strides is probably become overloaded, maybe we should rename `strided_slice` and `insert_strided_slice` as Insert/ExtractSlice and these ops as Insert/ExtractSliceTuple or something along those lines?
> > In particular the `stride` attribute is not really a stride but a `step`.
> > 
> > In any case, we should have help functions for computing this.
> > Could you please reuse/refactor/adapt the first functions in VectorTransforms.cpp and move the to VectorOps/Utils.h as appropriate?
> > Thanks!
> I see similar TODOs elsewhere, so let's postpone that into one CL later.
I think with the changes and proto we discussed offline, this will change significantly before landing?
I can't remember whether you'll still need to use existing utils.

Marking as "request changes" until we see the new E2E impl.  

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72808