Skip to content

[mlir][NFC] Make Property a subclass of PropConstraint #140848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

krzysz00
Copy link
Contributor

In preparation for allowing non-attribute properties in the declaritive rewrite pattern system, make Property a subclass of PropConstraint in tablegen and add a CK_Prop to the Constraint class for tablegen.

Like TypeConstraint but unlike other constraints, a PropConstraint has an additional field - the C++ interface type of the property being constraint (if it's known).

In preparation for allowing non-attribute properties in the declaritive
rewrite pattern system, make `Property` a subclass of `PropConstraint`
in tablegen and add a CK_Prop to the Constraint class for tablegen.

Like `TypeConstraint` but unlike other constraints, a `PropConstraint`
has an additional field - the C++ interface type of the property being
constraint (if it's known).
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir mlir:ods labels May 21, 2025
@llvmbot
Copy link
Member

llvmbot commented May 21, 2025

@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-mlir-ods

@llvm/pr-subscribers-mlir-core

Author: Krzysztof Drewniak (krzysz00)

Changes

In preparation for allowing non-attribute properties in the declaritive rewrite pattern system, make Property a subclass of PropConstraint in tablegen and add a CK_Prop to the Constraint class for tablegen.

Like TypeConstraint but unlike other constraints, a PropConstraint has an additional field - the C++ interface type of the property being constraint (if it's known).


Full diff: https://github.com/llvm/llvm-project/pull/140848.diff

6 Files Affected:

  • (modified) mlir/docs/DeclarativeRewrites.md (+2-2)
  • (modified) mlir/docs/DefiningDialects/Operations.md (+3-2)
  • (modified) mlir/include/mlir/IR/Constraints.td (+18-3)
  • (modified) mlir/include/mlir/IR/Properties.td (+20-20)
  • (modified) mlir/include/mlir/TableGen/Constraint.h (+8-1)
  • (modified) mlir/lib/TableGen/Constraint.cpp (+2)
diff --git a/mlir/docs/DeclarativeRewrites.md b/mlir/docs/DeclarativeRewrites.md
index 10cbd6d60e552..efc1f044e1f1f 100644
--- a/mlir/docs/DeclarativeRewrites.md
+++ b/mlir/docs/DeclarativeRewrites.md
@@ -109,8 +109,8 @@ The source pattern is for matching a DAG of operations. Arguments in the `dag`
 object are intended to **capture** the op arguments. They can also be used to
 **further limit** the match criteria. The capturing is done by specifying a
 symbol starting with the `$` sign, while further constraints are introduced by
-specifying a `TypeConstraint` (for an operand) or a `AttrConstraint` (for an
-attribute).
+specifying a `TypeConstraint` (for an operand), an `AttrConstraint` (for an
+attribute, or a `PropConstraint` for a property).
 
 #### Binding op arguments and limiting the match
 
diff --git a/mlir/docs/DefiningDialects/Operations.md b/mlir/docs/DefiningDialects/Operations.md
index 986ae292d64c2..b3bde055f04f0 100644
--- a/mlir/docs/DefiningDialects/Operations.md
+++ b/mlir/docs/DefiningDialects/Operations.md
@@ -104,7 +104,8 @@ their semantics via a special [TableGen backend][TableGenBackend]:
 *   The `Property` class hierarchy: They are used to specify non-attribute-backed
     properties that are inherent to operations. These properties can have
     constraints imposed on them using the `predicate` field or the
-    `ConfinedProp` class.
+    `ConfinedProp` class. The `PropConstraint` superclass of `Property` is used
+    to describe constraints on properties in rewrite patterns.
 
 An operation is defined by specializing the `Op` class with concrete contents
 for all the fields it requires. For example, `tf.AvgPool` is defined as
@@ -213,7 +214,7 @@ hierarchy. Similarly, `<attr-constraint>` is a TableGen `def` from the
 of `Property` (constraints can be imposed onto it using its `predicate` field
 or the `ConfinedProp` subclass).
 
-There is no requirements on the relative order of operands and attributes; they
+There are no requirements on the relative order of operands and attributes; they
 can mix freely. The relative order of operands themselves matters. From each
 named argument a named getter will be generated that returns the argument with
 the return type (in the case of attributes the return type will be constructed
diff --git a/mlir/include/mlir/IR/Constraints.td b/mlir/include/mlir/IR/Constraints.td
index 62bc84c387063..33e8581ecd356 100644
--- a/mlir/include/mlir/IR/Constraints.td
+++ b/mlir/include/mlir/IR/Constraints.td
@@ -169,6 +169,16 @@ class TypeConstraint<Pred predicate, string summary = "",
 class AttrConstraint<Pred predicate, string summary = ""> :
     Constraint<predicate, summary>;
 
+// Subclass for constraints on a property.
+class PropConstraint<Pred predicate, string summary = "", string interfaceTypeParam = ""> :
+    Constraint<predicate, summary> {
+  // The name of the C++ type of $_self, which will be the interface type of the
+  // property being constrained, or "" if it is to be inferred from context.
+  // Note, that leaving the interface type unspecified prevents the constraint from
+  // being uniqued.
+  code interfaceType = interfaceTypeParam;
+}
+
 // Subclass for constraints on a region.
 class RegionConstraint<Pred predicate, string summary = ""> :
     Constraint<predicate, summary>;
@@ -183,10 +193,15 @@ class SuccessorConstraint<Pred predicate, string summary = ""> :
 //   * Constraints on an op's operand/result definition
 //   * Further constraints to match an op's operand/result in source pattern
 //
-// * Use Attr (a subclass for AttrConstraint) for
-//   * Constraints on an op's attribute definition
+// * Use Attr (a subclass of AttrConstraint) for
+//   * Constraints on an op's attribute definitions
 // * Use AttrConstraint to specify
-//   * Further constraints to match an op's attribute in source pattern
+//   * Further constraints to match an op's attribute in rewrite source patterns.
+//
+// * Use Prop (a subclass of PropConstraint) for
+//   * Defining an op's properties or constraining them
+// * Use Prop constraint to specify
+//   * Further constraints to match an op's property in rewrite source patterns.
 //
 // * Use uncategorized constraint to specify
 //   * Multi-entity constraints in rewrite rules
diff --git a/mlir/include/mlir/IR/Properties.td b/mlir/include/mlir/IR/Properties.td
index 15b72969dc92f..644f94a364578 100644
--- a/mlir/include/mlir/IR/Properties.td
+++ b/mlir/include/mlir/IR/Properties.td
@@ -17,14 +17,21 @@ include "mlir/IR/Constraints.td"
 include "mlir/IR/Utils.td"
 
 // Base class for defining properties.
-class Property<string storageTypeParam = "", string desc = ""> {
-  // User-readable one line summary used in error reporting messages. If empty,
-  // a generic message will be used.
-  string summary = desc;
+// `desc` is the summary, a user-readable one-line description of the property
+// used in error messages. If empty, a generic message will be used.
+// `storageTypeParam` is the type which will be used to store the property in
+// a property struct, while `interfaceTypeParam` (which defaults to `storageTypeParam`)
+// is the type returned from the getter of the property and used as an argument
+// to the property's builder.
+// Properties have a `predicate` field. It defaults to the true predicate since
+// properties always hold their C++ type. Within these predicates, $_self is the
+// **interface** type of the property.
+class Property<string storageTypeParam = "", string desc = "", string interfaceTypeParam = storageTypeParam>
+    : PropConstraint<TruePred, desc, interfaceTypeParam> {
   // The full description of this property.
   string description = "";
   code storageType = storageTypeParam;
-  code interfaceType = storageTypeParam;
+  // Note: the interface type is a field on PropConstraint.
 
   // The expression to convert from the storage type to the Interface
   // type. For example, an enum can be stored as an int but returned as an
@@ -66,13 +73,6 @@ class Property<string storageTypeParam = "", string desc = ""> {
     return convertFromAttribute($_storage, $_attr, $_diag);
   }];
 
-  // The verification predicate for this property. Defaults to the true predicate,
-  // since properties are always their expected type.
-  // Within the predicate, `$_self` is an instance of the **interface**
-  // type of the property. Setting this field to ? will also result in a
-  // true predicate but is not recommended, as it breaks composability.
-  Pred predicate = TruePred;
-
   // The call expression to hash the property.
   //
   // Format:
@@ -240,8 +240,7 @@ def I32Property : IntProp<"int32_t">, Deprecated<"moved to shorter name I32Prop"
 def I64Property : IntProp<"int64_t">, Deprecated<"moved to shorter name I64Prop">;
 
 // Note: only a class so we can deprecate the old name
-class _cls_StringProp : Property<"std::string", "string"> {
-  let interfaceType = "::llvm::StringRef";
+class _cls_StringProp : Property<"std::string", "string", "::llvm::StringRef"> {
   let convertFromStorage = "::llvm::StringRef{$_storage}";
   let assignToStorage = "$_storage = $_value.str()";
   let optionalParser = [{
@@ -347,7 +346,8 @@ def UnitProperty : _cls_UnitProp, Deprecated<"moved to shorter name UnitProp">;
 /// 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 DefaultValuedProp<Property p, string default = "", string storageDefault = ""> : Property<p.storageType, p.summary> {
+class DefaultValuedProp<Property p, string default = "", string storageDefault = "">
+    : Property<p.storageType, p.summary, p.interfaceType> {
   let defaultValue = default;
   let storageTypeValueOverride = storageDefault;
   let baseProperty = p;
@@ -375,7 +375,7 @@ class DefaultValuedProperty<Property p, string default = "", string storageDefau
 /// predicates it may already have. If `newSummary` is provided, replace the
 /// summary of `p` with `newSummary`.
 class ConfinedProp<Property p, Pred pred, string newSummary = "">
-    : Property<p.storageType, !if(!empty(newSummary), p.summary, newSummary)> {
+    : Property<p.storageType, !if(!empty(newSummary), p.summary, newSummary), p.interfaceType> {
   let predicate = !if(!ne(p.predicate, TruePred), And<[p.predicate, pred]>, pred);
   let baseProperty = p;
   // Keep up to date with `Property` above.
@@ -441,8 +441,8 @@ class _makeStorageWrapperPred<Property wrappedProp> {
 /// subclass `ArrayProp`.
 class ArrayProp<Property elem = Property<>, string newSummary = ""> :
   Property<"::llvm::SmallVector<" # elem.storageType # ">",
-    !if(!empty(newSummary), "array of " # elem.summary, newSummary)> {
-  let interfaceType = "::llvm::ArrayRef<" # elem.storageType # ">";
+    !if(!empty(newSummary), "array of " # elem.summary, newSummary),
+    "::llvm::ArrayRef<" # elem.storageType # ">"> {
   let convertFromStorage = "::llvm::ArrayRef<" # elem.storageType # ">{$_storage}";
   let assignToStorage = "$_storage.assign($_value.begin(), $_value.end())";
 
@@ -585,7 +585,8 @@ class IntArrayProperty<Property elem, string newSummary="">
 /// syntax of the underlying property, or the keyword `none` in the rare cases that
 /// it is needed. This behavior can be disabled by setting `canDelegateParsing` to 0.
 class OptionalProp<Property p, bit canDelegateParsing = 1>
-    : Property<"std::optional<" # p.storageType # ">", "optional " # p.summary> {
+    : Property<"std::optional<" # p.storageType # ">",
+        "optional " # p.summary, "std::optional<" # p.interfaceType # ">"> {
 
   // In the cases where the underlying attribute is plain old data that's passed by
   // value, the conversion code is trivial.
@@ -596,7 +597,6 @@ class OptionalProp<Property p, bit canDelegateParsing = 1>
   defvar delegatesParsing = !and(!empty(p.defaultValue),
     !not(!empty(p.optionalParser)), canDelegateParsing);
 
-  let interfaceType = "std::optional<" # p.interfaceType # ">";
   let defaultValue = "std::nullopt";
 
   let convertFromStorage = !if(hasTrivialStorage,
diff --git a/mlir/include/mlir/TableGen/Constraint.h b/mlir/include/mlir/TableGen/Constraint.h
index 8877daaa77514..af00d2b4a7e7d 100644
--- a/mlir/include/mlir/TableGen/Constraint.h
+++ b/mlir/include/mlir/TableGen/Constraint.h
@@ -30,7 +30,14 @@ namespace tblgen {
 class Constraint {
 public:
   // Constraint kind
-  enum Kind { CK_Attr, CK_Region, CK_Successor, CK_Type, CK_Uncategorized };
+  enum Kind {
+    CK_Attr,
+    CK_Prop,
+    CK_Region,
+    CK_Successor,
+    CK_Type,
+    CK_Uncategorized
+  };
 
   // Create a constraint with a TableGen definition and a kind.
   Constraint(const llvm::Record *record, Kind kind) : def(record), kind(kind) {}
diff --git a/mlir/lib/TableGen/Constraint.cpp b/mlir/lib/TableGen/Constraint.cpp
index 8cf4ed08a2d54..92d33bff68bed 100644
--- a/mlir/lib/TableGen/Constraint.cpp
+++ b/mlir/lib/TableGen/Constraint.cpp
@@ -26,6 +26,8 @@ Constraint::Constraint(const llvm::Record *record)
     kind = CK_Type;
   } else if (def->isSubClassOf("AttrConstraint")) {
     kind = CK_Attr;
+  } else if (def->isSubClassOf("PropConstraint")) {
+    kind = CK_Prop;
   } else if (def->isSubClassOf("RegionConstraint")) {
     kind = CK_Region;
   } else if (def->isSubClassOf("SuccessorConstraint")) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir:ods mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants