[Mlir-commits] [mlir] [mlir] NamedAttribute utility generator (PR #75118)

[llvmlistbot at llvm.org]

sjw36 wrote:

> > No, that is true but most usages are StringLiteral except when copying from a NamedAttribute. It is complicated since it is context dependent. Do you have any suggestion?
> 
> What I have been doing in the past is defining these as StringAttr on the dialect instance instead.

Can you point me to an example. I see getConsumedAttrName() in the Transform dialect, but it is just using a StringLiteral under the hood and the usage seems cumbersome having to get loaded dialect first. Is there a better example that is storing these on the dialect?

> > Perhaps that is a bridge too far. We have scenarios where an inherent attribute on an op is copied to the enclosing func or module and should then be dialect-qualified. I could live with a DialectNamedAttrDef and an NamedAttrDef (unqualified).
> 
> The whole concept of using an "AttrDef" here is still unclear to me right now to begin with. I'm also not sure I'm making a direct connection between your usage of inherent attribute and a propagation as a discardable one.

If we set aside the inherent/property (see below) case for now and just consider a DialectNamedAttr then:
* the ODS class can be used throughout the code instead of "arbitrary_name" strings
* and the ODS class can be used to Get/Set the Typed Value on arbitrary Operations

In this way it formalizes a specific `NamedAttribute` in a Dialect with a specific Typed Value. See ROCDL example usage.

> In general with properties we should be relying more and more on Interfaces to access informations about an operation instead of relying on "name lookup": in this model the name of the inherent attribute shouldn't matter, and shouldn't even be exposed (it is an implementation detail of the storage).

This makes sense and certainly more optimal. I had a few use cases for promoting inherent attributes to parent ops but we are migrating them to properties in any case.


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