[Mlir-commits] [mlir] [mlir][linalg] Extend elementwise (PR #124661)

[Renato Golin]

rengolin wrote:

> How is this new `linalg.elementwise`/`linalg.elemwise` op supposed to relate to the existing `linalg.elemwise_unary` and `linalg.elemwise_binary` ops?
> Somehow I completely missed that these ops already exist. TBF, the RFC and PRs seem to have left this topic out as well.

We did discuss this in person and some threads in the forum, and there are two schools:

1. We `move` the op from OpDSL to ODS at the same time we add affine map semantics. Syntax and semantics needs to be identical, except for the new affine map. Changing semantics occur later. It's also slower to converge semantics, if not close enough to the goals. This is what @shahidact is doing with the `matmul` family, and I prefer it that way, but this is a personal preference.
2. We `create` a new op in ODS with affine maps, but there are no requirements on keeping the same syntax and semantics. This is easier to iterate upstream because whoever is using the old ones won't be affected until the move to the new ones. It's also confusing because now you have two similarly named and behaving operations upstream. This was championed by @MaheshRavishankar in both element wise and matmul threads.

I will not claim my preference is better, that's why I'm ok with this going as is. But I also didn't click the green button, because I want to make sure there is consensus and other people also prefer this route.

> * What's the benefit of lumping these ops under just one op? That is, is there a benefit to lumping arities?

On a single op, the enum business is complicated. Bit-field-like encoding helps clean up a bit, but it's still ugly. I'd rather see three ops (unary, binary, ternary), but again, that's my personal preference. If there's consensus on a single one, I'm also happy to acquiesce.


> * Is the intention to deprecate  `linalg.elemwise_unary` and `linalg.elemwise_binary`?

Definitely yes! There can be only one. :crossed_swords: 

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