[Mlir-commits] [mlir] [mlir-python] Fix duplicate EnumAttr builder registration across dialects. (PR #187191)

[Maksim Levental]

================
@@ -94,7 +99,11 @@ static bool emitAttributeBuilder(const EnumInfo &enumInfo, raw_ostream &os) {
     return false;
 
   int64_t bitwidth = enumInfo.getBitwidth();
-  os << formatv("@register_attribute_builder(\"{0}\")\n",
+  // These builders may be emitted by multiple dialect enum_gen files when
+  // dialects share enum definitions via .td includes. Use allow_existing=True
+  // so that the first loaded dialect registers the builder and subsequent
+  // loads silently skip (first-registration wins).
----------------
makslevental wrote:

https://mlir.llvm.org/docs/DefiningDialects/Operations/#wrapping-enums-in-attributes

reading this i guess the recommendation isn't superstrong and probably actually unrelated to the question of ambiguity:

> The most common of these is to use the EnumAttr class, which takes an EnumInfo (either a IntEnum or BitEnum) as a parameter and constructs an attribute that holds one argument - value of the enum. This attribute is defined within a dialect and can have its assembly format customized to, for example, print angle brackets around the enum value or assign a mnemonic.

reads like "do wrap your `EnumInfo` in `EnumAttr` and use the `EnumAttr`"

> An older form involves using the *IntEnumAttr and *BitEnumATtr classes and their corresponding *EnumAttrCase classes (which can be used anywhere a *EnumCase is needed). These classes store their values as a SignlessIntegerAttr of their bitwidth, imposing the constraint on it that it has a value within the valid range of the enum. If their genSpecializedAttr parameter is set, they will also generate a wrapper attribute instead of using a bare signless integer attribute for storage.

reads like this is the old way of doing it with drawbacks (but unrelated to dialect ownership).

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