Skip to content

[mlir] Allow specifying name for EnumAttr #119788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

smeenai
Copy link
Collaborator

@smeenai smeenai commented Dec 12, 2024

The name of an EnumAttr is currently always based on the name of the
underlying enum, which is a good default, but users may wish to
customize it. Add a template argument to enable this, while preserving
the old default. This is changing the position of the existing traits
argument, but that has no in-tree uses (so nothing needs to be updated),
and any out-of-tree uses should see a type mismatch error instead of
silently getting an incorrect argument value.

The name of an EnumAttr is currently always based on the name of the
underlying enum, which is a good default, but users may wish to
customize it. Add a template argument to enable this, while preserving
the old default. This is changing the position of the existing `traits`
argument, but that has no in-tree uses (so nothing needs to be updated),
and any out-of-tree uses should see a type mismatch error instead of
silently getting an incorrect argument value.
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir mlir:ods labels Dec 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 12, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-ods

Author: Shoaib Meenai (smeenai)

Changes

The name of an EnumAttr is currently always based on the name of the
underlying enum, which is a good default, but users may wish to
customize it. Add a template argument to enable this, while preserving
the old default. This is changing the position of the existing traits
argument, but that has no in-tree uses (so nothing needs to be updated),
and any out-of-tree uses should see a type mismatch error instead of
silently getting an incorrect argument value.


Full diff: https://github.com/llvm/llvm-project/pull/119788.diff

1 Files Affected:

  • (modified) mlir/include/mlir/IR/EnumAttr.td (+4-4)
diff --git a/mlir/include/mlir/IR/EnumAttr.td b/mlir/include/mlir/IR/EnumAttr.td
index 9fec28f03ec282..f7b5967437fb90 100644
--- a/mlir/include/mlir/IR/EnumAttr.td
+++ b/mlir/include/mlir/IR/EnumAttr.td
@@ -384,9 +384,9 @@ class EnumParameter<EnumAttrInfo enumInfo>
 // The op will appear in the IR as `my_dialect.my_op first`. However, the
 // generic format of the attribute will be `#my_dialect<"enum first">`. Override
 // the attribute's assembly format as required.
-class EnumAttr<Dialect dialect, EnumAttrInfo enumInfo, string name = "",
-               list <Trait> traits = []>
-    : AttrDef<dialect, enumInfo.className, traits> {
+class EnumAttr<Dialect dialect, EnumAttrInfo enumInfo, string mnemonicArg = "",
+               string name = "", list <Trait> traits = []>
+    : AttrDef<dialect, !if(!empty(name), enumInfo.className, name), traits> {
   let summary = enumInfo.summary;
   let description = enumInfo.description;
 
@@ -410,7 +410,7 @@ class EnumAttr<Dialect dialect, EnumAttrInfo enumInfo, string name = "",
   let parameters = (ins EnumParameter<enumInfo>:$value);
 
   // If a mnemonic was provided, use it to generate a custom assembly format.
-  let mnemonic = name;
+  let mnemonic = mnemonicArg;
 
   // The default assembly format for enum attributes. Selected to best work with
   // operation assembly formats.

@llvmbot
Copy link
Member

llvmbot commented Dec 12, 2024

@llvm/pr-subscribers-mlir-core

Author: Shoaib Meenai (smeenai)

Changes

The name of an EnumAttr is currently always based on the name of the
underlying enum, which is a good default, but users may wish to
customize it. Add a template argument to enable this, while preserving
the old default. This is changing the position of the existing traits
argument, but that has no in-tree uses (so nothing needs to be updated),
and any out-of-tree uses should see a type mismatch error instead of
silently getting an incorrect argument value.


Full diff: https://github.com/llvm/llvm-project/pull/119788.diff

1 Files Affected:

  • (modified) mlir/include/mlir/IR/EnumAttr.td (+4-4)
diff --git a/mlir/include/mlir/IR/EnumAttr.td b/mlir/include/mlir/IR/EnumAttr.td
index 9fec28f03ec282..f7b5967437fb90 100644
--- a/mlir/include/mlir/IR/EnumAttr.td
+++ b/mlir/include/mlir/IR/EnumAttr.td
@@ -384,9 +384,9 @@ class EnumParameter<EnumAttrInfo enumInfo>
 // The op will appear in the IR as `my_dialect.my_op first`. However, the
 // generic format of the attribute will be `#my_dialect<"enum first">`. Override
 // the attribute's assembly format as required.
-class EnumAttr<Dialect dialect, EnumAttrInfo enumInfo, string name = "",
-               list <Trait> traits = []>
-    : AttrDef<dialect, enumInfo.className, traits> {
+class EnumAttr<Dialect dialect, EnumAttrInfo enumInfo, string mnemonicArg = "",
+               string name = "", list <Trait> traits = []>
+    : AttrDef<dialect, !if(!empty(name), enumInfo.className, name), traits> {
   let summary = enumInfo.summary;
   let description = enumInfo.description;
 
@@ -410,7 +410,7 @@ class EnumAttr<Dialect dialect, EnumAttrInfo enumInfo, string name = "",
   let parameters = (ins EnumParameter<enumInfo>:$value);
 
   // If a mnemonic was provided, use it to generate a custom assembly format.
-  let mnemonic = name;
+  let mnemonic = mnemonicArg;
 
   // The default assembly format for enum attributes. Selected to best work with
   // operation assembly formats.

@smeenai smeenai requested a review from Mogball December 12, 2024 23:04
Copy link
Contributor

@Mogball Mogball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is useful, but can you add an in-tree use by adding an example usage of this in the TestDialect?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir:ods mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants