From 3b0106fe9da9a3adf9bd4402c7bc6cfbb47f1ab8 Mon Sep 17 00:00:00 2001 From: Mehdi Amini Date: Wed, 24 May 2023 16:13:02 -0700 Subject: [PATCH] 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 --- mlir/lib/Bytecode/Reader/BytecodeReader.cpp | 17 +++++++++++++++-- mlir/test/Bytecode/unregistered_dialect.mlir | 8 ++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 mlir/test/Bytecode/unregistered_dialect.mlir diff --git a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp index 92584d5..05a1d33 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 0000000..d645beb --- /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 +} -- 2.7.4