[Mlir-commits] [mlir] Remove Pure attribute from Linalg::IndexOp. (PR #68894)

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> 
> > > > we make nested linalg ops illegal then the above issue is potentially mute cause you would never have case where it would be illegal to cse linalg.index ops. This issue came about because someone tried to use nested linalg ops
> 
> > > Correction: I used inlining and a function call that uses linalg operations was inlined inside a linalg op.
> 
> > Having an operation of that granularity is also not the preferred path.
> 
> @MaheshRavishankar, when you are saying "having an operation of that granularity" you mean "having a call operation"? You are saying that having a call inside linalg is also not preferred?

Basically I am saying having tensor operations within the body of linalg operations does not seem necessary. It is not to say it is invalid. IIUC having such an operation in the body of a linalg op means that the computation is represented as tensor of tensors. In other words this is an alternative representation of tiled computation, instead of using loops and tiled linalg operations in the body. The latter is what is generated with linalg tiling transformations. The tensor of tensors representation might have some compositionality issues as well. For example if you have a `linalg.matmul` with tensor of tensors types, the inner tensor type dimensions also have an implicit relationship which can be an issue. In any case, I am not saying it is wrong but such "nested" types seem unnecessary. So I would expect some sharp edges along those paths.


> 
> > nor does lower to loops result in conversion of linalg operations to scalar code
> 
> Can you confirm that what you are also saying here is that lowering a nested linalg operations via `--convert-linalg-to-loops` does not work? Or what do you concretely mean "result in conversion to scalar code"? Can you provide a concrete example?
> 
> I ask because we have been using `--convert-linalg-to-loops` for a long time with no problems (except disabling CSE) to avoid this error.

Well, that's great to know! Might have been fixed since the last time I saw this cause I wouldn't have expected it to work based on how the lowering to loops used to work.

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