[Mlir-commits] [mlir] 8d6b241 - [mlir] Make `TypedStrAttr` actually enforce the string type. (#124770)

[llvmlistbot at llvm.org]

Author: Ingo Müller
Date: 2025-01-29T13:32:36+01:00
New Revision: 8d6b24167b7bdc7ac9c969abe73be857bbcf2b5a

URL: https://github.com/llvm/llvm-project/commit/8d6b24167b7bdc7ac9c969abe73be857bbcf2b5a
DIFF: https://github.com/llvm/llvm-project/commit/8d6b24167b7bdc7ac9c969abe73be857bbcf2b5a.diff

LOG: [mlir] Make `TypedStrAttr` actually enforce the string type. (#124770)

The tablgen definition `TypedStrAttr` is an attribute constraints that
is meant to restrict the type of a `StringAttr` to the type given as
parameter. However, the definition did not previously restrict the type;
any `StringAttr` was accepted. This PR makes the definition actually
enforce the type.

To test the constraints, the PR also changes the test op that was
previously used to test this constraint such that the enforced type is
`AnyInteger` instead of `AnyType`. The latter allowed any type, so not
enforcing that constraint had no observable effect. The PR then adds a
test case with a wrong type and ensures that diagnostics are produced.

Signed-off-by: Ingo Müller <ingomueller at google.com>

Added: 
    

Modified: 
    mlir/include/mlir/IR/CommonAttrConstraints.td
    mlir/test/IR/attribute.mlir
    mlir/test/lib/Dialect/Test/TestOps.td

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/CommonAttrConstraints.td b/mlir/include/mlir/IR/CommonAttrConstraints.td
index 17ca82c510f8a2..599f5ecba5803b 100644
--- a/mlir/include/mlir/IR/CommonAttrConstraints.td
+++ b/mlir/include/mlir/IR/CommonAttrConstraints.td
@@ -334,17 +334,19 @@ class StringBasedAttr<Pred condition, string descr> : Attr<condition, descr> {
   let valueType = NoneType;
 }
 
-def StrAttr : StringBasedAttr<CPred<"::llvm::isa<::mlir::StringAttr>($_self)">,
-                              "string attribute">;
+def StrAttrPred : CPred<"::llvm::isa<::mlir::StringAttr>($_self)">;
+
+def StrAttr : StringBasedAttr<StrAttrPred, "string attribute">;
 
 // A string attribute that represents the name of a symbol.
-def SymbolNameAttr : StringBasedAttr<CPred<"::llvm::isa<::mlir::StringAttr>($_self)">,
-                                     "string attribute">;
+def SymbolNameAttr : StringBasedAttr<StrAttrPred, "string attribute">;
 
 // String attribute that has a specific value type.
 class TypedStrAttr<Type ty>
-    : StringBasedAttr<CPred<"::llvm::isa<::mlir::StringAttr>($_self)">,
-                            "string attribute"> {
+    : StringBasedAttr<And<[StrAttrPred,
+        SubstLeaves<"$_self", "::mlir::cast<StringAttr>($_self).getType()",
+                    ty.predicate>]>,
+        "string attribute of " # ty.summary> {
   let valueType = ty;
 }
 

diff  --git a/mlir/test/IR/attribute.mlir b/mlir/test/IR/attribute.mlir
index 0085d64ae82b6b..5a005a393d8ac2 100644
--- a/mlir/test/IR/attribute.mlir
+++ b/mlir/test/IR/attribute.mlir
@@ -416,10 +416,29 @@ func.func @non_type_in_type_array_attr_fail() {
 // Test StringAttr with custom type
 //===----------------------------------------------------------------------===//
 
-// CHECK-LABEL: func @string_attr_custom_type
-func.func @string_attr_custom_type() {
-  // CHECK: "string_data" : !foo.string
-  test.string_attr_with_type "string_data" : !foo.string
+// CHECK-LABEL: func @string_attr_custom_type_valid
+func.func @string_attr_custom_type_valid() {
+  // CHECK: "string_data" : i64
+  test.string_attr_with_type "string_data" : i64
+  return
+}
+
+// -----
+
+func.func @string_attr_custom_type_invalid() {
+  // expected-error @+1 {{'attr' failed to satisfy constraint: string attribute of integer}}
+  test.string_attr_with_type "string_data" : f32
+  return
+}
+
+// -----
+
+// CHECK-LABEL: func @string_attr_custom_mixed_type
+func.func @string_attr_custom_mixed_type() {
+  // CHECK: "string_data" : i64
+  test.string_attr_with_mixed_type "string_data" : i64
+  // CHECK: 42 : i64
+  test.string_attr_with_mixed_type 42 : i64
   return
 }
 

diff  --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index f37573c1351cec..79840094686e19 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -193,8 +193,15 @@ def TypeArrayAttrWithDefaultOp : TEST_Op<"type_array_attr_with_default"> {
   let arguments = (ins DefaultValuedAttr<TypeArrayAttr, "{}">:$attr);
 }
 
-def TypeStringAttrWithTypeOp : TEST_Op<"string_attr_with_type"> {
-  let arguments = (ins TypedStrAttr<AnyType>:$attr);
+def TypedStringAttrWithTypeOp : TEST_Op<"string_attr_with_type"> {
+  let arguments = (ins TypedStrAttr<AnyInteger>:$attr);
+  let assemblyFormat = "$attr attr-dict";
+}
+
+def TypedStringAttrWithMixedTypeOp : TEST_Op<"string_attr_with_mixed_type"> {
+  let arguments = (ins
+    AnyAttrOf<[TypedStrAttr<AnyInteger>, I64Attr]>:$attr
+  );
   let assemblyFormat = "$attr attr-dict";
 }