[Mlir-commits] [mlir] [mlir][memref] Canonicalize memref.reinterpret_cast when offset/sizes/strides are constants. (PR #163505)

[llvmlistbot at llvm.org]

jeanPerier wrote:

> What I'd use is some value like `fir.unknown.size` which isn't `-1` but lowers to it when you're rewriting out of memref.

Thanks, that is an interesting solution that would definitely solves the FIR to Memref issue at hand!

> there's a decent amount of code that - for example - assumes that you can take the product of all memref.dims and get a meaningful value at runtime.

Right, and using `fir.unknown.size`  would not solve the issue that any core MLIR pass working on memref may just create a temporary copy for whatever reason assuming it can always do it, while this is just not possible with Fortran assumed size.
That is why I tend to agree with @matthias-springer that passes that rewrite fir.box to memref should only do it when the memref specifications supports representing the related Fortran array.

This means that Flang just cannot rely on MLIR memref dialect and related optimization passes to optimize code containing assumed-size manipulations (which are rather common in old and not so old Fortran code).

> That's valid, but not checkable at compile time.

Side note that I think this kind of things where, after constant folding/inlining, illegal constant values inputs reaches operations in code that cannot easily be proved to be reachable is a bit of a hassle. I think that it would be nice if there was an option for the verification pass to replace illegal operation by traps (or whatever the driver wants, personally I am not a fan of unreachable, it makes debugging bad user code very hard because of code pruning). I guess some verifier errors should always be fatal, but bad input that does not have to be compile time constant should not IMHO. We are not hitting the issue too much with flang, so I have not spent time trying to come up with something, but why not at some point if there is some interest.

https://github.com/llvm/llvm-project/pull/163505