[mlir:PDL] Fix a syntax ambiguity in pdl.attribute

pdl.attribute currently has a syntax ambiguity that leads to the incorrect parsing
of pdl.attribute operations with locations that don't also have a constant value. For example:

```
pdl.attribute loc("foo")
```

The above IR is treated as being a pdl.attribute with a constant value containing the location,
`loc("foo")`, which is incorrect. This commit changes the syntax to use `= <constant-value>` to
clearly distinguish when the constant value is present, as opposed to just trying to parse an attribute.

Differential Revision: https://reviews.llvm.org/D124582

diff --git a/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td b/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td
index 4e43b26..dfd8318 100644 (file)
--- a/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td
+++ b/mlir/include/mlir/Dialect/PDL/IR/PDLOps.td
@@ -118,14 +118,14 @@ def PDL_AttributeOp : PDL_Op<"attribute"> {
     %attr = pdl.attribute : %type
 
     // Define an attribute with a constant value:
-    %attr = pdl.attribute "hello"
+    %attr = pdl.attribute = "hello"
     ```
   }];
 
   let arguments = (ins Optional<PDL_Type>:$type,
                        OptionalAttr<AnyAttr>:$value);
   let results = (outs PDL_Attribute:$attr);
-  let assemblyFormat = "(`:` $type^)? ($value^)? attr-dict-with-keyword";
+  let assemblyFormat = "(`:` $type^)? (`=` $value^)? attr-dict-with-keyword";
 
   let builders = [
     OpBuilder<(ins CArg<"Value", "Value()">:$type), [{

diff --git a/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir b/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir
index 4570427..b94451c 100644 (file)
--- a/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir
+++ b/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir
@@ -49,7 +49,7 @@ module @attributes {
   // CHECK-DAG:   pdl_interp.check_type %[[ATTR1_TYPE]] is i64
   pdl.pattern : benefit(1) {
     %type = type : i64
-    %attr = attribute 10 : i64
+    %attr = attribute = 10 : i64
     %attr1 = attribute : %type
     %root = operation {"attr" = %attr, "attr1" = %attr1}
     rewrite %root with "rewriter"
@@ -583,7 +583,7 @@ module @attribute_literal {
 
   // Check the correct lowering of an attribute that hasn't been bound.
   pdl.pattern : benefit(1) {
-    %attr = attribute 10
+    %attr = attribute = 10
     pdl.apply_native_constraint "constraint"(%attr: !pdl.attribute)
 
     %root = operation

diff --git a/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-rewriter.mlir b/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-rewriter.mlir
index 4d6d524..9f3141a 100644 (file)
--- a/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-rewriter.mlir
+++ b/mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-rewriter.mlir
@@ -42,7 +42,7 @@ module @operation_attributes {
     %attr = attribute
     %root = operation "foo.op" {"attr" = %attr}
     rewrite %root {
-      %attr1 = attribute true
+      %attr1 = attribute = true
       %newOp = operation "foo.op" {"attr" = %attr, "attr1" = %attr1}
       erase %root
     }

diff --git a/mlir/test/Dialect/PDL/invalid.mlir b/mlir/test/Dialect/PDL/invalid.mlir
index a39d1c5..09d4467 100644 (file)
--- a/mlir/test/Dialect/PDL/invalid.mlir
+++ b/mlir/test/Dialect/PDL/invalid.mlir
@@ -36,7 +36,7 @@ pdl.pattern : benefit(1) {
   %type = type
 
   // expected-error@below {{expected only one of [`type`, `value`] to be set}}
-  %attr = attribute : %type 10
+  %attr = attribute : %type = 10
 
   %op = operation "foo.op" {"attr" = %attr} -> (%type : !pdl.type)
   rewrite %op with "rewriter"

diff --git a/mlir/test/Dialect/PDL/ops.mlir b/mlir/test/Dialect/PDL/ops.mlir
index 472dd25..c15f53d 100644 (file)
--- a/mlir/test/Dialect/PDL/ops.mlir
+++ b/mlir/test/Dialect/PDL/ops.mlir
@@ -1,6 +1,5 @@
 // RUN: mlir-opt -split-input-file %s | mlir-opt
-// Verify the generic form can be parsed.
-// RUN: mlir-opt -split-input-file -mlir-print-op-generic %s | mlir-opt
+// RUN: mlir-opt -split-input-file -mlir-print-op-generic -mlir-print-local-scope %s | FileCheck %s --check-prefix=CHECK-GENERIC
 
 // -----
 
@@ -138,7 +137,21 @@ pdl.pattern @apply_rewrite_with_no_results : benefit(1) {
 pdl.pattern @attribute_with_dict : benefit(1) {
   %root = operation
   rewrite %root {
-    %attr = attribute {some_unit_attr} attributes {pdl.special_attribute}
+    %attr = attribute = {some_unit_attr} attributes {pdl.special_attribute}
     apply_native_rewrite "NativeRewrite"(%attr : !pdl.attribute)
   }
 }
+
+// -----
+
+// Check that we don't treat the trailing location of a pdl.attribute as the
+// attribute value.
+
+pdl.pattern @attribute_with_loc : benefit(1) {
+  // CHECK-GENERIC: "pdl.attribute"
+  // CHECK-GENERIC-NOT: value = loc
+  %attr = attribute loc("bar")
+  
+  %root = operation {"attribute" = %attr}
+  rewrite %root with "rewriter"
+}

diff --git a/mlir/test/Integration/Dialect/PDL/CPU/multiroot.mlir b/mlir/test/Integration/Dialect/PDL/CPU/multiroot.mlir
index e5b7683..eb04ceb 100644 (file)
--- a/mlir/test/Integration/Dialect/PDL/CPU/multiroot.mlir
+++ b/mlir/test/Integration/Dialect/PDL/CPU/multiroot.mlir
@@ -15,7 +15,7 @@ module @patterns {
     %rxact = pdl.operand : %in_type
     %weight = pdl.operand : %weight_type
 
-    %attr0 = pdl.attribute false
+    %attr0 = pdl.attribute = false
     %op0 = pdl.operation "tf.MatMul" (%rxact, %weight : !pdl.value, !pdl.value) {"transpose_a" = %attr0, "transpose_b" = %attr0} -> (%out_type : !pdl.type)
 
     pdl.rewrite %op0 {
@@ -35,8 +35,8 @@ module @patterns {
     %rxdelta = pdl.operand : %out_type
     %weight = pdl.operand : %weight_type
 
-    %attr0 = pdl.attribute true
-    %attr1 = pdl.attribute false
+    %attr0 = pdl.attribute = true
+    %attr1 = pdl.attribute = false
     %op0 = pdl.operation "tf.MatMul" (%rxact, %rxdelta : !pdl.value, !pdl.value) {"transpose_a" = %attr0, "transpose_b" = %attr1} -> (%weight_type : !pdl.type)
     %val0 = pdl.result 0 of %op0
     %op1 = pdl.operation "tf.Const" -> (%const_type : !pdl.type)
@@ -156,7 +156,7 @@ module @patterns {
     %weight = pdl.operand : %weight_type
     %bias = pdl.operand : %bias_type
 
-    %attr0 = pdl.attribute false
+    %attr0 = pdl.attribute = false
     %op0 = pdl.operation "tf.MatMul" (%rxact, %weight : !pdl.value, !pdl.value) {"transpose_a" = %attr0, "transpose_b" = %attr0} -> (%out_type : !pdl.type)
     %val0 = pdl.result 0 of %op0
     %op1 = pdl.operation "tf.BiasAdd" (%val0, %bias : !pdl.value, !pdl.value) -> (%out_type : !pdl.type)
@@ -165,7 +165,7 @@ module @patterns {
     %val2 = pdl.result 0 of %op2
     %op3 = pdl.operation "tf.ReluGrad" (%rxdelta, %val2 : !pdl.value, !pdl.value) -> (%out_type : !pdl.type)
     %val3 = pdl.result 0 of %op3
-    %attr1 = pdl.attribute true
+    %attr1 = pdl.attribute = true
     %op4 = pdl.operation "tf.MatMul" (%rxact, %val3 : !pdl.value, !pdl.value) {"transpose_a" = %attr1, "transpose_b" = %attr0} -> (%weight_type : !pdl.type)
     %val4 = pdl.result 0 of %op4
     %op5 = pdl.operation "kernel.GD" (%weight, %val4 : !pdl.value, !pdl.value) -> (%weight_type : !pdl.type)
@@ -198,9 +198,9 @@ module @patterns {
     %rxdelta = pdl.operand : %out_type
     %weight = pdl.operand : %weight_type
 
-    %attr0 = pdl.attribute false
+    %attr0 = pdl.attribute = false
     %op0 = pdl.operation "tf.MatMul" (%rxact, %weight : !pdl.value, !pdl.value) {"transpose_a" = %attr0, "transpose_b" = %attr0} -> (%out_type : !pdl.type)
-    %attr1 = pdl.attribute true
+    %attr1 = pdl.attribute = true
     %op1 = pdl.operation "tf.MatMul" (%rxdelta, %weight : !pdl.value, !pdl.value) {"transpose_a" = %attr0, "transpose_b" = %attr1} -> (%in_type : !pdl.type)
     %op2 = pdl.operation "tf.MatMul" (%rxact, %rxdelta : !pdl.value, !pdl.value) {"transpose_a" = %attr1, "transpose_b" = %attr0} -> (%weight_type : !pdl.type)
     %val2 = pdl.result 0 of %op2

diff --git a/mlir/test/mlir-pdll/CodeGen/MLIR/expr.pdll b/mlir/test/mlir-pdll/CodeGen/MLIR/expr.pdll
index e8db46c..2d69602 100644 (file)
--- a/mlir/test/mlir-pdll/CodeGen/MLIR/expr.pdll
+++ b/mlir/test/mlir-pdll/CodeGen/MLIR/expr.pdll
@@ -5,7 +5,7 @@
 //===----------------------------------------------------------------------===//
 
 // CHECK: pdl.pattern @AttrExpr
-// CHECK: %[[ATTR:.*]] = attribute 10
+// CHECK: %[[ATTR:.*]] = attribute = 10
 // CHECK: operation({{.*}}) {"attr" = %[[ATTR]]}
 Pattern AttrExpr => erase op<> { attr = attr<"10"> };
 

diff --git a/mlir/test/python/dialects/pdl_ops.py b/mlir/test/python/dialects/pdl_ops.py
index b575f19..3d9cd19 100644 (file)
--- a/mlir/test/python/dialects/pdl_ops.py
+++ b/mlir/test/python/dialects/pdl_ops.py
@@ -220,7 +220,7 @@ def test_native_rewrite():
 # CHECK:   pdl.pattern @attribute_with_value : benefit(1)  {
 # CHECK:     %0 = operation
 # CHECK:     rewrite %0  {
-# CHECK:       %1 = attribute "value"
+# CHECK:       %1 = attribute = "value"
 # CHECK:       apply_native_rewrite "NativeRewrite"(%1 : !pdl.attribute)
 # CHECK:     }
 # CHECK:   }