Skip to content

[NFC][mlir][AsmPrinter] Don't compute resourceStr when --mlir-elide-resource-strings-if-larger=0 #138275

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
May 2, 2025

Conversation

python3kgae
Copy link
Contributor

When skipping the printing of large DenseResourceElementsAttr with --mlir-elide-resource-strings-if-larger,
we need to compute resourceStr to check if the string is small enough to print.

With --mlir-elide-resource-strings-if-larger set to 0, nothing should be printed, but we still compute the size.

This change will allow an early return when --mlir-elide-resource-strings-if-larger=0, making the print process faster.

…esource-strings-if-larger=0

When skipping the printing of large DenseResourceElementsAttr with --mlir-elide-resource-strings-if-larger, we need to compute resourceStr to check if the string is small enough to print. With --mlir-elide-resource-strings-if-larger set to 0, nothing should be printed, but we still compute the size.
This change will allow an early return when --mlir-elide-resource-strings-if-larger=0, making the print process faster.
@python3kgae python3kgae requested review from Mogball and River707 May 2, 2025 13:34
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels May 2, 2025
@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

@llvm/pr-subscribers-mlir-core

Author: Xiang Li (python3kgae)

Changes

When skipping the printing of large DenseResourceElementsAttr with --mlir-elide-resource-strings-if-larger,
we need to compute resourceStr to check if the string is small enough to print.

With --mlir-elide-resource-strings-if-larger set to 0, nothing should be printed, but we still compute the size.

This change will allow an early return when --mlir-elide-resource-strings-if-larger=0, making the print process faster.


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

