[PATCH] D136377: [mlir][MemRefToLLVM] Remove the code for lowering subview

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: mlir/test/Conversion/MemRefToLLVM/expand-then-convert-to-llvm.mlir:8-13
+// Note: We run CSE in that test to get rid of duplicated
+// unrealized_conversion_cast that are inserted with
+// convert-memref-to-llvm and then convert-arith-to-llvm.
+// The final code is still not perfect, because we have
+// noop unrealized_conversion_cast from i64 to index
+// and back.
----------------
qcolombet wrote:
> ftynse wrote:
> > I'd expect `-reconcile-unrealized-casts` to fix that.
> Ah thanks, let me add that to the pipeline.
That fix that but only if I run `convert-func-to-llvm` before because otherwise I end up with illegal unrealized casts for function arguments. (See example below.)

@ftynse, I'm inclined to leave the test as is since lowering `func` to `llvm` creates a lot of unnecessary noise IMHO. (E.g., memref are expanded in a bunch of arguments.)
What do you think?

```
error: failed to legalize operation 'builtin.unrealized_conversion_cast' that was explicitly marked illegal
    %0 = builtin.unrealized_conversion_cast %arg1 : index to i64
         ^
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136377