[Mlir-commits] [mlir] [mlir] Fix semantics of linalg::ReduceOp with several inputs (PR #107005)

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> > The PR description is misleading. It says "fixing a crash", but it is actually changing the semantics of the operation.
> 
> You're right, although there was a logic error in the verifier code that caused a crash, so it also does fix a crash. Since the semantics of this op in case it has several input operands are not documented or tested, I interpreted what the semantics should be. In my view, the number of inputs and number of outputs should not be related. No other linalg op has this restriction afaict.

That is not entirely true. There is a very specific relation ship between all the inputs and outputs of a `linalg` op. They all have indexing maps with same domains and there is a relationship between how different dimensions are constrained based on the range of the indexing maps used. Also having a dimension that is marked as reduction showing up in the result indexing map is not allowed. We can have multiple inputs and multiple outputs for a reduction. Thats totally OK, but you are saying its one op running independent reductions to generate different outputs, but with the same iteration space and dependence? That works.

> 
> It seems like the only consistent alternative semantics is to say that the op must have exactly one input and output, which is restrictive for no good reason.
> 
> > Is there anything more to reduction than just saying that the iterator types are "reduction". I dont see it providing any more value other than that.
> 
> Sure, but this can be said of every structured linalg op. `linalg.map` doesn't require the same number of inputs and outputs, which allows some useful things to be written (as in [its documentation](https://mlir.llvm.org/docs/Dialects/Linalg/#linalgmap-linalgmapop), whose example is similar to the example in [this comment](https://github.com/llvm/llvm-project/pull/107005#issuecomment-2361800230)). Making `reduce` have the same freedom makes the linalg dialect more consistent.

I am fine with that but just need to document the semantics that you expect.

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