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

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache added inline comments.


================
Comment at: mlir/lib/Conversion/LinalgToLLVM/LinalgToLLVM.cpp:179
+
+    if (!dstType.hasStaticShape())
+      return matchFailure();
----------------
ftynse wrote:
> 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...
I am not sure I follow. If the destination does not have static shape we can early exit and no need to compute the strides.
If it does, we still need to check that it has static strides, which happens below. 


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72316