From: Mehdi Amini Date: Thu, 15 Jul 2021 02:13:30 +0000 (+0000) Subject: Defend early against operation created without a registered dialect X-Git-Tag: llvmorg-14-init~1421 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0f9e6451a836886f39137818c4f0cfd69ae31e62;p=platform%2Fupstream%2Fllvm.git Defend early against operation created without a registered dialect Reviewed By: rriddle Differential Revision: https://reviews.llvm.org/D105961 --- diff --git a/mlir/lib/IR/BuiltinAttributes.cpp b/mlir/lib/IR/BuiltinAttributes.cpp index 763ab80..85914be 100644 --- a/mlir/lib/IR/BuiltinAttributes.cpp +++ b/mlir/lib/IR/BuiltinAttributes.cpp @@ -360,7 +360,7 @@ LogicalResult OpaqueAttr::verify(function_ref emitError, << " attribute created with unregistered dialect. If this is " "intended, please call allowUnregisteredDialects() on the " "MLIRContext, or use -allow-unregistered-dialect with " - "mlir-opt"; + "the MLIR opt tool used"; } return success(); diff --git a/mlir/lib/IR/BuiltinTypes.cpp b/mlir/lib/IR/BuiltinTypes.cpp index f350596..5dfb14b 100644 --- a/mlir/lib/IR/BuiltinTypes.cpp +++ b/mlir/lib/IR/BuiltinTypes.cpp @@ -264,7 +264,7 @@ LogicalResult OpaqueType::verify(function_ref emitError, << "` type created with unregistered dialect. If this is " "intended, please call allowUnregisteredDialects() on the " "MLIRContext, or use -allow-unregistered-dialect with " - "mlir-opt"; + "the MLIR opt tool used"; } return success(); diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp index cff6865..97059ba 100644 --- a/mlir/lib/IR/Operation.cpp +++ b/mlir/lib/IR/Operation.cpp @@ -176,6 +176,14 @@ Operation::Operation(Location location, OperationName name, unsigned numResults, numRegions(numRegions), hasOperandStorage(hasOperandStorage), name(name), attrs(attributes) { assert(attributes && "unexpected null attribute dictionary"); +#ifndef NDEBUG + if (!getDialect() && !getContext()->allowsUnregisteredDialects()) + llvm::report_fatal_error( + name.getStringRef() + + " created with unregistered dialect. If this is intended, please call " + "allowUnregisteredDialects() on the MLIRContext, or use " + "-allow-unregistered-dialect with the MLIR opt tool used"); +#endif } // Operations are deleted through the destroy() member because they are diff --git a/mlir/lib/IR/Verifier.cpp b/mlir/lib/IR/Verifier.cpp index 4206867..084fd8c 100644 --- a/mlir/lib/IR/Verifier.cpp +++ b/mlir/lib/IR/Verifier.cpp @@ -230,7 +230,7 @@ LogicalResult OperationVerifier::verifyOperation( << "created with unregistered dialect. If this is " "intended, please call allowUnregisteredDialects() on the " "MLIRContext, or use -allow-unregistered-dialect with " - "mlir-opt"; + "the MLIR opt tool used"; } return success(); } diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp index 08d8dd9..3bf2260 100644 --- a/mlir/lib/Parser/Parser.cpp +++ b/mlir/lib/Parser/Parser.cpp @@ -653,7 +653,7 @@ Value OperationParser::resolveSSAUse(SSAUseInfo useInfo, Type type) { // Otherwise, this is a forward reference. Create a placeholder and remember // that we did so. - auto result = createForwardRefPlaceholder(useInfo.loc, type); + Value result = createForwardRefPlaceholder(useInfo.loc, type); entries[useInfo.number] = {result, useInfo.loc}; return maybeRecordUse(result); } @@ -730,7 +730,7 @@ Value OperationParser::createForwardRefPlaceholder(SMLoc loc, Type type) { // We create these placeholders as having an empty name, which we know // cannot be created through normal user input, allowing us to distinguish // them. - auto name = OperationName("placeholder", getContext()); + auto name = OperationName("unrealized_conversion_cast", getContext()); auto *op = Operation::create( getEncodedSourceLocation(loc), name, type, /*operands=*/{}, /*attributes=*/llvm::None, /*successors=*/{}, /*numRegions=*/0); diff --git a/mlir/test/CAPI/ir.c b/mlir/test/CAPI/ir.c index ac8a0dc..3bff3b8 100644 --- a/mlir/test/CAPI/ir.c +++ b/mlir/test/CAPI/ir.c @@ -479,6 +479,7 @@ static int constructAndTraverseIr(MlirContext ctx) { /// block/operation-relative API and their final order is checked. static void buildWithInsertionsAndPrint(MlirContext ctx) { MlirLocation loc = mlirLocationUnknownGet(ctx); + mlirContextSetAllowUnregisteredDialects(ctx, true); MlirRegion owningRegion = mlirRegionCreate(); MlirBlock nullBlock = mlirRegionGetFirstBlock(owningRegion); @@ -542,6 +543,7 @@ static void buildWithInsertionsAndPrint(MlirContext ctx) { mlirOperationDump(op); mlirOperationDestroy(op); + mlirContextSetAllowUnregisteredDialects(ctx, false); // clang-format off // CHECK-LABEL: "insertion.order.test" // CHECK: ^{{.*}}(%{{.*}}: i1 @@ -1561,6 +1563,8 @@ int testOperands() { // CHECK-LABEL: @testOperands MlirContext ctx = mlirContextCreate(); + mlirRegisterAllDialects(ctx); + mlirContextGetOrLoadDialect(ctx, mlirStringRefCreateFromCString("test")); MlirLocation loc = mlirLocationUnknownGet(ctx); MlirType indexType = mlirIndexTypeGet(ctx); @@ -1590,6 +1594,7 @@ int testOperands() { MlirValue constOneValue = mlirOperationGetResult(constOne, 0); // Create the operation under test. + mlirContextSetAllowUnregisteredDialects(ctx, true); MlirOperationState opState = mlirOperationStateGet(mlirStringRefCreateFromCString("dummy.op"), loc); MlirValue initialOperands[] = {constZeroValue}; @@ -1604,13 +1609,13 @@ int testOperands() { MlirValue opOperand = mlirOperationGetOperand(op, 0); fprintf(stderr, "Original operand: "); mlirValuePrint(opOperand, printToStderr, NULL); - // CHECK: Original operand: {{.+}} {value = 0 : index} + // CHECK: Original operand: {{.+}} constant 0 : index mlirOperationSetOperand(op, 0, constOneValue); opOperand = mlirOperationGetOperand(op, 0); fprintf(stderr, "Updated operand: "); mlirValuePrint(opOperand, printToStderr, NULL); - // CHECK: Updated operand: {{.+}} {value = 1 : index} + // CHECK: Updated operand: {{.+}} constant 1 : index mlirOperationDestroy(op); mlirOperationDestroy(constZero); @@ -1626,6 +1631,8 @@ int testClone() { // CHECK-LABEL: @testClone MlirContext ctx = mlirContextCreate(); + mlirRegisterAllDialects(ctx); + mlirContextGetOrLoadDialect(ctx, mlirStringRefCreateFromCString("std")); MlirLocation loc = mlirLocationUnknownGet(ctx); MlirType indexType = mlirIndexTypeGet(ctx); MlirStringRef valueStringRef = mlirStringRefCreateFromCString("value"); @@ -1646,8 +1653,8 @@ int testClone() { mlirOperationPrint(constZero, printToStderr, NULL); mlirOperationPrint(constOne, printToStderr, NULL); - // CHECK: %0 = "std.constant"() {value = 0 : index} : () -> index - // CHECK: %0 = "std.constant"() {value = 1 : index} : () -> index + // CHECK: constant 0 : index + // CHECK: constant 1 : index return 0; } diff --git a/mlir/test/Dialect/PDL/invalid.mlir b/mlir/test/Dialect/PDL/invalid.mlir index e371d84..009b622 100644 --- a/mlir/test/Dialect/PDL/invalid.mlir +++ b/mlir/test/Dialect/PDL/invalid.mlir @@ -167,7 +167,7 @@ pdl.pattern : benefit(1) { // expected-error@below {{expected only `pdl` operations within the pattern body}} pdl.pattern : benefit(1) { // expected-note@below {{see non-`pdl` operation defined here}} - "foo.other_op"() : () -> () + "test.foo.other_op"() : () -> () %root = pdl.operation "foo.op" pdl.rewrite %root with "foo" diff --git a/mlir/test/IR/invalid-module-op.mlir b/mlir/test/IR/invalid-module-op.mlir index 741a3a9..d0de3db 100644 --- a/mlir/test/IR/invalid-module-op.mlir +++ b/mlir/test/IR/invalid-module-op.mlir @@ -6,9 +6,9 @@ func @module_op() { // expected-error@+1 {{Operations with a 'SymbolTable' must have exactly one block}} module { ^bb1: - "module_terminator"() : () -> () + "test.dummy"() : () -> () ^bb2: - "module_terminator"() : () -> () + "test.dummy"() : () -> () } return } diff --git a/mlir/test/IR/invalid-unregistered.mlir b/mlir/test/IR/invalid-unregistered.mlir index 37ac45e..30d4b7a 100644 --- a/mlir/test/IR/invalid-unregistered.mlir +++ b/mlir/test/IR/invalid-unregistered.mlir @@ -1,5 +1,7 @@ // RUN: mlir-opt %s -split-input-file -verify-diagnostics +// REQUIRES: noasserts + // expected-error @below {{op created with unregistered dialect}} "unregistered_dialect.op"() : () -> () diff --git a/mlir/test/lit.cfg.py b/mlir/test/lit.cfg.py index dd38b8f..a362645 100644 --- a/mlir/test/lit.cfg.py +++ b/mlir/test/lit.cfg.py @@ -111,3 +111,8 @@ if config.enable_bindings_python: # lib/Bindings/Python/CMakeLists.txt for where this is set up. os.path.join(config.llvm_obj_root, 'python'), ], append_path=True) + +if config.enable_assertions: + config.available_features.add('asserts') +else: + config.available_features.add('noasserts') diff --git a/mlir/test/python/dialects/python_test.py b/mlir/test/python/dialects/python_test.py index 678b189..231c5ad 100644 --- a/mlir/test/python/dialects/python_test.py +++ b/mlir/test/python/dialects/python_test.py @@ -9,7 +9,9 @@ def run(f): # CHECK-LABEL: TEST: testAttributes def testAttributes(): - with Context(), Location.unknown(): + with Context() as ctx, Location.unknown(): + ctx.allow_unregistered_dialects = True + # # Check op construction with attributes. # diff --git a/mlir/unittests/Pass/PassManagerTest.cpp b/mlir/unittests/Pass/PassManagerTest.cpp index 6e4283d..8db7e5e6 100644 --- a/mlir/unittests/Pass/PassManagerTest.cpp +++ b/mlir/unittests/Pass/PassManagerTest.cpp @@ -91,6 +91,7 @@ struct InvalidPass : Pass { TEST(PassManagerTest, InvalidPass) { MLIRContext context; + context.allowUnregisteredDialects(); // Create a module OwningModuleRef module(ModuleOp::create(UnknownLoc::get(&context)));