[Mlir-commits] [mlir] [mlir] Declare promised interfaces for all dialects (PR #78368)
Oleksandr Alex Zinenko
llvmlistbot at llvm.org
Thu Feb 22 10:59:48 PST 2024
ftynse wrote:
> I don't follow how you reach this conclusion?
> The point of promises is that you need to provide an implementations if and only if you will actually need them. This should be fully decoupled, otherwise I don't quite get the point of the promise if the coupling still exists...
Certainly, for the case where implementations aren't needed, there is no need to register implementations. For the case where they are needed, there is the asymmetry that I'm pointing out above. The setup of that case will more or less be "load DialectA, register interface for DialectA, load Dialect B, register interface for DialectB, register interface for BuiltinDialect" (why? are we missing the load part for it? if it is builtin, why extra action is necessary?)
I suppose I could argue for the builtin dialect to be less builtin and require explicit loading, that would make the model clearer for me.
Another possibility is for the interface itself to contain the implementation for the builtin dialect. Specific example of that is data layout. Currently, the data layout analysis knows about builtin types/operations so they don't implement the interface. That being said, I'd rather remove that special case so let's take that as a case study for promised interfaces.
(note to other casual readers, this discussion is not blocking the PR from being merged)
> I agree, but this isn't a dependency when this is about interfaces, this is basically "we need to know this class exists and get its name" IIUC?
I agree that this is not a dependency in library sense, but only an include chain. The class that we need is defined in `Transform/IR/TransformInterfaces.h`, we should include that directly instead of transitively through `Tensor/TransformOps/...h`
https://github.com/llvm/llvm-project/pull/78368
More information about the Mlir-commits
mailing list