[Mlir-commits] [mlir] 67a339e - [MLIR] Disallow `sym_visibility`, `sym_name` and `type` attributes in the parsed attribute dictionary.

[Rahul Joshi]

Author: Rahul Joshi
Date: 2021-01-12T09:11:02-08:00
New Revision: 67a339e96839cdecb5efad0e2731ab20a4ee458e

URL: https://github.com/llvm/llvm-project/commit/67a339e96839cdecb5efad0e2731ab20a4ee458e
DIFF: https://github.com/llvm/llvm-project/commit/67a339e96839cdecb5efad0e2731ab20a4ee458e.diff

LOG: [MLIR] Disallow `sym_visibility`, `sym_name` and `type` attributes in the parsed attribute dictionary.

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

Added: 
    

Modified: 
    mlir/lib/IR/FunctionImplementation.cpp
    mlir/test/Dialect/Tosa/inlining.mlir
    mlir/test/IR/core-ops.mlir
    mlir/test/IR/invalid-func-op.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/IR/FunctionImplementation.cpp b/mlir/lib/IR/FunctionImplementation.cpp
index 90ea91d49fb6..4bec1684b5ee 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 363358b0781b..f6789fafe3ed 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<f32>, %arg1: tensor<f32>, %arg2: tensor<i1>) -
   }) : (tensor<i1>, tensor<f32>, tensor<f32>) -> tensor<f32>
   return %0 : tensor<f32>
 }
-func @add(%arg0: tensor<f32>, %arg1: tensor<f32>) -> tensor<f32> attributes {sym_visibility = "private"} {
+func private @add(%arg0: tensor<f32>, %arg1: tensor<f32>) -> tensor<f32> {
   %0 = "tosa.add"(%arg0, %arg1) : (tensor<f32>, tensor<f32>) -> tensor<f32>
   return %0 : tensor<f32>
 }
-func @sub(%arg0: tensor<f32>, %arg1: tensor<f32>) -> tensor<f32> attributes {sym_visibility = "private"} {
+func private @sub(%arg0: tensor<f32>, %arg1: tensor<f32>) -> tensor<f32> {
   %0 = "tosa.sub"(%arg0, %arg1) : (tensor<f32>, tensor<f32>) -> tensor<f32>
   return %0 : tensor<f32>
 }
@@ -45,12 +45,12 @@ func @inlined_while_fn(%arg0: tensor<i32>, %arg1: tensor<i32>, %arg2: tensor<i32
   }) : (tensor<i32>, tensor<i32>, tensor<i32>, tensor<10xi32>) -> (tensor<i32>, tensor<i32>, tensor<i32>, tensor<10xi32>)
   return %1#3 : tensor<10xi32>
 }
-func @while_body_50(%arg0: tensor<i32>, %arg1: tensor<i32>, %arg2: tensor<i32>, %arg3: tensor<10xi32>) -> (tensor<i32>, tensor<i32>, tensor<i32>, tensor<10xi32>) attributes {sym_visibility = "private"} {
+func private @while_body_50(%arg0: tensor<i32>, %arg1: tensor<i32>, %arg2: tensor<i32>, %arg3: tensor<10xi32>) -> (tensor<i32>, tensor<i32>, tensor<i32>, tensor<10xi32>) {
   %1 = "tosa.add"(%arg0, %arg1) : (tensor<i32>, tensor<i32>) -> tensor<i32>
   %2 = "tosa.add"(%arg3, %1) : (tensor<10xi32>, tensor<i32>) -> tensor<10xi32>
   return %1, %arg1, %arg2, %2: tensor<i32>, tensor<i32>, tensor<i32>, tensor<10xi32>
 }
-func @while_cond_40(%arg0: tensor<i32>, %arg1: tensor<i32>, %arg2: tensor<i32>, %arg3: tensor<10xi32>) -> tensor<i1> attributes {sym_visibility = "private"} {
+func private @while_cond_40(%arg0: tensor<i32>, %arg1: tensor<i32>, %arg2: tensor<i32>, %arg3: tensor<10xi32>) -> tensor<i1> {
   %0 = "tosa.greater_equal"(%arg0, %arg1) : (tensor<i32>, tensor<i32>) -> tensor<i1>
   %1 = "tosa.logical_not"(%0) : (tensor<i1>) -> tensor<i1>
   return %1 : tensor<i1>

diff  --git a/mlir/test/IR/core-ops.mlir b/mlir/test/IR/core-ops.mlir
index fc712d4939ba..396211c10430 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 afa736998076..c2ceefeb3ee7 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 at +1 {{symbol declaration cannot have public visibility}}
 func @invalid_public_declaration()
+
+// -----
+
+// expected-error at +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 at +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 at +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" }
+