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

[Clément Fournier]

oowekyala 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.

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.



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