llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-mlir Author: Krzysztof Drewniak (krzysz00) <details> <summary>Changes</summary> Now that `Property` is a `PropConstraint`, hook it up to the same constraint-uniquing machinery that other types of constraints use. This will primarily save on code size for types, like enums, that have inherent constraints which are shared across many operations. --- Patch is 23.43 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/140849.diff 6 Files Affected: - (modified) mlir/include/mlir/TableGen/CodeGenHelpers.h (+21) - (modified) mlir/include/mlir/TableGen/Property.h (+15-9) - (modified) mlir/lib/TableGen/CodeGenHelpers.cpp (+64) - (modified) mlir/lib/TableGen/Property.cpp (+11-7) - (modified) mlir/test/mlir-tblgen/op-properties-predicates.td (+45-16) - (modified) mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp (+45-29) ``````````diff diff --git a/mlir/include/mlir/TableGen/CodeGenHelpers.h b/mlir/include/mlir/TableGen/CodeGenHelpers.h index 465240907a3de..cf14f65b93ed2 100644 --- a/mlir/include/mlir/TableGen/CodeGenHelpers.h +++ b/mlir/include/mlir/TableGen/CodeGenHelpers.h @@ -153,6 +153,23 @@ class StaticVerifierFunctionEmitter { std::optional<StringRef> getAttrConstraintFn(const Constraint &constraint) const; + /// Get the name of the static function used for the given property + /// constraint. These functions are in the form: + /// + /// LogicalResult(Operation *op, T property, StringRef propName); + /// + /// where T is the interface type specified in the constraint. + /// If a uniqued constraint was not found, this function returns std::nullopt. + /// The uniqued constraints cannot be used in the context of an OpAdaptor. + /// + /// Pattern constraints have the form: + /// + /// LogicalResult(PatternRewriter &rewriter, Operation *op, T property, + /// StringRef failureStr); + /// + std::optional<StringRef> + getPropConstraintFn(const Constraint &constraint) const; + /// Get the name of the static function used for the given successor /// constraint. These functions are in the form: /// @@ -175,6 +192,8 @@ class StaticVerifierFunctionEmitter { void emitTypeConstraints(); /// Emit static attribute constraint functions. void emitAttrConstraints(); + /// Emit static property constraint functions. + void emitPropConstraints(); /// Emit static successor constraint functions. void emitSuccessorConstraints(); /// Emit static region constraint functions. @@ -212,6 +231,8 @@ class StaticVerifierFunctionEmitter { ConstraintMap typeConstraints; /// The set of attribute constraints used in the current file. ConstraintMap attrConstraints; + /// The set of property constraints used in the current file. + ConstraintMap propConstraints; /// The set of successor constraints used in the current file. ConstraintMap successorConstraints; /// The set of region constraints used in the current file. diff --git a/mlir/include/mlir/TableGen/Property.h b/mlir/include/mlir/TableGen/Property.h index 386159191ef1f..6af96f077efe5 100644 --- a/mlir/include/mlir/TableGen/Property.h +++ b/mlir/include/mlir/TableGen/Property.h @@ -29,14 +29,26 @@ class Dialect; class Type; class Pred; +// Wrapper class providing helper methods for accesing property constraint +// values. +class PropConstraint : public Constraint { + using Constraint::Constraint; + +public: + static bool classof(const Constraint *c) { return c->getKind() == CK_Prop; } + + StringRef getInterfaceType() const; +}; + // Wrapper class providing helper methods for accessing MLIR Property defined // in TableGen. This class should closely reflect what is defined as class // `Property` in TableGen. -class Property { +class Property : public PropConstraint { public: - explicit Property(const llvm::Record *record); + explicit Property(const llvm::Record *def); explicit Property(const llvm::DefInit *init); - Property(StringRef summary, StringRef description, StringRef storageType, + Property(const llvm::Record *maybeDef, StringRef summary, + StringRef description, StringRef storageType, StringRef interfaceType, StringRef convertFromStorageCall, StringRef assignToStorageCall, StringRef convertToAttributeCall, StringRef convertFromAttributeCall, StringRef parserCall, @@ -131,13 +143,7 @@ class Property { // property constraints, this function is added for future-proofing) Property getBaseProperty() const; - // Returns the TableGen definition this Property was constructed from. - const llvm::Record &getDef() const { return *def; } - private: - // The TableGen definition of this constraint. - const llvm::Record *def; - // Elements describing a Property, in general fetched from the record. StringRef summary; StringRef description; diff --git a/mlir/lib/TableGen/CodeGenHelpers.cpp b/mlir/lib/TableGen/CodeGenHelpers.cpp index f4031be24dfdb..4ce6ab1dbfce5 100644 --- a/mlir/lib/TableGen/CodeGenHelpers.cpp +++ b/mlir/lib/TableGen/CodeGenHelpers.cpp @@ -53,6 +53,7 @@ void StaticVerifierFunctionEmitter::emitOpConstraints( NamespaceEmitter namespaceEmitter(os, Operator(*opDefs[0]).getCppNamespace()); emitTypeConstraints(); emitAttrConstraints(); + emitPropConstraints(); emitSuccessorConstraints(); emitRegionConstraints(); } @@ -83,6 +84,15 @@ std::optional<StringRef> StaticVerifierFunctionEmitter::getAttrConstraintFn( : StringRef(it->second); } +// Find a uniqued property constraint. Since not all property constraints can +// be uniqued, return std::nullopt if one was not found. +std::optional<StringRef> StaticVerifierFunctionEmitter::getPropConstraintFn( + const Constraint &constraint) const { + const auto *it = propConstraints.find(constraint); + return it == propConstraints.end() ? std::optional<StringRef>() + : StringRef(it->second); +} + StringRef StaticVerifierFunctionEmitter::getSuccessorConstraintFn( const Constraint &constraint) const { const auto *it = successorConstraints.find(constraint); @@ -146,6 +156,25 @@ static ::llvm::LogicalResult {0}( } )"; +/// Code for a property constraint. These may be called from ops only. +/// Property constraints cannot reference anything other than `$_self` and +/// `$_op`. {3} is the interface type of the property. +static const char *const propConstraintCode = R"( + static ::llvm::LogicalResult {0}( + {3} prop, ::llvm::StringRef propName, llvm::function_ref<::mlir::InFlightDiagnostic()> emitError) {{ + if (!({1})) + return emitError() << "property '" << propName + << "' failed to satisfy constraint: {2}"; + return ::mlir::success(); + } + static ::llvm::LogicalResult {0}( + ::mlir::Operation *op, {3} prop, ::llvm::StringRef propName) {{ + return {0}(prop, propName, [op]() {{ + return op->emitOpError(); + }); + } + )"; + /// Code for a successor constraint. static const char *const successorConstraintCode = R"( static ::llvm::LogicalResult {0}( @@ -210,6 +239,20 @@ void StaticVerifierFunctionEmitter::emitAttrConstraints() { emitConstraints(attrConstraints, "attr", attrConstraintCode); } +/// Unlike with the other helpers, this one has to substitute in the interface +/// type of the property, so we can't just use the generic function. +void StaticVerifierFunctionEmitter::emitPropConstraints() { + FmtContext ctx; + ctx.addSubst("_op", "*op").withSelf("prop"); + for (auto &it : propConstraints) { + auto propConstraint = cast<PropConstraint>(it.first); + os << formatv(propConstraintCode, it.second, + tgfmt(propConstraint.getConditionTemplate(), &ctx), + escapeString(it.first.getSummary()), + propConstraint.getInterfaceType()); + } +} + void StaticVerifierFunctionEmitter::emitSuccessorConstraints() { emitConstraints(successorConstraints, "successor", successorConstraintCode); } @@ -251,6 +294,20 @@ static bool canUniqueAttrConstraint(Attribute attr) { return !StringRef(test).contains("<no-subst-found>"); } +/// A property constraint that references anything other than itself and the +/// current op cannot be generically extracted into a function, just as with +/// canUnequePropConstraint(). Additionally, property constraints without +/// an interface type specified can't be uniqued, and ones that are a literal +/// "true" shouldn't be constrained. +static bool canUniquePropConstraint(Property prop) { + FmtContext ctx; + auto test = tgfmt(prop.getConditionTemplate(), + &ctx.withSelf("prop").addSubst("_op", "*op")) + .str(); + return !StringRef(test).contains("<no-subst-found>") && test != "true" && + !prop.getInterfaceType().empty(); +} + std::string StaticVerifierFunctionEmitter::getUniqueName(StringRef kind, unsigned index) { return ("__mlir_ods_local_" + kind + "_constraint_" + uniqueOutputLabel + @@ -286,6 +343,13 @@ void StaticVerifierFunctionEmitter::collectOpConstraints( canUniqueAttrConstraint(namedAttr.attr)) collectConstraint(attrConstraints, "attr", namedAttr.attr); } + /// Collect non-trivial property constraints. + for (const NamedProperty &namedProp : op.getProperties()) { + if (!namedProp.prop.getPredicate().isNull() && + canUniquePropConstraint(namedProp.prop)) { + collectConstraint(propConstraints, "prop", namedProp.prop); + } + } /// Collect successor constraints. for (const NamedSuccessor &successor : op.getSuccessors()) { if (!successor.constraint.getPredicate().isNull()) { diff --git a/mlir/lib/TableGen/Property.cpp b/mlir/lib/TableGen/Property.cpp index 13851167ddaf7..9a70c1b6e8d62 100644 --- a/mlir/lib/TableGen/Property.cpp +++ b/mlir/lib/TableGen/Property.cpp @@ -33,9 +33,13 @@ static StringRef getValueAsString(const Init *init) { return {}; } +StringRef PropConstraint::getInterfaceType() const { + return getValueAsString(def->getValueInit("interfaceType")); +} + Property::Property(const Record *def) : Property( - getValueAsString(def->getValueInit("summary")), + def, getValueAsString(def->getValueInit("summary")), getValueAsString(def->getValueInit("description")), getValueAsString(def->getValueInit("storageType")), getValueAsString(def->getValueInit("interfaceType")), @@ -51,16 +55,15 @@ Property::Property(const Record *def) getValueAsString(def->getValueInit("hashProperty")), getValueAsString(def->getValueInit("defaultValue")), getValueAsString(def->getValueInit("storageTypeValueOverride"))) { - this->def = def; assert((def->isSubClassOf("Property") || def->isSubClassOf("Attr")) && "must be subclass of TableGen 'Property' class"); } Property::Property(const DefInit *init) : Property(init->getDef()) {} -Property::Property(StringRef summary, StringRef description, - StringRef storageType, StringRef interfaceType, - StringRef convertFromStorageCall, +Property::Property(const llvm::Record *maybeDef, StringRef summary, + StringRef description, StringRef storageType, + StringRef interfaceType, StringRef convertFromStorageCall, StringRef assignToStorageCall, StringRef convertToAttributeCall, StringRef convertFromAttributeCall, StringRef parserCall, @@ -69,8 +72,9 @@ Property::Property(StringRef summary, StringRef description, StringRef writeToMlirBytecodeCall, StringRef hashPropertyCall, StringRef defaultValue, StringRef storageTypeValueOverride) - : def(nullptr), summary(summary), description(description), - storageType(storageType), interfaceType(interfaceType), + : PropConstraint(maybeDef, Constraint::CK_Prop), summary(summary), + description(description), storageType(storageType), + interfaceType(interfaceType), convertFromStorageCall(convertFromStorageCall), assignToStorageCall(assignToStorageCall), convertToAttributeCall(convertToAttributeCall), diff --git a/mlir/test/mlir-tblgen/op-properties-predicates.td b/mlir/test/mlir-tblgen/op-properties-predicates.td index 9834edd0cbb57..7cd24aad97424 100644 --- a/mlir/test/mlir-tblgen/op-properties-predicates.td +++ b/mlir/test/mlir-tblgen/op-properties-predicates.td @@ -35,39 +35,68 @@ def OpWithPredicates : NS_Op<"op_with_predicates"> { ); } +// CHECK-LABEL: static ::llvm::LogicalResult __mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates1 +// CHECK-NEXT: int64_t prop +// CHECK-NEXT: if (!((prop >= 0))) +// CHECK: failed to satisfy constraint: non-negative int64_t + +// CHECK-LABEL: static ::llvm::LogicalResult __mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates2 +// CHECK-NEXT: std::optional<int64_t> prop +// CHECK-NEXT: if (!(((!prop.has_value())) || (((*(prop)) >= 0)))) +// CHECK: failed to satisfy constraint: optional non-negative int64_t + +// CHECK-LABEL: static ::llvm::LogicalResult __mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates3 +// CHECK-NEXT: int64_t prop +// CHECK-NEXT: if (!(((prop >= 0)) && ((prop <= 5)))) +// CHECK: failed to satisfy constraint: between 0 and 5 + +// CHECK-LABEL: static ::llvm::LogicalResult __mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates4 +// CHECK-NEXT: ::llvm::ArrayRef<int64_t> prop +// CHECK-NEXT: (!(::llvm::all_of(prop, [](const int64_t& baseStore) -> bool { return [](int64_t baseIface) -> bool { return ((baseIface >= 0)); }(baseStore); }))) +// CHECK: failed to satisfy constraint: array of non-negative int64_t + +// CHECK-LABEL: static ::llvm::LogicalResult __mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates5 +// CHECK-NEXT: ::llvm::ArrayRef<int64_t> prop +// CHECK-NEXT: if (!(!((prop.empty())))) +// CHECK: failed to satisfy constraint: non-empty array of int64_t + +// CHECK-LABEL: static ::llvm::LogicalResult __mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates6 +// CHECK-NEXT: ::llvm::ArrayRef<int64_t> prop +// CHECX-NEXT: if (!((::llvm::all_of(prop, [](const int64_t& baseStore) -> bool { return [](int64_t baseIface) -> bool { return ((baseIface >= 0)); }(baseStore); })) && (!((prop.empty()))))) +// CHECK: failed to satisfy constraint: non-empty array of non-negative int64_t + +// CHECK-LABEL: static ::llvm::LogicalResult __mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates7 +// CHECK-NEXT: std::optional<::llvm::ArrayRef<int64_t>> prop +// CHECK-NEXT: if (!(((!prop.has_value())) || ((::llvm::all_of((*(prop)), [](const int64_t& baseStore) -> bool { return [](int64_t baseIface) -> bool { return ((baseIface >= 0)); }(baseStore); })) && (!(((*(prop)).empty())))))) +// CHECK: failed to satisfy constraint: optional non-empty array of non-negative int64_ + // CHECK-LABEL: OpWithPredicates::verifyInvariantsImpl() // Note: for test readability, we capture [[maybe_unused]] into the variable maybe_unused // CHECK: [[maybe_unused:\[\[maybe_unused\]\]]] int64_t tblgen_scalar = this->getScalar(); -// CHECK: if (!((tblgen_scalar >= 0))) -// CHECK-NEXT: return emitOpError("property 'scalar' failed to satisfy constraint: non-negative int64_t"); +// CHECK: if (::mlir::failed(__mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates1(*this, tblgen_scalar, "scalar"))) +// CHECK-NEXT: return ::mlir::failure() // CHECK: [[maybe_unused]] std::optional<int64_t> tblgen_optional = this->getOptional(); -// CHECK: if (!(((!tblgen_optional.has_value())) || (((*(tblgen_optional)) >= 0)))) -// CHECK-NEXT: return emitOpError("property 'optional' failed to satisfy constraint: optional non-negative int64_t"); +// CHECK: if (::mlir::failed(__mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates2(*this, tblgen_optional, "optional"))) +// COM: The predicates for "scalar" and "defaulted" are uniqued // CHECK: [[maybe_unused]] int64_t tblgen_defaulted = this->getDefaulted(); -// CHECK: if (!((tblgen_defaulted >= 0))) -// CHECK-NEXT: return emitOpError("property 'defaulted' failed to satisfy constraint: non-negative int64_t"); +// CHECK: if (::mlir::failed(__mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates1(*this, tblgen_defaulted, "defaulted"))) // CHECK: [[maybe_unused]] int64_t tblgen_moreConstrained = this->getMoreConstrained(); -// CHECK: if (!(((tblgen_moreConstrained >= 0)) && ((tblgen_moreConstrained <= 5)))) -// CHECK-NEXT: return emitOpError("property 'moreConstrained' failed to satisfy constraint: between 0 and 5"); +// CHECK: if (::mlir::failed(__mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates3(*this, tblgen_moreConstrained, "moreConstrained"))) // CHECK: [[maybe_unused]] ::llvm::ArrayRef<int64_t> tblgen_array = this->getArray(); -// CHECK: if (!(::llvm::all_of(tblgen_array, [](const int64_t& baseStore) -> bool { return [](int64_t baseIface) -> bool { return ((baseIface >= 0)); }(baseStore); }))) -// CHECK-NEXT: return emitOpError("property 'array' failed to satisfy constraint: array of non-negative int64_t"); +// CHECK: if (::mlir::failed(__mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates4(*this, tblgen_array, "array"))) // CHECK: [[maybe_unused]] ::llvm::ArrayRef<int64_t> tblgen_nonEmptyUnconstrained = this->getNonEmptyUnconstrained(); -// CHECK: if (!(!((tblgen_nonEmptyUnconstrained.empty())))) -// CHECK-NEXT: return emitOpError("property 'non_empty_unconstrained' failed to satisfy constraint: non-empty array of int64_t"); +// CHECK: if (::mlir::failed(__mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates5(*this, tblgen_nonEmptyUnconstrained, "non_empty_unconstrained"))) // CHECK: [[maybe_unused]] ::llvm::ArrayRef<int64_t> tblgen_nonEmptyConstrained = this->getNonEmptyConstrained(); -// CHECK: if (!((::llvm::all_of(tblgen_nonEmptyConstrained, [](const int64_t& baseStore) -> bool { return [](int64_t baseIface) -> bool { return ((baseIface >= 0)); }(baseStore); })) && (!((tblgen_nonEmptyConstrained.empty()))))) -// CHECK-NEXT: return emitOpError("property 'non_empty_constrained' failed to satisfy constraint: non-empty array of non-negative int64_t"); +// CHECK: if (::mlir::failed(__mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates6(*this, tblgen_nonEmptyConstrained, "non_empty_constrained"))) // CHECK: [[maybe_unused]] std::optional<::llvm::ArrayRef<int64_t>> tblgen_nonEmptyOptional = this->getNonEmptyOptional(); -// CHECK: (!(((!tblgen_nonEmptyOptional.has_value())) || ((::llvm::all_of((*(tblgen_nonEmptyOptional)), [](const int64_t& baseStore) -> bool { return [](int64_t baseIface) -> bool { return ((baseIface >= 0)); }(baseStore); })) && (!(((*(tblgen_nonEmptyOptional)).empty())))))) -// CHECK-NEXT: return emitOpError("property 'non_empty_optional' failed to satisfy constraint: optional non-empty array of non-negative int64_t"); +// CHECK: if (::mlir::failed(__mlir_ods_local_prop_constraint_op2Dproperties2Dpredicates7(*this, tblgen_nonEmptyOptional, "non_empty_optional"))) // CHECK-NOT: int64_t tblgen_unconstrained // CHECK: return ::mlir::success(); diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp index 373d3762cbb1a..c15f478ad9c07 100644 --- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp +++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp @@ -491,28 +491,28 @@ void OpOrAdaptorHelper::computeAttrMetadata() { } auto makeProperty = [&](StringRef storageType, StringRef parserCall) { - return Property( - /*summary=*/"", - /*description=*/"", - /*storageType=*/storageType, - /*interfaceType=*/"::llvm::ArrayRef<int32_t>", - /*convertFromStorageCall=*/"$_storage", - /*assignToStorageCall=*/ - "::llvm::copy($_value, $_storage.begin())", - /*convertToAttributeCall=*/ - "return ::mlir::DenseI32ArrayAttr::get($_ctxt, $_storage);", - /*convertFromAttributeCall=*/ - "return convertFromAttribute($_storage, $_attr, $_diag);", - /*parserCall=*/parserCall, - /*optionalParserCall=*/"", - /*printerCall=*/printTextualSegmentSize, - /*readFromMlirBytecodeCall=*/readBytecodeSegmentSizeNative, - /*writeToMlirBytecodeCall=*/writeBytecodeSegmentSizeNative, - /*hashPropertyCall=*/ - "::llvm::hash_combine_range(std::begin($_storage), " - "std::end($_storage));", - /*StringRef defaultValue=*/"", - /*storageTypeValueOverride=*/""); + return Property(/*maybeDef=*/nullptr, + /*summary=*/"", + /*description=*/"", + /*storageType=*/storageType, + /*interfaceType=*/"::llvm::ArrayRef<int32_t>", + /*convertFromStorageCall=*/"$_storage", + /*assignToStorageCall=*/ + "::llvm::copy($_value, $_storage.begin())", + /*convertToAttributeCall=*/ + "return ::mlir::DenseI32ArrayAttr::get($_ctxt, $_storage);", + /*convertFromAttributeCall=*/ + "return convertFromAttribute($_storage, $_attr, $_diag);", + ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/140849 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
