[PATCH] D72316: [mlir][Linalg] Lower linalg.reshape to LLVM for the static case

[Alex Zinenko via Phabricator via llvm-commits]

ftynse marked an inline comment as done.
ftynse added inline comments.


================
Comment at: mlir/lib/Conversion/LinalgToLLVM/LinalgToLLVM.cpp:179
+
+    if (!dstType.hasStaticShape())
+      return matchFailure();
----------------
I wonder whether `hasStaticShape` is the right name here. It only checks the sizes, but not the strides, which is a bit counter-intuitive. Not for this commit certainly...


================
Comment at: mlir/lib/Conversion/LinalgToLLVM/LinalgToLLVM.cpp:189
+      return matchFailure();
+
+    edsc::ScopedContext context(rewriter, op->getLoc());
----------------
Can reshape do permutations using affine maps? It does not seem supported here, neither it is guarded against in the matching.


================
Comment at: mlir/lib/Conversion/LinalgToLLVM/LinalgToLLVM.cpp:198
+    for (auto en : llvm::enumerate(dstType.getShape()))
+      desc.setSize(en.index(), constant_index(en.value()));
+    for (auto en : llvm::enumerate(strides))
----------------
`MemRefDescriptor` has `setConstantSize` for this purposes, consider replicating it in `BaseViewConversionHelper` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72316