Skip to content

[MLIR][LLVM] Cleanup attr-dict printing (NFC) #115765

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

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

gysit
Copy link
Contributor

@gysit gysit commented Nov 11, 2024

This commit simplifies the custom attribute dictionary printing and uses it only for printing ops that have fast math flags.

@gysit gysit force-pushed the cleanup-attr-dict-printing branch from 5b5dfad to 1dbaeea Compare November 11, 2024 20:44
@gysit gysit marked this pull request as ready for review November 12, 2024 06:47
@gysit gysit requested a review from Dinistro November 12, 2024 06:47
@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2024

@llvm/pr-subscribers-mlir-llvm

Author: Tobias Gysi (gysit)

Changes

This commit simplifies the custom attribute dictionary printing and uses it only for printing ops that have fast math flags.


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

2 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td (+2-4)
  • (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+1-9)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 81e9f69f15acf6..43342beae256bc 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -69,8 +69,7 @@ class LLVM_IntArithmeticOpWithOverflowFlag<string mnemonic, string instName,
     $res = op;
   }];
   let assemblyFormat = [{
-    $lhs `,` $rhs `` custom<OverflowFlags>($overflowFlags)
-    `` custom<LLVMOpAttrs>(attr-dict) `:` type($res)
+    $lhs `,` $rhs `` custom<OverflowFlags>($overflowFlags) attr-dict `:` type($res)
   }];
   string llvmBuilder =
     "$res = builder.Create" # instName #
@@ -88,7 +87,7 @@ class LLVM_IntArithmeticOpWithExactFlag<string mnemonic, string instName,
     $res = op;
   }];
   let assemblyFormat = [{
-    (`exact` $isExact^)? $lhs `,` $rhs custom<LLVMOpAttrs>(attr-dict) `:` type($res)
+    (`exact` $isExact^)? $lhs `,` $rhs attr-dict `:` type($res)
   }];
   string llvmBuilder =
     "$res = builder.Create" # instName #
@@ -524,7 +523,6 @@ class LLVM_CastOpWithNNegFlag<string mnemonic, string instName, Type type,
     $res = op;
   }];
 }
-
 class LLVM_CastOpWithOverflowFlag<string mnemonic, string instName, Type type,
                   Type resultType, list<Trait> traits = []> :
     LLVM_Op<mnemonic, !listconcat([Pure], [DeclareOpInterfaceMethods<IntegerOverflowFlagsInterface>], traits)>,
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 6b2d8943bf4885..d4f8c4c1faf956 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -140,15 +140,7 @@ static ParseResult parseLLVMOpAttrs(OpAsmParser &parser,
 static void printLLVMOpAttrs(OpAsmPrinter &printer, Operation *op,
                              DictionaryAttr attrs) {
   auto filteredAttrs = processFMFAttr(attrs.getValue());
-  if (auto iface = dyn_cast<IntegerOverflowFlagsInterface>(op)) {
-    printer.printOptionalAttrDict(
-        filteredAttrs, /*elidedAttrs=*/{iface.getOverflowFlagsAttrName()});
-  } else if (auto iface = dyn_cast<ExactFlagInterface>(op)) {
-    printer.printOptionalAttrDict(filteredAttrs,
-                                  /*elidedAttrs=*/{iface.getIsExactName()});
-  } else {
-    printer.printOptionalAttrDict(filteredAttrs);
-  }
+  printer.printOptionalAttrDict(filteredAttrs);
 }
 
 /// Verifies `symbol`'s use in `op` to ensure the symbol is a valid and

@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2024

@llvm/pr-subscribers-mlir

Author: Tobias Gysi (gysit)

Changes

This commit simplifies the custom attribute dictionary printing and uses it only for printing ops that have fast math flags.


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

2 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td (+2-4)
  • (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+1-9)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 81e9f69f15acf6..43342beae256bc 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -69,8 +69,7 @@ class LLVM_IntArithmeticOpWithOverflowFlag<string mnemonic, string instName,
     $res = op;
   }];
   let assemblyFormat = [{
-    $lhs `,` $rhs `` custom<OverflowFlags>($overflowFlags)
-    `` custom<LLVMOpAttrs>(attr-dict) `:` type($res)
+    $lhs `,` $rhs `` custom<OverflowFlags>($overflowFlags) attr-dict `:` type($res)
   }];
   string llvmBuilder =
     "$res = builder.Create" # instName #
@@ -88,7 +87,7 @@ class LLVM_IntArithmeticOpWithExactFlag<string mnemonic, string instName,
     $res = op;
   }];
   let assemblyFormat = [{
-    (`exact` $isExact^)? $lhs `,` $rhs custom<LLVMOpAttrs>(attr-dict) `:` type($res)
+    (`exact` $isExact^)? $lhs `,` $rhs attr-dict `:` type($res)
   }];
   string llvmBuilder =
     "$res = builder.Create" # instName #
@@ -524,7 +523,6 @@ class LLVM_CastOpWithNNegFlag<string mnemonic, string instName, Type type,
     $res = op;
   }];
 }
-
 class LLVM_CastOpWithOverflowFlag<string mnemonic, string instName, Type type,
                   Type resultType, list<Trait> traits = []> :
     LLVM_Op<mnemonic, !listconcat([Pure], [DeclareOpInterfaceMethods<IntegerOverflowFlagsInterface>], traits)>,
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 6b2d8943bf4885..d4f8c4c1faf956 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -140,15 +140,7 @@ static ParseResult parseLLVMOpAttrs(OpAsmParser &parser,
 static void printLLVMOpAttrs(OpAsmPrinter &printer, Operation *op,
                              DictionaryAttr attrs) {
   auto filteredAttrs = processFMFAttr(attrs.getValue());
-  if (auto iface = dyn_cast<IntegerOverflowFlagsInterface>(op)) {
-    printer.printOptionalAttrDict(
-        filteredAttrs, /*elidedAttrs=*/{iface.getOverflowFlagsAttrName()});
-  } else if (auto iface = dyn_cast<ExactFlagInterface>(op)) {
-    printer.printOptionalAttrDict(filteredAttrs,
-                                  /*elidedAttrs=*/{iface.getIsExactName()});
-  } else {
-    printer.printOptionalAttrDict(filteredAttrs);
-  }
+  printer.printOptionalAttrDict(filteredAttrs);
 }
 
 /// Verifies `symbol`'s use in `op` to ensure the symbol is a valid and

Copy link
Contributor

@Dinistro Dinistro left a comment

Choose a reason for hiding this comment

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

Should this be marked as NFC?
Otherwise, LGTM!

@@ -524,7 +523,6 @@ class LLVM_CastOpWithNNegFlag<string mnemonic, string instName, Type type,
$res = op;
}];
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Seems unrelated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is a small format cleanup...

This commit simplifies the custom attribute dictionary printing
and uses it only for printing ops that have fast math flags.
@gysit gysit force-pushed the cleanup-attr-dict-printing branch from 1dbaeea to 54750b1 Compare November 12, 2024 07:10
@gysit gysit changed the title [MLIR][LLVM] Cleanup custom attr-dict printing [MLIR][LLVM] Cleanup attr-dict printing (NFC) Nov 12, 2024
@gysit gysit merged commit fa98887 into llvm:main Nov 12, 2024
8 checks passed
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
This commit simplifies the custom attribute dictionary printing and uses
it only for printing ops that have fast math flags.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants