[Mlir-commits] [mlir] 3b0106f - Fix MLIR bytecode reader for unregistered dialects

[Mehdi Amini]

Author: Mehdi Amini
Date: 2023-05-25T00:27:59-07:00
New Revision: 3b0106fe9da9a3adf9bd4402c7bc6cfbb47f1ab8

URL: https://github.com/llvm/llvm-project/commit/3b0106fe9da9a3adf9bd4402c7bc6cfbb47f1ab8
DIFF: https://github.com/llvm/llvm-project/commit/3b0106fe9da9a3adf9bd4402c7bc6cfbb47f1ab8.diff

LOG: Fix MLIR bytecode reader for unregistered dialects

At the moment we accept (in tests) unregistered dialects and in particular:

  "new_processor_id_and_range"()

where there is no `.` separator. We probably will remove support for this
from the parser, but for now we're adding compatibility support in the
reader.

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

Added: 
    mlir/test/Bytecode/unregistered_dialect.mlir

Modified: 
    mlir/lib/Bytecode/Reader/BytecodeReader.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
index 92584d5e688cd..05a1d33276101 100644
--- a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
+++ b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
@@ -14,6 +14,7 @@
 #include "mlir/Bytecode/Encoding.h"
 #include "mlir/IR/BuiltinDialect.h"
 #include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Diagnostics.h"
 #include "mlir/IR/OpImplementation.h"
 #include "mlir/IR/Verifier.h"
 #include "mlir/IR/Visitors.h"
@@ -1609,8 +1610,20 @@ BytecodeReader::Impl::parseOpName(EncodingReader &reader) {
                                 reader);
     if (failed(opName->dialect->load(dialectReader, getContext())))
       return failure();
-    opName->opName.emplace((opName->dialect->name + "." + opName->name).str(),
-                           getContext());
+    // If the opName is empty, this is because we use to accept names such as
+    // `foo` without any `.` separator. We shouldn't tolerate this in textual
+    // format anymore but for now we'll be backward compatible. This can only
+    // happen with unregistered dialects.
+    if (opName->name.empty()) {
+      if (opName->dialect->getLoadedDialect())
+        return emitError(fileLoc) << "has an empty opname for dialect '"
+                                  << opName->dialect->name << "'\n";
+
+      opName->opName.emplace(opName->dialect->name, getContext());
+    } else {
+      opName->opName.emplace((opName->dialect->name + "." + opName->name).str(),
+                             getContext());
+    }
   }
   return *opName->opName;
 }

diff  --git a/mlir/test/Bytecode/unregistered_dialect.mlir b/mlir/test/Bytecode/unregistered_dialect.mlir
new file mode 100644
index 0000000000000..d645beb83776c
--- /dev/null
+++ b/mlir/test/Bytecode/unregistered_dialect.mlir
@@ -0,0 +1,8 @@
+// RUN: mlir-opt -emit-bytecode -allow-unregistered-dialect %s | mlir-opt -allow-unregistered-dialect | FileCheck %s
+
+// verify that we round-trip an op without a dialect name (as long as we support this)
+func.func @map1d(%lb: index, %ub: index, %step: index) {
+// CHECK: "new_processor_id_and_range"
+  %0:2 = "new_processor_id_and_range"() : () -> (index, index)
+  return
+}