[Mlir-commits] [mlir] [mlir] Declare promised interfaces for all dialects (PR #78368)

[Oleksandr Alex Zinenko]

ftynse wrote:

>> Assuming it's not the dialect itself that declares the promise
>
>that doesn’t lineup with your previous message I was quoting:
>
>> I'm not sure having a promised interface for the builtin dialect even makes sense.
>
> That is: it was about the built in dialect declaring the promise, but now you start with assuming the opposite: what do I miss?

I didn't expect the builtin dialect to declare promises in either of my replies. Let me try explaining this differently. The builtin dialect is always loaded in the context, so its existence is basically invisible at project setup. One doesn't has to register or load it explicitly. Now if it had promises, project setup would have to provide implementations for those promises. This makes it partially visible to the setup, and a bit less builtin if you wish. What's worse, the dialect still doesn't need to be registered, so there is an asymmetry with regular dialects that I find confusing. So I don't think the builtin dialect should be declaring promises, at least in the current setup. (Also, wouldn't that create a dependency from the builtin dialect on the interface?)

> Just look at the mlir/IR includes: I am pretty sure they include other directories (and everything depends on IR…)

There are includes from `mlir/Support` (totally legit, library dependency), `mlir/Bytecode` (bytecode is part of the IR if you ask me) and a handful from `mlir/Interfaces`.  There is no include from `mlir/Transform`, for example, which would look weird.

Here, we would be getting (transitive) includes from `mlir/Dialect/Tensor/Transforms` in `mlir/Dialect/Tensor/IR` (via `mlir/Dialect/Tensor/TransformOps`). I think it's wrong to have IR depend on Transform, be it within a dialect scope or top-level scope. Hence my proposition to revert and have `mlir/Dialect/Tensor/IR` depend on `mlir/Dialect/Transform/IR`, which itself doesn't depend on dialect-specific transforms. 

TBH, I'd put transform interfaces to `lib/Interfaces` at this point, but this is beyond the scope of this discussion.



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