[Mlir-commits] [mlir] [mlir][tensor][linalg] Move Pack/Unpack Ops to Linalg (PR #123902)

[Renato Golin]

rengolin wrote:

> From my experience, some reviewers prefer **big changes**, while others favor many **smaller patches**. Personally, I find multiple smaller patches more manageable. 

I _"prefer"_ less churn, and sometimes that's lots of small patches, sometimes that one giant patch.

> However, in this case, I was concerned about creating a state where we have:
> 
> * `tensor.pack` + `tensor.unpack`, and
> * `linalg.pack` + `linalg.unpack`

Yes, I'm worried about that, too. Do that in between releases and you have a broken compiler. In any case, perhaps we should wait for the new release to branch before merging?

> Also, given our current struggles with reviews, I thought that perhaps "less would be more". I'm happy either way - whatever makes things easier for reviewers.

So, I looked at the code and it looks very much like my work-in-progress branch doing the same thing. I don't think there's a better way of doing these moves, other than creating new operations entirely, which leads to duplication.

The main problem here is that pack/unpack has received a lot of attention in the past few months (thanks, btw!), and the infra around it got quite big.

For us, a single commit would be easier to rebase on top of. I imagine this is true for others, too. So to me, this would be less churn. But we can accommodate multiple patches if others prefer it that way.

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