2 Files Affected:

  • (modified) mlir/lib/IR/AsmPrinter.cpp (+4)
  • (modified) mlir/test/IR/pretty-resources-print.mlir (+10)
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index 5b5ec841917e7..c7194aca7ff12 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -3463,6 +3463,10 @@ void OperationPrinter::printResourceFileMetadata(
       std::optional<uint64_t> charLimit =
           printerFlags.getLargeResourceStringLimit();
       if (charLimit.has_value()) {
+        // Don't compute resourceStr when charLimit is 0.
+        if (charLimit.value() == 0)
+          return;
+
         llvm::raw_string_ostream ss(resourceStr);
         valueFn(ss);
 
diff --git a/mlir/test/IR/pretty-resources-print.mlir b/mlir/test/IR/pretty-resources-print.mlir
index 297c83bbb1389..980af80343d47 100644
--- a/mlir/test/IR/pretty-resources-print.mlir
+++ b/mlir/test/IR/pretty-resources-print.mlir
@@ -2,11 +2,16 @@
 
 // RUN: mlir-opt %s --mlir-elide-resource-strings-if-larger=20| FileCheck %s
 
+// RUN: mlir-opt %s --mlir-elide-resource-strings-if-larger=0| FileCheck %s --check-prefix=ZERO
+
+
 // To ensure we print the resource keys, have reference to them
 // CHECK: attr = dense_resource<blob1> : tensor<3xi64>
+// ZERO: attr = dense_resource<blob1> : tensor<3xi64>
 "test.blob1op"() {attr = dense_resource<blob1> : tensor<3xi64> } : () -> ()
 
 // CHECK-NEXT: attr = dense_resource<blob2> : tensor<3xi64>
+// ZERO-NEXT: attr = dense_resource<blob2> : tensor<3xi64>
 "test.blob2op"() {attr = dense_resource<blob2> : tensor<3xi64> } : () -> ()
 
 // CHECK:      {-#
@@ -21,6 +26,11 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT: #-}
 
+// Make sure no external_resources are printed when --mlir-elide-resource-strings-if-larger=0
+// ZERO:      {-#
+// ZERO-EMPTY:
+// ZERO-NEXT: #-}
+
 {-#
   dialect_resources: {
     builtin: {

@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

@llvm/pr-subscribers-mlir

Author: Xiang Li (python3kgae)

Changes

When skipping the printing of large DenseResourceElementsAttr with --mlir-elide-resource-strings-if-larger,
we need to compute resourceStr to check if the string is small enough to print.

With --mlir-elide-resource-strings-if-larger set to 0, nothing should be printed, but we still compute the size.

This change will allow an early return when --mlir-elide-resource-strings-if-larger=0, making the print process faster.


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

2 Files Affected:

  • (modified) mlir/lib/IR/AsmPrinter.cpp (+4)
  • (modified) mlir/test/IR/pretty-resources-print.mlir (+10)
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index 5b5ec841917e7..c7194aca7ff12 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -3463,6 +3463,10 @@ void OperationPrinter::printResourceFileMetadata(
       std::optional<uint64_t> charLimit =
           printerFlags.getLargeResourceStringLimit();
       if (charLimit.has_value()) {
+        // Don't compute resourceStr when charLimit is 0.
+        if (charLimit.value() == 0)
+          return;
+
         llvm::raw_string_ostream ss(resourceStr);
         valueFn(ss);
 
diff --git a/mlir/test/IR/pretty-resources-print.mlir b/mlir/test/IR/pretty-resources-print.mlir
index 297c83bbb1389..980af80343d47 100644
--- a/mlir/test/IR/pretty-resources-print.mlir
+++ b/mlir/test/IR/pretty-resources-print.mlir
@@ -2,11 +2,16 @@
 
 // RUN: mlir-opt %s --mlir-elide-resource-strings-if-larger=20| FileCheck %s
 
+// RUN: mlir-opt %s --mlir-elide-resource-strings-if-larger=0| FileCheck %s --check-prefix=ZERO
+
+
 // To ensure we print the resource keys, have reference to them
 // CHECK: attr = dense_resource<blob1> : tensor<3xi64>
+// ZERO: attr = dense_resource<blob1> : tensor<3xi64>
 "test.blob1op"() {attr = dense_resource<blob1> : tensor<3xi64> } : () -> ()
 
 // CHECK-NEXT: attr = dense_resource<blob2> : tensor<3xi64>
+// ZERO-NEXT: attr = dense_resource<blob2> : tensor<3xi64>
 "test.blob2op"() {attr = dense_resource<blob2> : tensor<3xi64> } : () -> ()
 
 // CHECK:      {-#
@@ -21,6 +26,11 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT: #-}
 
+// Make sure no external_resources are printed when --mlir-elide-resource-strings-if-larger=0
+// ZERO:      {-#
+// ZERO-EMPTY:
+// ZERO-NEXT: #-}
+
 {-#
   dialect_resources: {
     builtin: {

@python3kgae python3kgae merged commit c54122f into llvm:main May 2, 2025
14 checks passed
@python3kgae python3kgae deleted the elide-resource-strings branch May 2, 2025 14:41
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…esource-strings-if-larger=0 (llvm#138275)

When skipping the printing of large DenseResourceElementsAttr with
--mlir-elide-resource-strings-if-larger,
we need to compute resourceStr to check if the string is small enough to
print.

With --mlir-elide-resource-strings-if-larger set to 0, nothing should be
printed, but we still compute the size.

This change will allow an early return when
--mlir-elide-resource-strings-if-larger=0, making the print process
faster.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…esource-strings-if-larger=0 (llvm#138275)

When skipping the printing of large DenseResourceElementsAttr with
--mlir-elide-resource-strings-if-larger,
we need to compute resourceStr to check if the string is small enough to
print.

With --mlir-elide-resource-strings-if-larger set to 0, nothing should be
printed, but we still compute the size.

This change will allow an early return when
--mlir-elide-resource-strings-if-larger=0, making the print process
faster.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…esource-strings-if-larger=0 (llvm#138275)

When skipping the printing of large DenseResourceElementsAttr with
--mlir-elide-resource-strings-if-larger,
we need to compute resourceStr to check if the string is small enough to
print.

With --mlir-elide-resource-strings-if-larger set to 0, nothing should be
printed, but we still compute the size.

This change will allow an early return when
--mlir-elide-resource-strings-if-larger=0, making the print process
faster.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…esource-strings-if-larger=0 (llvm#138275)

When skipping the printing of large DenseResourceElementsAttr with
--mlir-elide-resource-strings-if-larger,
we need to compute resourceStr to check if the string is small enough to
print.

With --mlir-elide-resource-strings-if-larger set to 0, nothing should be
printed, but we still compute the size.

This change will allow an early return when
--mlir-elide-resource-strings-if-larger=0, making the print process
faster.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants