[Mlir-commits] [mlir] [mlir] Decouple DenseResourceElementsAttr from BuiltinDialect (PR #191486)

[Pavel Prokofyev]

integralpro wrote:

> > This prevented custom dialects from owning resource blobs referenced by this attribute.
> ```c++
> using MyDialectResourceBlobHandle = DialectResourceBlobHandle<MyDialect>;
> ```

Thank you for your comment!

I think these are two different concerns.

Yes, it is relatively easy to define a new dialect-specific attribute that uses a dialect-specific `DialectResourceBlobHandle`. But that does not solve the problem here, which is interoperability with the existing `DenseResourceElementsAttr`.

Today `DenseResourceElementsAttr` is the generic attribute for “elements backed by an external resource”, and in assembly it is spelled as `dense_resource<...>`. Nothing in that surface syntax suggests builtin-dialect ownership, but the current implementation effectively hardcodes that by using `DialectResourceBlobHandle<BuiltinDialect>` internally.

The issue is not whether a custom dialect can define its own resource-backed attribute. It can. The issue is that code which already understands `DenseResourceElementsAttr` does not automatically understand a new dialect-specific replacement. Introducing a parallel attribute pushes the burden onto every consumer that wants to handle resource-backed elements generically.

On the “two ways of referring to a dialect’s blob manager” point: I do not think this PR introduces that situation. That possibility already exists today, because multiple attributes can refer to the same blob manager. For example, a dialect could already define both:
```
test.dense_resource<"custom_blob">
test.dense_resource2<"custom_blob">
```

So the duplication concern is not created by making `DenseResourceElementsAttr` dialect-generic. What this change does is separate the interface-level concept (“resource-backed dense elements”) from the particular storage owner (BuiltinDialect today).

In other words, it decouples `DenseResourceElementsAttr` and `dense_resource<...>` from builtin-owned blob storage rather than requiring each dialect to reinvent an equivalent attribute just to change resource ownership.

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