[Mlir-commits] [mlir] [NFC] Make AggregateOpInterface part of mlir:: instead of linalg:: (PR #70089)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Fri Oct 27 16:46:47 PDT 2023


MaheshRavishankar wrote:

> > > I don't think this is ready to proceed.
> > 
> > 
> > could you give some feedback, then how to proceed then. This interface is not being added. It is in the Linalg dialect (where it really does not belong). So this PR is only moving it.
> 
> I thought you concluded in the thread above where you wrote:
> 
> > I am happy to go with one of two options
> > 
> > 1. We keep the interface in Linalg...
> > 2. We move the interface out of core into IREE... Since there we have more control of which ops implement the interface, we can prevent unintended uses of it.
> 
> I'm confused by your comment there?
> 
> Right now the "service" this interface provide is contextual: that is for a given op, the implementation you want from the will differ based on the pass, and this as-is can't be a generic interface, but looks like a pass-specific interface.

so... leave it where it is, and drop this PR? IMO this is in the wrong place under Linalg... Id rather move it out of there, and be very prescriptive of which ops in tree to add this interface to (add a big red warning flag saying this is to be used with caution). But I am not tied to this. I am happy to leave it in Linalg. I dont want to move it to IREE because this has upstream dependencies, i.e. Softmax op in Linalg dialect (which also doesnt belong in Linalg, but thats a different story).

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


More information about the Mlir-commits mailing list