[mlir] Unique property constraints where possible

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.

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;

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()) {

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),

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();