[Mlir-commits] [mlir] [mlir][IR] Add `getPropertyAsAttr` and `setPropertyFromAttr` methods. (PR #150060)

[Fabian Mora]

fabianmcg wrote:

Preface, we all have mental models of what props should be and I think we loosely agree on that model.

I think where we are diverging is in the mental model of what properties currently are with their def, and I'm being extremely pedantic on this point. That's why I keep insisting, that one option is changing and documenting the model -even if it's not implemented it should be documented.

> > IMO, the semantics and meaning of an attr are owned by the attr, while for a prop they are owned (let's say jointly) by the op.
> 
> You're not following up on everything I asked: I mentioned the example of the IntegerAttr to rebute this. The meaning of an IntegerAttr does not seems different from the meaning of an `IntegerProp`! And I argue the same for EnumAttr vs EnumProp.

I didn't mention IntegerProp vs IntgerAttr because I was making my case for *Prop and *Attr. To me they are currently different in all cases.

> An attribute is just like a property: it is only storing some data. All of these get semantics only in conjunction with an operation: the semantics is part of the op definition.
> 
> The distinction you're making is artificial and does not exist in my mental model.

IMO it's not artificial, it's their current state.

> > An attribute is a standalone unit, that can be parsed, printed
> Sure, we've already covered this: it is **only** a matter of mnemnonic and registration though.

Adding a mnemonic and registration is a big departure from the current model. It would be turning them into "mutable non unique attributes". If that's the path we choose, that's fine, but it must be an explicit decision.

> > stored generically
> 
> generically? I don't think so: every attribute comes with a custom storage mechanism, no different than properties. 

To me this is one the critical distinctions between them, props are just the C++ storage mechanism, similar to the storage class of an attr. Props are missing the '`Attribute`' side of things, allowing things like `dyn_cast<MyProp>(opaqueProp)`...


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