[Mlir-commits] [mlir] 1d5140d - [mlir] Fix printing of dialect resources

[Jeff Niu]

Author: Jeff Niu
Date: 2023-07-14T21:19:20-04:00
New Revision: 1d5140dca1ff57a60dbc0bc0e9d4679b079bbba0

URL: https://github.com/llvm/llvm-project/commit/1d5140dca1ff57a60dbc0bc0e9d4679b079bbba0
DIFF: https://github.com/llvm/llvm-project/commit/1d5140dca1ff57a60dbc0bc0e9d4679b079bbba0.diff

LOG: [mlir] Fix printing of dialect resources

It was forgetting commas.

Reviewed By: rriddle, jpienaar

Differential Revision: https://reviews.llvm.org/D155348

Added: 
    

Modified: 
    mlir/lib/IR/AsmPrinter.cpp
    mlir/test/IR/file-metadata-resources.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index 78762082e05314..ea31d18d832cdc 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -3168,6 +3168,8 @@ void OperationPrinter::printResourceFileMetadata(
     function_ref<void()> checkAddMetadataDict, Operation *op) {
   // Functor used to add data entries to the file metadata dictionary.
   bool hadResource = false;
+  bool needResourceComma = false;
+  bool needEntryComma = false;
   auto processProvider = [&](StringRef dictName, StringRef name, auto &provider,
                              auto &&...providerArgs) {
     bool hadEntry = false;
@@ -3175,13 +3177,19 @@ void OperationPrinter::printResourceFileMetadata(
       checkAddMetadataDict();
 
       // Emit the top-level resource entry if we haven't yet.
-      if (!std::exchange(hadResource, true))
+      if (!std::exchange(hadResource, true)) {
+        if (needResourceComma)
+          os << "," << newLine;
         os << "  " << dictName << "_resources: {" << newLine;
+      }
       // Emit the parent resource entry if we haven't yet.
-      if (!std::exchange(hadEntry, true))
+      if (!std::exchange(hadEntry, true)) {
+        if (needEntryComma)
+          os << "," << newLine;
         os << "    " << name << ": {" << newLine;
-      else
+      } else {
         os << "," << newLine;
+      }
 
       os << "      " << key << ": ";
       valueFn(os);
@@ -3189,6 +3197,7 @@ void OperationPrinter::printResourceFileMetadata(
     ResourceBuilder entryBuilder(*this, printFn);
     provider.buildResources(op, providerArgs..., entryBuilder);
 
+    needEntryComma = hadEntry;
     if (hadEntry)
       os << newLine << "    }";
   };
@@ -3210,6 +3219,8 @@ void OperationPrinter::printResourceFileMetadata(
 
   // Print the `external_resources` section if we have any external clients with
   // resources.
+  needEntryComma = false;
+  needResourceComma = hadResource;
   hadResource = false;
   for (const auto &printer : state.getResourcePrinters())
     processProvider("external", printer.getName(), printer);

diff  --git a/mlir/test/IR/file-metadata-resources.mlir b/mlir/test/IR/file-metadata-resources.mlir
index a531c7ce975636..dabee72611a31d 100644
--- a/mlir/test/IR/file-metadata-resources.mlir
+++ b/mlir/test/IR/file-metadata-resources.mlir
@@ -1,16 +1,25 @@
-// RUN: mlir-opt %s -split-input-file | FileCheck %s
+// RUN: mlir-opt %s | mlir-opt | FileCheck %s
 
 // Check that we only preserve the blob that got referenced.
-// CHECK:      test: {
-// CHECK-NEXT:   blob1: "0x08000000010000000000000002000000000000000300000000000000"
-// CHECK-NEXT: }
+// CHECK:      {-#
+// CHECK-NEXT:   dialect_resources: {
+// CHECK-NEXT:     test: {
+// CHECK-NEXT:       blob1: "0x08000000010000000000000002000000000000000300000000000000"
+// CHECK-NEXT:     }
+// CHECK-NEXT:   },
 
 // Check that we properly preserve unknown external resources.
-// CHECK:      external: {
-// CHECK-NEXT:   blob: "0x08000000010000000000000002000000000000000300000000000000"
-// CHECK-NEXT:   bool: true
-// CHECK-NEXT:   string: "string"
-// CHECK-NEXT: }
+// CHECK-NEXT:   external_resources: {
+// CHECK-NEXT:     external: {
+// CHECK-NEXT:       blob: "0x08000000010000000000000002000000000000000300000000000000"
+// CHECK-NEXT:       bool: true
+// CHECK-NEXT:       string: "string"
+// CHECK-NEXT:     },
+// CHECK-NEXT:     other_stuff: {
+// CHECK-NEXT:       bool: true
+// CHECK-NEXT:     }
+// CHECK-NEXT:   }
+// CHECK-NEXT: #-}
 
 module attributes { test.blob_ref = #test.e1di64_elements<blob1> : tensor<*xi1>} {}
 
@@ -26,6 +35,9 @@ module attributes { test.blob_ref = #test.e1di64_elements<blob1> : tensor<*xi1>}
       blob: "0x08000000010000000000000002000000000000000300000000000000",
       bool: true,
       string: "string"
+    },
+    other_stuff: {
+      bool: true
     }
   }
 #-}