From a495f960e0cc4d23e151288cfbfefe47d5e0c244 Mon Sep 17 00:00:00 2001 From: River Riddle Date: Sat, 2 Mar 2019 22:34:18 -0800 Subject: [PATCH] Introduce the notion of dialect attributes and dependent attributes. A dialect attribute derives its context from a specific dialect, whereas a dependent attribute derives context from what it is attached to. Following this, we now enforce that functions and function arguments may only contain dialect specific attributes. These are generic entities and cannot provide any specific context for a dependent attribute. Dialect attributes are defined as: dialect-namespace `.` attr-name `:` attribute-value Dialects can override any of the following hooks to verify the validity of a given attribute: * verifyFunctionAttribute * verifyFunctionArgAttribute * verifyInstructionAttribute PiperOrigin-RevId: 236507970 --- mlir/g3doc/LangRef.md | 72 ++++++++++++++++++++++++---------- mlir/include/mlir/IR/Dialect.h | 20 ++++++++++ mlir/lib/Analysis/Verifier.cpp | 55 ++++++++++++++++++++++---- mlir/lib/IR/AsmPrinter.cpp | 22 ++--------- mlir/lib/Parser/Parser.cpp | 10 +---- mlir/test/IR/invalid.mlir | 5 +++ mlir/test/IR/parser.mlir | 26 ++++++------ 7 files changed, 143 insertions(+), 67 deletions(-) diff --git a/mlir/g3doc/LangRef.md b/mlir/g3doc/LangRef.md index 7a7b07708ce6..4d975af9cfe7 100644 --- a/mlir/g3doc/LangRef.md +++ b/mlir/g3doc/LangRef.md @@ -867,14 +867,48 @@ Syntax: ``` {.ebnf} attribute-dict ::= `{` `}` | `{` attribute-entry (`,` attribute-entry)* `}` -attribute-entry ::= `:`? bare-id `:` attribute-value +attribute-entry ::= dialect-attribute-entry | dependent-attribute-entry +dialect-attribute-entry ::= dialect-namespace `.` bare-id `:` attribute-value +dependent-attribute-entry ::= dependent-attribute-name `:` attribute-value +dependent-attribute-name ::= (letter|[_]) (letter|digit|[_$])* ``` Attributes are the mechanism for specifying constant data in MLIR in places where a variable is never allowed - e.g. the index of a -[`dim` operation](#'dim'-operation), or the stride of a convolution. +[`dim` operation](#'dim'-operation), or the stride of a convolution. They +consist of a name and a [concrete attribute value](#attribute-values). It is +possible to attach attributes to operations, functions, and function arguments. +The set of expected attributes, their structure, and their interpretation are +all contextually dependent on what they are attached to. -Attributes have a name, and their values are represented by the following forms: +There are two main classes of attributes; dependent and dialect. Dependent +attributes derive their structure and meaning from what they are attached to, +e.g the meaning of the `index` attribute on a `dim` operation is defined by the +`dim` operation. Dialect attributes, on the other hand, derive their context and +meaning from a specific dialect. An example of a dialect attribute may be a +`swift.self` function argument attribute that indicates an argument is the +self/context parameter. The context of this attribute is defined by the `swift` +dialect and not the function argument. + +### Function and Argument Attributes + +Functions and function arguments in MLIR may have optional attributes attached +to them. The sole constraint for these attributes is that they must be dialect +specific attributes. This is because functions, and function arguments, are a +generic entities and thus cannot apply any meaningful context necessary for +dependent attributes. This has the added benefit of avoiding collisions between +common attribute names, such as `noalias`. + +### Operation Attributes + +Operations, unlike functions and function arguments, may include both dialect +specific and dependent attributes. This is because an operation represents a +distinct semantic context, and can thus provide a single source of meaning to +dependent attributes. + +### Attribute Values {#attribute-values} + +Attributes values are represented by the following forms: ``` {.ebnf} attribute-value ::= affine-map-attribute @@ -889,11 +923,7 @@ attribute-value ::= affine-map-attribute | type-attribute ``` -It is possible to attach attributes to instructions and functions, and the set -of expected attributes, their structure, and the definition of that meaning is -contextually dependent on the operation they are attached to. - -### AffineMap Attribute {#affine-map-attribute} +#### AffineMap Attribute {#affine-map-attribute} Syntax: @@ -903,7 +933,7 @@ affine-map-attribute ::= affine-map An affine-map attribute is an attribute that represents a affine-map object. -### Array Attribute {#array-attribute} +#### Array Attribute {#array-attribute} Syntax: @@ -914,7 +944,7 @@ array-attribute ::= `[` (attribute-value (`,` attribute-value)*)? `]` An array attribute is an attribute that represents a collection of attribute values. -### Boolean Attribute {#bool-attribute} +#### Boolean Attribute {#bool-attribute} Syntax: @@ -925,7 +955,7 @@ bool-attribute ::= bool-literal A boolean attribute is a literal attribute that represents a one-bit boolean value, true or false. -### Elements Attributes {#elements-attributes} +#### Elements Attributes {#elements-attributes} Syntax: @@ -939,7 +969,7 @@ elements-attribute ::= dense-elements-attribute An elements attribute is a literal attribute that represents a constant [vector](#vector-type) or [tensor](#tensor-type) value. -#### Dense Elements Attribute {#dense-elements-attribute} +##### Dense Elements Attribute {#dense-elements-attribute} Syntax: @@ -953,7 +983,7 @@ constant vector or tensor value has been packed to the element bitwidth. The element type of the vector or tensor constant must be of integer, index, or floating point type. -#### Opaque Elements Attribute {#opaque-elements-attribute} +##### Opaque Elements Attribute {#opaque-elements-attribute} Syntax: @@ -970,7 +1000,7 @@ it. Note: The parsed string literal must be in hexadecimal form. -#### Sparse Elements Attribute {#sparse-elements-attribute} +##### Sparse Elements Attribute {#sparse-elements-attribute} Syntax: @@ -1000,7 +1030,7 @@ Example: /// [0, 0, 0, 0]] ``` -#### Splat Elements Attribute {#splat-elements-attribute} +##### Splat Elements Attribute {#splat-elements-attribute} Syntax: @@ -1012,7 +1042,7 @@ splat-elements-attribute ::= `splat` `<` ( tensor-type | vector-type ) `,` A splat elements attribute is an elements attribute that represents a tensor or vector constant where all elements have the same value. -### Integer Attribute {#integer-attribute} +#### Integer Attribute {#integer-attribute} Syntax: @@ -1024,7 +1054,7 @@ An integer attribute is a literal attribute that represents an integral value of the specified integer or index type. The default type for this attribute, if one is not specified, is a 64-bit integer. -### Integer Set Attribute {#integer-set-attribute} +#### Integer Set Attribute {#integer-set-attribute} Syntax: @@ -1034,7 +1064,7 @@ integer-set-attribute ::= affine-map An integer-set attribute is an attribute that represents a integer-set object. -### Float Attribute {#float-attribute} +#### Float Attribute {#float-attribute} Syntax: @@ -1045,7 +1075,7 @@ float-attribute ::= float-literal (`:` float-type)? A float attribute is a literal attribute that represents a floating point value of the specified [float type](#floating-point-types). -### Function Attribute {#function-attribute} +#### Function Attribute {#function-attribute} Syntax: @@ -1056,7 +1086,7 @@ function-attribute ::= function-id `:` function-type A function attribute is a literal attribute that represents a reference to the given function object. -### String Attribute {#string-attribute} +#### String Attribute {#string-attribute} Syntax: @@ -1066,7 +1096,7 @@ string-attribute ::= string-literal A string attribute is an attribute that represents a string literal value. -### Type Attribute {#type-attribute} +#### Type Attribute {#type-attribute} Syntax: diff --git a/mlir/include/mlir/IR/Dialect.h b/mlir/include/mlir/IR/Dialect.h index 1024c139ad52..d677f97fa005 100644 --- a/mlir/include/mlir/IR/Dialect.h +++ b/mlir/include/mlir/IR/Dialect.h @@ -101,6 +101,26 @@ public: virtual void getTypeAliases(SmallVectorImpl> &aliases) {} + /// Verify an attribute from this dialect on the given function. Returns true + /// if the verification failed, false otherwise. + virtual bool verifyFunctionAttribute(const Function *, Attribute) { + return false; + } + + /// Verify an attribute from this dialect on the argument at 'argIndex' for + /// the given function. Returns true if the verification failed, false + /// otherwise. + virtual bool verifyFunctionArgAttribute(const Function *, unsigned argIndex, + Attribute) { + return false; + } + + /// Verify an attribute from this dialect on the given instruction. Returns + /// true if the verification failed, false otherwise. + virtual bool verifyInstructionAttribute(const Instruction *, Attribute) { + return false; + } + virtual ~Dialect(); /// Utility function that returns if the given string is a valid dialect diff --git a/mlir/lib/Analysis/Verifier.cpp b/mlir/lib/Analysis/Verifier.cpp index 1be4a86692ac..9f617e6d5f1e 100644 --- a/mlir/lib/Analysis/Verifier.cpp +++ b/mlir/lib/Analysis/Verifier.cpp @@ -35,6 +35,7 @@ #include "mlir/Analysis/Dominance.h" #include "mlir/IR/Attributes.h" +#include "mlir/IR/Dialect.h" #include "mlir/IR/Function.h" #include "mlir/IR/Instruction.h" #include "mlir/IR/Module.h" @@ -68,6 +69,15 @@ public: return failure(message, fn); } + /// Returns the registered dialect for a dialect-specific attribute. + template + Dialect *getDialectForAttribute(const NamedAttribute &attr, + const ErrorContext &ctx) { + assert(attr.first.strref().contains('.') && "expected dialect attribute"); + auto dialectNamePair = attr.first.strref().split('.'); + return fn.getContext()->getRegisteredDialect(dialectNamePair.first); + } + template bool verifyAttribute(Attribute attr, const ErrorContext &ctx) { if (!attr.isOrContainsFunction()) @@ -103,7 +113,7 @@ public: bool verifyInstDominance(const Instruction &inst); explicit FuncVerifier(const Function &fn) - : fn(fn), attrNameRegex("^:?[a-zA-Z_][a-zA-Z_0-9\\.\\$]*$") {} + : fn(fn), identifierRegex("^[a-zA-Z_][a-zA-Z_0-9\\.\\$]*$") {} private: /// The function being checked. @@ -113,7 +123,7 @@ private: DominanceInfo *domInfo = nullptr; /// Regex checker for attribute names. - llvm::Regex attrNameRegex; + llvm::Regex identifierRegex; }; } // end anonymous namespace @@ -122,29 +132,53 @@ bool FuncVerifier::verify() { fn.getName().c_str()); // Check that the function name is valid. - llvm::Regex funcNameRegex("^[a-zA-Z_][a-zA-Z_0-9\\.\\$]*$"); - if (!funcNameRegex.match(fn.getName().strref())) + if (!identifierRegex.match(fn.getName().strref())) return failure("invalid function name '" + fn.getName().strref() + "'", fn); /// Verify that all of the attributes are okay. for (auto attr : fn.getAttrs()) { - if (!attrNameRegex.match(attr.first)) + if (!identifierRegex.match(attr.first)) return failure("invalid attribute name '" + attr.first.strref() + "'", fn); if (verifyAttribute(attr.second, fn)) return true; + + /// Check that the attribute is a dialect attribute, i.e. contains a '.' for + /// the namespace. + if (!attr.first.strref().contains('.')) { + // TODO: Remove the remaining usages of non dialect attributes on + // functions and then enable this check. + // return failure("functions may only have dialect attributes", fn); + continue; + } + + // Verify this attribute with the defining dialect. + if (auto *dialect = getDialectForAttribute(attr, fn)) + if (dialect->verifyFunctionAttribute(&fn, attr.second)) + return true; } /// Verify that all of the argument attributes are okay. for (unsigned i = 0, e = fn.getNumArguments(); i != e; ++i) { for (auto attr : fn.getArgAttrs(i)) { - if (!attrNameRegex.match(attr.first)) + if (!identifierRegex.match(attr.first)) return failure( llvm::formatv("invalid attribute name '{0}' on argument {1}", attr.first.strref(), i), fn); if (verifyAttribute(attr.second, fn)) return true; + + /// Check that the attribute is a dialect attribute, i.e. contains a '.' + /// for the namespace. + if (!attr.first.strref().contains('.')) + return failure("function arguments may only have dialect attributes", + fn); + + // Verify this attribute with the defining dialect. + if (auto *dialect = getDialectForAttribute(attr, fn)) + if (dialect->verifyFunctionArgAttribute(&fn, i, attr.second)) + return true; } } @@ -257,11 +291,18 @@ bool FuncVerifier::verifyOperation(const Instruction &op) { /// Verify that all of the attributes are okay. for (auto attr : op.getAttrs()) { - if (!attrNameRegex.match(attr.first)) + if (!identifierRegex.match(attr.first)) return failure("invalid attribute name '" + attr.first.strref() + "'", op); if (verifyAttribute(attr.second, op)) return true; + + // Check for any optional dialect specific attributes. + if (!attr.first.strref().contains('.')) + continue; + if (auto *dialect = getDialectForAttribute(attr, op)) + if (dialect->verifyInstructionAttribute(&op, attr.second)) + return true; } // If we can get operation info for this, check the custom hook. diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp index b96501960c49..b88de85482bf 100644 --- a/mlir/lib/IR/AsmPrinter.cpp +++ b/mlir/lib/IR/AsmPrinter.cpp @@ -77,12 +77,6 @@ static llvm::cl::opt llvm::cl::desc("Print the generic op form"), llvm::cl::init(false), llvm::cl::Hidden); -static llvm::cl::opt printInternalAttributes( - "mlir-print-internal-attributes", - llvm::cl::desc( - "Print internal function and instruction attributes (':' prefix)."), - llvm::cl::init(false)); - namespace { class ModuleState { public: @@ -1022,21 +1016,13 @@ void ModulePrinter::printOptionalAttrDict(ArrayRef attrs, // Filter out any attributes that shouldn't be included. SmallVector filteredAttrs; for (auto attr : attrs) { - auto attrName = attr.first.strref(); - // By default, never print attributes that start with a colon. These are - // attributes represent location or other internal metadata. - if (!printInternalAttributes && attrName.startswith(":")) - return; - // If the caller has requested that this attribute be ignored, then drop it. - bool ignore = false; - for (auto elide : elidedAttrs) - ignore |= attrName == elide; + if (llvm::any_of(elidedAttrs, + [&](StringRef elided) { return attr.first.is(elided); })) + continue; // Otherwise add it to our filteredAttrs list. - if (!ignore) { - filteredAttrs.push_back(attr); - } + filteredAttrs.push_back(attr); } // If there are no attributes left to print after filtering, then we're done. diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp index d6805562f350..7d977ae953d4 100644 --- a/mlir/lib/Parser/Parser.cpp +++ b/mlir/lib/Parser/Parser.cpp @@ -1432,7 +1432,7 @@ ParseResult Parser::parseLocationInstance(llvm::Optional *loc) { /// /// attribute-dict ::= `{` `}` /// | `{` attribute-entry (`,` attribute-entry)* `}` -/// attribute-entry ::= `:`? bare-id `:` attribute-value +/// attribute-entry ::= bare-id `:` attribute-value /// ParseResult Parser::parseAttributeDict(SmallVectorImpl &attributes) { @@ -1440,17 +1440,11 @@ Parser::parseAttributeDict(SmallVectorImpl &attributes) { return ParseFailure; auto parseElt = [&]() -> ParseResult { - // Check for an internal attribute. - bool isInternalAttr = consumeIf(Token::colon); - // We allow keywords as attribute names. if (getToken().isNot(Token::bare_identifier, Token::inttype) && !getToken().isKeyword()) return emitError("expected attribute name"); - Identifier nameId = - isInternalAttr - ? builder.getIdentifier(Twine(":" + getTokenSpelling()).str()) - : builder.getIdentifier(getTokenSpelling()); + Identifier nameId = builder.getIdentifier(getTokenSpelling()); consumeToken(); if (parseToken(Token::colon, "expected ':' in attribute list")) diff --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir index e6f85524a114..b96eaaaeca5f 100644 --- a/mlir/test/IR/invalid.mlir +++ b/mlir/test/IR/invalid.mlir @@ -928,3 +928,8 @@ func @invalid_unknown_type_dialect_name() -> !invalid.dialect<""> // expected-error @+1 {{@ identifier expected to start with letter or '_'}} func @$invalid_function_name() + +// ----- + +// expected-error @+1 {{function arguments may only have dialect attributes}} +func @invalid_func_arg_attr(i1 {non_dialect_attr: 10}) diff --git a/mlir/test/IR/parser.mlir b/mlir/test/IR/parser.mlir index 04068edd7e84..34b746214905 100644 --- a/mlir/test/IR/parser.mlir +++ b/mlir/test/IR/parser.mlir @@ -1,4 +1,4 @@ -// RUN: mlir-opt -mlir-print-internal-attributes %s | FileCheck %s +// RUN: mlir-opt %s | FileCheck %s // CHECK-DAG: #map{{[0-9]+}} = (d0, d1, d2, d3, d4)[s0] -> (d0, d1, d2, d4, d3) #map0 = (d0, d1, d2, d3, d4)[s0] -> (d0, d1, d2, d4, d3) @@ -517,8 +517,8 @@ func @floatAttrs() -> () { // CHECK-LABEL: func @externalfuncattr func @externalfuncattr() -> () - // CHECK: attributes {a: "a\22quoted\22string", b: 4.000000e+00, c: tensor<*xf32>} - attributes {a: "a\"quoted\"string", b: 4.0, c: tensor<*xf32>} + // CHECK: attributes {dialect.a: "a\22quoted\22string", dialect.b: 4.000000e+00, dialect.c: tensor<*xf32>} + attributes {dialect.a: "a\"quoted\"string", dialect.b: 4.0, dialect.c: tensor<*xf32>} // CHECK-LABEL: func @funcattrempty func @funcattrempty() -> () @@ -527,8 +527,8 @@ func @funcattrempty() -> () // CHECK-LABEL: func @funcattr func @funcattr() -> () - // CHECK: attributes {a: "a\22quoted\22string", b: 4.000000e+00, c: tensor<*xf32>} - attributes {a: "a\"quoted\"string", b: 4.0, c: tensor<*xf32>} { + // CHECK: attributes {dialect.a: "a\22quoted\22string", dialect.b: 4.000000e+00, dialect.c: tensor<*xf32>} + attributes {dialect.a: "a\"quoted\"string", dialect.b: 4.0, dialect.c: tensor<*xf32>} { ^bb0: return } @@ -803,20 +803,20 @@ func @unregistered_term(%arg0 : i1) -> i1 { return %arg1 : i1 } -// CHECK-LABEL: func @internal_attrs -func @internal_attrs() - // CHECK-NEXT: attributes {:internal.attr: 10 - attributes {:internal.attr: 10} { +// CHECK-LABEL: func @dialect_attrs +func @dialect_attrs() + // CHECK-NEXT: attributes {dialect.attr: 10 + attributes {dialect.attr: 10} { return } // CHECK-LABEL: func @_valid.function$name func @_valid.function$name() -// CHECK-LABEL: func @external_func_arg_attrs(i32, i1 {arg.attr: 10}, i32) -func @external_func_arg_attrs(i32, i1 {arg.attr: 10}, i32) +// CHECK-LABEL: func @external_func_arg_attrs(i32, i1 {dialect.attr: 10}, i32) +func @external_func_arg_attrs(i32, i1 {dialect.attr: 10}, i32) -// CHECK-LABEL: func @func_arg_attrs(%arg0: i1 {arg.attr: 10}) -func @func_arg_attrs(%arg0: i1 {arg.attr: 10}) { +// CHECK-LABEL: func @func_arg_attrs(%arg0: i1 {dialect.attr: 10}) +func @func_arg_attrs(%arg0: i1 {dialect.attr: 10}) { return }