[Mlir-commits] [mlir] [MLIR][Tensor] Canonicalize fully covering slice insertions into tensors with unit prefixes (PR #92912)

[Stella Laurenzo]

stellaraccident wrote:

I am pretty sure we don't have a "first to invent" policy on patches implementing a disputed design. And some things in these algebras are quite sensitive with non obvious tradeoffs.

I tire of these education sessions in rhetoric. Bottom line, if I look at the commit history and project affiliations of the author and the person rebutting, I'm willing to believe they both have an intuition and feel for the algebra here. And it is completely reasonable to call for a pause to assess: this is a volunteer project and detailed arguments for something with as much nuance as this dialect are definitely work and may not just be able to be synthesized out of thin air (or in a fly by patch review) or in a patch thread like this.

But we're not talking about that. 

So -1 from me: this patch shouldn't land without consensus. It may have merit, but it is a very specific thing, not at all in the category of "fold right" as for constants. It is obvious that there are two people with presumed expertise and contribution history to this dialect who disagree. Happens all the time. The traditional thing we do in this community is to pause and pop that up to the list. That is not blocking. That is just how we work. 

I've read this patch and the thread and I still don't know why the author wanted it -- I presume they have good reasons, or why Mahesh doesn't (who I expect also has a lot of experience on this point exactly). Instead I've got an escalation based on the form of the argument, and I don't find that salient for understanding or contributing.

I'd recommend raising in the list and discussing experiences with these forms and seeing if we're even close or far on consensus.

For something like this, principles are something that we often arrive at through interrogation and experience. Just calling for them in some kind of summary fashion is not helpful.

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