From 67a339e96839cdecb5efad0e2731ab20a4ee458e Mon Sep 17 00:00:00 2001 From: Rahul Joshi Date: Wed, 6 Jan 2021 16:32:59 -0800 Subject: [PATCH] [MLIR] Disallow `sym_visibility`, `sym_name` and `type` attributes in the parsed attribute dictionary. Differential Revision: https://reviews.llvm.org/D94200 --- mlir/lib/IR/FunctionImplementation.cpp | 19 +++++++++++++++++-- mlir/test/Dialect/Tosa/inlining.mlir | 8 ++++---- mlir/test/IR/core-ops.mlir | 3 --- mlir/test/IR/invalid-func-op.mlir | 16 ++++++++++++++++ 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/mlir/lib/IR/FunctionImplementation.cpp b/mlir/lib/IR/FunctionImplementation.cpp index 90ea91d..4bec168 100644 --- a/mlir/lib/IR/FunctionImplementation.cpp +++ b/mlir/lib/IR/FunctionImplementation.cpp @@ -180,7 +180,7 @@ mlir::impl::parseFunctionLikeOp(OpAsmParser &parser, OperationState &result, return failure(); // Parse the function signature. - auto signatureLocation = parser.getCurrentLocation(); + llvm::SMLoc signatureLocation = parser.getCurrentLocation(); bool isVariadic = false; if (parseFunctionSignature(parser, allowVariadic, entryArgs, argTypes, argAttrs, isVariadic, resultTypes, resultAttrs)) @@ -196,9 +196,24 @@ mlir::impl::parseFunctionLikeOp(OpAsmParser &parser, OperationState &result, << (errorMessage.empty() ? "" : ": ") << errorMessage; // If function attributes are present, parse them. - if (parser.parseOptionalAttrDictWithKeyword(result.attributes)) + NamedAttrList parsedAttributes; + llvm::SMLoc attributeDictLocation = parser.getCurrentLocation(); + if (parser.parseOptionalAttrDictWithKeyword(parsedAttributes)) return failure(); + // Disallow attributes that are inferred from elsewhere in the attribute + // dictionary. + for (StringRef disallowed : + {SymbolTable::getVisibilityAttrName(), SymbolTable::getSymbolAttrName(), + getTypeAttrName()}) { + if (parsedAttributes.get(disallowed)) + return parser.emitError(attributeDictLocation, "'") + << disallowed + << "' is an inferred attribute and should not be specified in the " + "explicit attribute dictionary"; + } + result.attributes.append(parsedAttributes); + // Add the attributes to the function arguments. assert(argAttrs.size() == argTypes.size()); assert(resultAttrs.size() == resultTypes.size()); diff --git a/mlir/test/Dialect/Tosa/inlining.mlir b/mlir/test/Dialect/Tosa/inlining.mlir index 363358b..f6789fa 100644 --- a/mlir/test/Dialect/Tosa/inlining.mlir +++ b/mlir/test/Dialect/Tosa/inlining.mlir @@ -19,11 +19,11 @@ func @inlined_if_fn(%arg0: tensor, %arg1: tensor, %arg2: tensor) - }) : (tensor, tensor, tensor) -> tensor return %0 : tensor } -func @add(%arg0: tensor, %arg1: tensor) -> tensor attributes {sym_visibility = "private"} { +func private @add(%arg0: tensor, %arg1: tensor) -> tensor { %0 = "tosa.add"(%arg0, %arg1) : (tensor, tensor) -> tensor return %0 : tensor } -func @sub(%arg0: tensor, %arg1: tensor) -> tensor attributes {sym_visibility = "private"} { +func private @sub(%arg0: tensor, %arg1: tensor) -> tensor { %0 = "tosa.sub"(%arg0, %arg1) : (tensor, tensor) -> tensor return %0 : tensor } @@ -45,12 +45,12 @@ func @inlined_while_fn(%arg0: tensor, %arg1: tensor, %arg2: tensor, tensor, tensor, tensor<10xi32>) -> (tensor, tensor, tensor, tensor<10xi32>) return %1#3 : tensor<10xi32> } -func @while_body_50(%arg0: tensor, %arg1: tensor, %arg2: tensor, %arg3: tensor<10xi32>) -> (tensor, tensor, tensor, tensor<10xi32>) attributes {sym_visibility = "private"} { +func private @while_body_50(%arg0: tensor, %arg1: tensor, %arg2: tensor, %arg3: tensor<10xi32>) -> (tensor, tensor, tensor, tensor<10xi32>) { %1 = "tosa.add"(%arg0, %arg1) : (tensor, tensor) -> tensor %2 = "tosa.add"(%arg3, %1) : (tensor<10xi32>, tensor) -> tensor<10xi32> return %1, %arg1, %arg2, %2: tensor, tensor, tensor, tensor<10xi32> } -func @while_cond_40(%arg0: tensor, %arg1: tensor, %arg2: tensor, %arg3: tensor<10xi32>) -> tensor attributes {sym_visibility = "private"} { +func private @while_cond_40(%arg0: tensor, %arg1: tensor, %arg2: tensor, %arg3: tensor<10xi32>) -> tensor { %0 = "tosa.greater_equal"(%arg0, %arg1) : (tensor, tensor) -> tensor %1 = "tosa.logical_not"(%0) : (tensor) -> tensor return %1 : tensor diff --git a/mlir/test/IR/core-ops.mlir b/mlir/test/IR/core-ops.mlir index fc712d4..396211c 100644 --- a/mlir/test/IR/core-ops.mlir +++ b/mlir/test/IR/core-ops.mlir @@ -942,6 +942,3 @@ func @subtensor_insert(%t: tensor<8x16x4xf32>, %t2: tensor<16x32x8xf32>, %idx : return } - -// CHECK-LABEL: func private @legacy_visibility_syntax -func @legacy_visibility_syntax() attributes { sym_visibility = "private" } diff --git a/mlir/test/IR/invalid-func-op.mlir b/mlir/test/IR/invalid-func-op.mlir index afa7369..c2ceefe 100644 --- a/mlir/test/IR/invalid-func-op.mlir +++ b/mlir/test/IR/invalid-func-op.mlir @@ -78,3 +78,19 @@ func @f(%arg0: i64) -> (i64 {test.invalid_attr}) { // expected-error@+1 {{symbol declaration cannot have public visibility}} func @invalid_public_declaration() + +// ----- + +// expected-error@+1 {{'sym_visibility' is an inferred attribute and should not be specified in the explicit attribute dictionary}} +func @legacy_visibility_syntax() attributes { sym_visibility = "private" } + +// ----- + +// expected-error@+1 {{'sym_name' is an inferred attribute and should not be specified in the explicit attribute dictionary}} +func private @invalid_symbol_name_attr() attributes { sym_name = "x" } + +// ----- + +// expected-error@+1 {{'type' is an inferred attribute and should not be specified in the explicit attribute dictionary}} +func private @invalid_symbol_type_attr() attributes { type = "x" } + -- 2.7.4