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

[Clément Fournier]

oowekyala wrote:

A couple more thoughts, to make sure this change is not misunderstood. 

I have not actually been proposing to extend the semantics of linalg.reduce. It is already possible to write the MAC code that I presented above like so:
```mlir
  linalg.reduce
      ins(%input, %input2:memref<16x32x64xi32>, memref<16x32x64xi32>)
      outs(%init, %init2: memref<16x64xi32>, memref<16x64xi32>)
      dimensions = [1]
      (%in: i32, %in2: i32, %out: i32, %out2: i32) {
        %0 = arith.muli %in, %in2: i32
        %1 = arith.addi %out, %0: i32
        linalg.yield %1, %1: i32, i32
     }
```
But in current linalg, if you have 2 inputs then you must have 2 outputs, so here we have to return a second result that we don't care about. To me this restriction is most likely an accident. It makes no sense to require that, unless maybe you restrict each input to flow _only_ to its corresponding output, but that would be better expressed by several unary reduce. It is also not documented why this is the case or what the intended semantics should be. My change is I think lifting an artificial restriction, but it is not making `reduce` more powerful. That fixes the bugs in the verifier so I think that's the semantics that the original authors intended. All in all as far as semantic changes go, this one is IMO very minor.

Of course if to fit within @rengolin's new linalg design, `reduce` must be unary, then the current semantics are indeed too general... So there is a choice to be made.

Anyway I will close this and reopen a PR with just the verifier fix. 

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