llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-mlir-core Author: Krzysztof Drewniak (krzysz00) <details> <summary>Changes</summary> Give the properties from tablegen a `predicate` field that holds the predicate that the property needs to satisfy, if one exists, and hook that field up to verifier generation. --- Patch is 26.65 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/120176.diff 8 Files Affected: - (modified) mlir/include/mlir/IR/Properties.td (+96-33) - (modified) mlir/include/mlir/TableGen/Property.h (+5) - (modified) mlir/lib/TableGen/Property.cpp (+12-2) - (added) mlir/test/IR/test-op-property-predicates.mlir (+148) - (modified) mlir/test/lib/Dialect/Test/TestOps.td (+28-4) - (added) mlir/test/mlir-tblgen/op-properties-predicates.td (+72) - (modified) mlir/test/mlir-tblgen/op-properties.td (+1-1) - (modified) mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp (+55) ``````````diff diff --git a/mlir/include/mlir/IR/Properties.td b/mlir/include/mlir/IR/Properties.td index 0becf7d0098356..63ba0fda2ac979 100644 --- a/mlir/include/mlir/IR/Properties.td +++ b/mlir/include/mlir/IR/Properties.td @@ -13,6 +13,8 @@ #ifndef PROPERTIES #define PROPERTIES +include "mlir/IR/Constraints.td" + // Base class for defining properties. class Property<string storageTypeParam = "", string desc = ""> { // User-readable one line summary used in error reporting messages. If empty, @@ -63,6 +65,12 @@ class Property<string storageTypeParam = "", string desc = ""> { return convertFromAttribute($_storage, $_attr, $_diag); }]; + // The verification predicate for this property. Defaults to And<[]>, + // which is trivially true, since properties are always their expected type. + // Within the predicate, `$_self` is an instance of the **interface** + // type of the property. + Pred predicate = ?; + // The call expression to hash the property. // // Format: @@ -150,8 +158,8 @@ class Property<string storageTypeParam = "", string desc = ""> { return ::mlir::failure(); }]; - // Base definition for the property. (Will be) used for `OptionalProperty` and - // such cases, analogously to `baseAttr`. + // Base definition for the property. Used to look through `OptionalProperty` + // for some format generation, as with the `baseAttr` field on attributes. Property baseProperty = ?; // Default value for the property within its storage. This should be an expression @@ -224,8 +232,7 @@ def I64Property : IntProperty<"int64_t">; class EnumProperty<string storageTypeParam, string desc = "", string default = ""> : Property<storageTypeParam, desc> { - // TODO: take advantage of EnumAttrInfo and the like to make this share nice - // parsing code with EnumAttr. + // TODO: implement predicate for enum validity. let writeToMlirBytecode = [{ $_writer.writeVarInt(static_cast<uint64_t>($_storage)); }]; @@ -330,6 +337,56 @@ def UnitProperty : Property<"bool", "unit property"> { }]; } +//===----------------------------------------------------------------------===// +// Property field overwrites + +/// Class for giving a property a default value. +/// This doesn't change anything about the property other than giving it a default +/// which can be used by ODS to elide printing. +class DefaultValuedProperty<Property p, string default = "", string storageDefault = ""> : Property<p.storageType, p.summary> { + let defaultValue = default; + let storageTypeValueOverride = storageDefault; + let baseProperty = p; + // Keep up to date with `Property` above. + let summary = p.summary; + let description = p.description; + let storageType = p.storageType; + let interfaceType = p.interfaceType; + let convertFromStorage = p.convertFromStorage; + let assignToStorage = p.assignToStorage; + let convertToAttribute = p.convertToAttribute; + let convertFromAttribute = p.convertFromAttribute; + let predicate = p.predicate; + let hashProperty = p.hashProperty; + let parser = p.parser; + let optionalParser = p.optionalParser; + let printer = p.printer; + let readFromMlirBytecode = p.readFromMlirBytecode; + let writeToMlirBytecode = p.writeToMlirBytecode; +} + +class ConfinedProperty<Property p, Pred pred, string newSummary = ""> + : Property<p.storageType, !if(!empty(newSummary), p.summary, newSummary)> { + let predicate = !if(!initialized(p.predicate), And<[p.predicate, pred]>, pred); + let baseProperty = p; + // Keep up to date with `Property` above. + let description = p.description; + let storageType = p.storageType; + let interfaceType = p.interfaceType; + let convertFromStorage = p.convertFromStorage; + let assignToStorage = p.assignToStorage; + let convertToAttribute = p.convertToAttribute; + let convertFromAttribute = p.convertFromAttribute; + let hashProperty = p.hashProperty; + let parser = p.parser; + let optionalParser = p.optionalParser; + let printer = p.printer; + let readFromMlirBytecode = p.readFromMlirBytecode; + let writeToMlirBytecode = p.writeToMlirBytecode; + let defaultValue = p.defaultValue; + let storageTypeValueOverride = p.storageTypeValueOverride; +} + //===----------------------------------------------------------------------===// // Primitive property combinators @@ -342,14 +399,37 @@ class _makePropStorage<Property prop, string name> { true : "") # ";"; } +/// Construct a `Pred`icate `ret` that wraps the predicate of the underlying +/// property `childProp` with: +/// +/// [](childProp.storageType& s) { +/// return [](childProp.interfaceType i) { +/// return leafSubst(childProp.predicate, "$_self" to "i"); +/// }(childProp.convertFromStorage(s)) +/// } +/// +/// and then appends `prefix` and `suffix`. +class _makeChildWrapperPred<string prefix, Property wrappedProp, string suffix> { + Pred ret = + !if(!initialized(wrappedProp.predicate), + Concat< + prefix # "[](" + # "const " # wrappedProp.storageType # "& baseStore) -> bool { return [](" + # wrappedProp.interfaceType # " baseIface) -> bool { return (", + SubstLeaves<"$_self", "baseIface", wrappedProp.predicate>, + "); }(" # !subst("$_storage", "baseStore", wrappedProp.convertFromStorage) + # "); }" # suffix + >, ?); +} + /// The generic class for arrays of some other property, which is stored as a /// `SmallVector` of that property. This uses an `ArrayAttr` as its attribute form /// though subclasses can override this, as is the case with IntArrayAttr below. /// Those wishing to use a non-default number of SmallVector elements should /// subclass `ArrayProperty`. -class ArrayProperty<Property elem = Property<>, string desc = ""> : - Property<"::llvm::SmallVector<" # elem.storageType # ">", desc> { - let summary = "array of " # elem.summary; +class ArrayProperty<Property elem = Property<>, string newSummary = ""> : + Property<"::llvm::SmallVector<" # elem.storageType # ">", + !if(!empty(newSummary), "array of " # elem.summary, newSummary)> { let interfaceType = "::llvm::ArrayRef<" # elem.storageType # ">"; let convertFromStorage = "::llvm::ArrayRef<" # elem.storageType # ">{$_storage}"; let assignToStorage = "$_storage.assign($_value.begin(), $_value.end())"; @@ -382,6 +462,8 @@ class ArrayProperty<Property elem = Property<>, string desc = ""> : return ::mlir::ArrayAttr::get($_ctxt, elems); }]; + let predicate = _makeChildWrapperPred<"::llvm::all_of($_self, ", elem, ")">.ret; + defvar theParserBegin = [{ auto& storage = $_storage; auto parseElemFn = [&]() -> ::mlir::ParseResult { @@ -463,8 +545,8 @@ class ArrayProperty<Property elem = Property<>, string desc = ""> : }]); } -class IntArrayProperty<string storageTypeParam = "", string desc = ""> : - ArrayProperty<IntProperty<storageTypeParam, desc>> { +class IntArrayProperty<Property elem, string newSummary=""> : + ArrayProperty<elem, newSummary> { // Bring back the trivial conversions we don't get in the general case. let convertFromAttribute = [{ return convertFromAttribute($_storage, $_attr, $_diag); @@ -474,30 +556,6 @@ class IntArrayProperty<string storageTypeParam = "", string desc = ""> : }]; } -/// Class for giving a property a default value. -/// This doesn't change anything about the property other than giving it a default -/// which can be used by ODS to elide printing. -class DefaultValuedProperty<Property p, string default = "", string storageDefault = ""> : Property<p.storageType, p.summary> { - let defaultValue = default; - let storageTypeValueOverride = storageDefault; - let baseProperty = p; - // Keep up to date with `Property` above. - let summary = p.summary; - let description = p.description; - let storageType = p.storageType; - let interfaceType = p.interfaceType; - let convertFromStorage = p.convertFromStorage; - let assignToStorage = p.assignToStorage; - let convertToAttribute = p.convertToAttribute; - let convertFromAttribute = p.convertFromAttribute; - let hashProperty = p.hashProperty; - let parser = p.parser; - let optionalParser = p.optionalParser; - let printer = p.printer; - let readFromMlirBytecode = p.readFromMlirBytecode; - let writeToMlirBytecode = p.writeToMlirBytecode; -} - /// An optional property, stored as an std::optional<p.storageType> /// interfaced with as an std::optional<p.interfaceType>.. /// The syntax is `none` (or empty string if elided) for an absent value or @@ -575,6 +633,11 @@ class OptionalProperty<Property p, bit canDelegateParsing = 1> return ::mlir::ArrayAttr::get($_ctxt, {attr}); }]; + let predicate = !if(!initialized(p.predicate), + Or<[CPred<"!$_self.has_value()">, + SubstLeaves<"$_self", "(*($_self))", p.predicate>]>, + ?); + defvar delegatedParserBegin = [{ if (::mlir::succeeded($_parser.parseOptionalKeyword("none"))) { $_storage = std::nullopt; diff --git a/mlir/include/mlir/TableGen/Property.h b/mlir/include/mlir/TableGen/Property.h index 702e6756e6a95c..386159191ef1f4 100644 --- a/mlir/include/mlir/TableGen/Property.h +++ b/mlir/include/mlir/TableGen/Property.h @@ -27,6 +27,7 @@ namespace mlir { namespace tblgen { class Dialect; class Type; +class Pred; // Wrapper class providing helper methods for accessing MLIR Property defined // in TableGen. This class should closely reflect what is defined as class @@ -74,6 +75,10 @@ class Property { return convertFromAttributeCall; } + // Return the property's predicate. Properties that didn't come from + // tablegen (the hardcoded ones) have the null predicate. + Pred getPredicate() const; + // Returns the method call which parses this property from textual MLIR. StringRef getParserCall() const { return parserCall; } diff --git a/mlir/lib/TableGen/Property.cpp b/mlir/lib/TableGen/Property.cpp index b86b87df91c607..13851167ddaf75 100644 --- a/mlir/lib/TableGen/Property.cpp +++ b/mlir/lib/TableGen/Property.cpp @@ -14,6 +14,7 @@ #include "mlir/TableGen/Property.h" #include "mlir/TableGen/Format.h" #include "mlir/TableGen/Operator.h" +#include "mlir/TableGen/Predicate.h" #include "llvm/TableGen/Record.h" using namespace mlir; @@ -68,8 +69,8 @@ Property::Property(StringRef summary, StringRef description, StringRef writeToMlirBytecodeCall, StringRef hashPropertyCall, StringRef defaultValue, StringRef storageTypeValueOverride) - : summary(summary), description(description), storageType(storageType), - interfaceType(interfaceType), + : def(nullptr), summary(summary), description(description), + storageType(storageType), interfaceType(interfaceType), convertFromStorageCall(convertFromStorageCall), assignToStorageCall(assignToStorageCall), convertToAttributeCall(convertToAttributeCall), @@ -91,6 +92,15 @@ StringRef Property::getPropertyDefName() const { return def->getName(); } +Pred Property::getPredicate() const { + if (!def) + return Pred(); + const llvm::RecordVal *maybePred = def->getValue("predicate"); + if (!maybePred || !maybePred->getValue()) + return Pred(); + return Pred(maybePred->getValue()); +} + Property Property::getBaseProperty() const { if (const auto *defInit = llvm::dyn_cast<llvm::DefInit>(def->getValueInit("baseProperty"))) { diff --git a/mlir/test/IR/test-op-property-predicates.mlir b/mlir/test/IR/test-op-property-predicates.mlir new file mode 100644 index 00000000000000..4dba420bc8f825 --- /dev/null +++ b/mlir/test/IR/test-op-property-predicates.mlir @@ -0,0 +1,148 @@ +// RUN: mlir-opt -split-input-file -verify-diagnostics %s + +test.op_with_property_predicates <{ + scalar = 1 : i64, + optional = [2 : i64], + defaulted = 3 : i64, + more_constrained = 4 : i64, + array = [], + non_empty_unconstrained = [5: i64], + non_empty_constrained = [6 : i64], + non_empty_optional = [[7 : i64]], + unconstrained = 8 : i64}> + +// ----- + +test.op_with_property_predicates <{ + scalar = 1 : i64, + more_constrained = 4 : i64, + array = [], + non_empty_unconstrained = [5: i64], + non_empty_constrained = [6 : i64], + unconstrained = 8 : i64}> + +// ----- + +// expected-error @+1 {{'test.op_with_property_predicates' op property 'scalar' failed to satiisfy constraint: non-negative int64_t}} +test.op_with_property_predicates <{ + scalar = -1 : i64, + optional = [2 : i64], + defaulted = 3 : i64, + more_constrained = 4 : i64, + array = [], + non_empty_unconstrained = [5: i64], + non_empty_constrained = [6 : i64], + non_empty_optional = [[7 : i64]], + unconstrained = 8 : i64}> + +// ----- + +// expected-error @+1 {{'test.op_with_property_predicates' op property 'optional' failed to satiisfy constraint: optional non-negative int64_t}} +test.op_with_property_predicates <{ + scalar = 1 : i64, + optional = [-1 : i64], + defaulted = 3 : i64, + more_constrained = 4 : i64, + array = [], + non_empty_unconstrained = [5: i64], + non_empty_constrained = [6 : i64], + non_empty_optional = [[7 : i64]], + unconstrained = 8 : i64}> + +// ----- + +// expected-error @+1 {{'test.op_with_property_predicates' op property 'defaulted' failed to satiisfy constraint: non-negative int64_t}} +test.op_with_property_predicates <{ + scalar = 1 : i64, + optional = [2 : i64], + defaulted = -1 : i64, + more_constrained = 4 : i64, + array = [], + non_empty_unconstrained = [5: i64], + non_empty_constrained = [6 : i64], + non_empty_optional = [[7 : i64]], + unconstrained = 8 : i64}> + +// ----- + +// expected-error @+1 {{'test.op_with_property_predicates' op property 'more_constrained' failed to satiisfy constraint: between 0 and 5}} +test.op_with_property_predicates <{ + scalar = 1 : i64, + optional = [2 : i64], + defaulted = 3 : i64, + more_constrained = 100 : i64, + array = [], + non_empty_unconstrained = [5: i64], + non_empty_constrained = [6 : i64], + non_empty_optional = [[7 : i64]], + unconstrained = 8 : i64}> + +// ----- + +// expected-error @+1 {{'test.op_with_property_predicates' op property 'array' failed to satiisfy constraint: array of non-negative int64_t}} +test.op_with_property_predicates <{ + scalar = 1 : i64, + optional = [2 : i64], + defaulted = 3 : i64, + more_constrained = 4 : i64, + array = [-1 : i64], + non_empty_unconstrained = [5: i64], + non_empty_constrained = [6 : i64], + non_empty_optional = [[7 : i64]], + unconstrained = 8 : i64}> + +// ----- + +// expected-error @+1 {{'test.op_with_property_predicates' op property 'non_empty_unconstrained' failed to satiisfy constraint: non-empty array of int64_t}} +test.op_with_property_predicates <{ + scalar = 1 : i64, + optional = [2 : i64], + defaulted = 3 : i64, + more_constrained = 4 : i64, + array = [], + non_empty_unconstrained = [], + non_empty_constrained = [6 : i64], + non_empty_optional = [[7 : i64]], + unconstrained = 8 : i64}> + +// ----- + +// expected-error @+1 {{'test.op_with_property_predicates' op property 'non_empty_constrained' failed to satiisfy constraint: non-empty array of non-negative int64_t}} +test.op_with_property_predicates <{ + scalar = 1 : i64, + optional = [2 : i64], + defaulted = 3 : i64, + more_constrained = 4 : i64, + array = [], + non_empty_unconstrained = [5: i64], + non_empty_constrained = [], + non_empty_optional = [[7 : i64]], + unconstrained = 8 : i64}> + +// ----- + +// expected-error @+1 {{'test.op_with_property_predicates' op property 'non_empty_constrained' failed to satiisfy constraint: non-empty array of non-negative int64_t}} +test.op_with_property_predicates <{ + scalar = 1 : i64, + optional = [2 : i64], + defaulted = 3 : i64, + more_constrained = 4 : i64, + array = [], + non_empty_unconstrained = [5: i64], + non_empty_constrained = [-1 : i64], + non_empty_optional = [[7 : i64]], + unconstrained = 8 : i64}> + +// ----- + +// expected-error @+1 {{'test.op_with_property_predicates' op property 'non_empty_optional' failed to satiisfy constraint: optional non-empty array of non-negative int64_t}} +test.op_with_property_predicates <{ + scalar = 1 : i64, + optional = [2 : i64], + defaulted = 3 : i64, + more_constrained = 4 : i64, + array = [], + non_empty_unconstrained = [5: i64], + non_empty_constrained = [6 : i64], + non_empty_optional = [[]], + unconstrained = 8 : i64}> diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td index d24d52f356d88f..384139d9c32c02 100644 --- a/mlir/test/lib/Dialect/Test/TestOps.td +++ b/mlir/test/lib/Dialect/Test/TestOps.td @@ -2998,7 +2998,7 @@ def TestOpWithProperties : TEST_Op<"with_properties"> { StrAttr:$b, // Attributes can directly be used here. StringProperty:$c, BoolProperty:$flag, - IntArrayProperty<"int64_t">:$array // example of an array + IntArrayProperty<I64Property>:$array // example of an array ); } @@ -3040,15 +3040,15 @@ def TestOpWithEmptyProperties : TEST_Op<"empty_properties"> { def TestOpUsingPropertyInCustom : TEST_Op<"using_property_in_custom"> { let assemblyFormat = "custom<UsingPropertyInCustom>($prop) attr-dict"; - let arguments = (ins IntArrayProperty<"int64_t">:$prop); + let arguments = (ins IntArrayProperty<I64Property>:$prop); } def TestOpUsingPropertyInCustomAndOther : TEST_Op<"using_property_in_custom_and_other"> { let assemblyFormat = "custom<UsingPropertyInCustom>($prop) prop-dict attr-dict"; let arguments = (ins - IntArrayProperty<"int64_t">:$prop, - IntProperty<"int64_t">:$other + IntArrayProperty<I64Property>:$prop, + I64Property:$other ); } @@ -3253,6 +3253,30 @@ def TestOpWithArrayProperties : TEST_Op<"with_array_properties"> { ); } +def NonNegativeI64Property : ConfinedProperty<I64Property, + CPred<"$_self >= 0">, "non-negative int64_t">; + +class NonEmptyArray<Property p> : ConfinedProperty + <ArrayProperty<p>, Neg<CPred<"$_self.empty()">>, + "non-empty array of " # p.summary>; + +def OpWithPropertyPredicates : TEST_Op<"op_with_property_predicates"> { + let arguments = (ins + NonNegativeI64Property:$scalar, + OptionalProperty<NonNegativeI64Property>:$optional, + DefaultValuedProperty<NonNegativeI64Property, "0">:$defaulted, + ConfinedProperty<NonNegativeI64Property, + CPred<"$_self <= 5">, "between 0 and 5">:$more_constrained, + ArrayProperty<NonNegativeI64Property>:$array, + NonEmptyArray<I64Property>:$non_empty_unconstrained, + NonEmptyArray<NonNegativeI64Property>:$non_empty_constrained, + // Test applying predicates when the fromStorage() on the optional<> isn't trivial. + OptionalProperty<NonEmptyArray<NonNegativeI64Property>>:$non_empty_optional, + I64Property:$unconstrained + ); + let assemblyFormat = "attr-dict prop-dict"; +} + //===----------------------------------------------------------------------===// // Test Dataflow //===----------------------------------------------------------------------===// diff --git a/mlir/test/mlir-tblgen/op-properties-predicates.td b/mlir/test/mlir-tblgen/op-properties-predicates.td new file mode 100644 index 00000000000000..0a036e9f0cae51 --- /dev/null +++ b/mlir/test/mlir-tblgen/op-properties-predicates.td @@ -0,0 +1,72 @@ +// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s + +include "mlir/IR/Constraints.td" +include "mlir/IR/EnumAttr.td" +include "mlir/IR/OpBase.td" +include "mlir/IR/Properties.td" + +def Test_Dialect : Dialect { + let name = "test"; + let cppNamespace = "foobar"; +} +class NS_Op<string mnemonic, list<Trait> traits = []> : + Op<Test_Dialect, mnemonic, traits>; + +def NonNegativeI64Property : ConfinedProperty<I64Property, + CPred<"$_self >= 0">, "non-negative int64_t">; + +class NonEmptyArray<Property p> : ConfinedProperty + <ArrayProperty<p>, Neg<CPred<"$_self.empty()">>, + "non-empty array of " # p.summary>; + +def OpWithPredicates : NS_Op<"op_with_predicates"> { + let arguments = (ins + NonNegativeI64Property:$scalar, + OptionalProperty<NonNegativeI64Property>:$optional, + DefaultValuedProperty<NonNegativeI64Property, "0">:$defaulted, + ConfinedProperty<NonNegativeI64Property, + CPred<"$_self <= 5">, "between 0 and 5">:$moreConstrained, + ArrayProperty<NonNegativeI64Property>:$array, + NonEmptyArray<I64Property>:$non_empty_unconstrained, + NonEmptyArray<NonNegativeI64Property>:$non_empty_constrained,... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/120176 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
