[Mlir-commits] [mlir] [mlir][fold-memref-alias-ops] Add support for folding memref.expand_shape involving dynamic dims (PR #89093)

Prathamesh Tagore llvmlistbot at llvm.org
Sun May 5 11:40:48 PDT 2024


meshtag wrote:

> return a mixed static dynamic list like you are doing here. Isnt this what you are looking to do here as well?

We have separate control paths for static and dynamic values (because we need different maps and different utility functions to calculate suffix product and linearize variables). All of this related infra expects things to be either in `llvm::ArrayRef<int64_t>` or `llvm::ArrayRef<Value>`, so we require extra separation/casting (from `SmallVector<OpFoldResult>`, which is what the utility returns) if we use the above mentioned utility (another potentially overkill option might be to make analysis utilities deal with mixed values themselves - not all of whom are implemented in this PR). Additionally, the function also expects the input shape as `ArrayRef<OpFoldResult>`, which means an extra step/loop for dimension extraction and casting. 

I feel this does not compose well and would convolute the logic even more without reducing the code (since we anyway require separate paths for static and dynamic values). I feel it might've made sense to do that if we had a single control flow path. Unless it's a blocker to land this patch, I am hesitant on going through that path. 

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


More information about the Mlir-commits mailing list