[PATCH] D133625: [mlir][MemRef] Simplify extract_strided_metadata(expand_shape)

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache added a comment.

Surfacing a general comment that appeared ofline:

I think we can always consider the general case with all `?` and let the folders do their job on `OpFoldResult`:

  sizes<?>, strides<?> expand to sizes<?x..x?>, strides<?x..x?> by expansionFactors(?x...x?)`` 
  
  If we reduce to this, this is all about determining ```SmallVector<OpFoldResult> expansionFactors(...)`. 
  In the most general case (NYI), these will be SSA values in the op (see e.g. https://github.com/llvm/llvm-project/issues/51564).
  
  But even in the absence of the general case, I believe we greatly simplify the problem by thinking in those terms.
  
  I'd add a helper `SmallVector<OpFoldResult> ExpandShapeOp:::buildExpansionFactors(b, loc, groupId)` to the op itself that constructs `expansionFactors` from the `source` memref  and the result type (actually may need to be in some utils because of Affine cyclic dependencies .. sigh).
  
  We'd likely want a counterpart for `CollapseShape` when we get there.
  
  I think these suggestions will make the impl both future proof and readable.
  
  Still I have not tried so I may be missing something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133625