[Mlir-commits] [llvm] [mlir] [MLIR] Generalize expand_shape to take shape as explicit input (PR #90040)

[Oleksandr Alex Zinenko]

ftynse wrote:

> @ftynse unless this arith -> utils dependency breaks builds (I dont think it does, cause there is no cyclic dependency afaics), i think it might be better to land this and then remove the dependency as a quick follow up. 

I don't see a cyclic dependency immediately. This is a bit of a bikeshedding discussion, one can simply move a file under a new `ShapedTypeArithUtils` and resolve the surface concern: `DialectUtils` suggests that it's something cheap and useful for all dialects, but it comes with a dependency on Arith (and UB, etc.). There's a deeper concern: we don't have a dialect-agnostic way to create constants, so anything that creates constants will have to depend on Arith or similar. I certainly won't block this on resolving that concern.

> This is fairly big change, and more things might get added that will have to be resolved. I am inclined to take the snap shot as is, but fix the dependency issue quickly after.

If the issue can be fixed quickly, why not quickly add the solution to the PR? :)

My worry here is by allowing this dependency temporarily, more uses will creep in, making it harder to remove as the time goes. I have made a similar comment this week on another PR...

How about we ask a third opinion, @joker-eph ?

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