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

[rriddle]

Summary

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.

Depends on D124581

[nicolasvasilache]

mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir

52

This looks weird to me; was something like %attr = attribute<10> : i64 not an option ?
Looks a bit like a template this way which feels more intuitive to me..

[rriddle]

mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir

52

Do you have any suggestions here? Happy to change to something else.

mlir/include/mlir/Dialect/PDL/IR/PDLOps.td

let description = [{
     // Define an attribute:
     %attr = pdl.attribute
 
     // Define an attribute with an expected type:
     %type = pdl.type : i32
     %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), [{
       build($_builder, $_state, $_builder.getType<AttributeType>(), type,
             Attribute());
     }]>,
     OpBuilder<(ins "Attribute":$attr), [{
       build($_builder, $_state, $_builder.getType<AttributeType>(), Value(), attr);

mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir

module @attributes {
 
   // Check the type of "attr1".
   // CHECK-DAG:   %[[ATTR1:.*]] = pdl_interp.get_attribute "attr1" of %[[ROOT]]
   // CHECK-DAG:   pdl_interp.is_not_null %[[ATTR1]] : !pdl.attribute
   // CHECK-DAG:   %[[ATTR1_TYPE:.*]] = pdl_interp.get_attribute_type of %[[ATTR1]]
   // 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"
   }
 }
 
 // -----
 

 // CHECK-LABEL: module @attribute_literal
 module @attribute_literal {
   // CHECK: func @matcher(%{{.*}}: !pdl.operation)
   // CHECK: %[[ATTR:.*]] = pdl_interp.create_attribute 10 : i64
   // CHECK: pdl_interp.apply_constraint "constraint"(%[[ATTR]] : !pdl.attribute)
 
   // 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
     rewrite %root with "rewriter"
   }
 }
 
 // -----

mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-rewriter.mlir

module @operation_attributes {
   // CHECK: module @rewriters
   // CHECK:   func @pdl_generated_rewriter(%[[ATTR:.*]]: !pdl.attribute, %[[ROOT:.*]]: !pdl.operation)
   // CHECK:     %[[ATTR1:.*]] = pdl_interp.create_attribute true
   // CHECK:     pdl_interp.create_operation "foo.op" {"attr" = %[[ATTR]], "attr1" = %[[ATTR1]]}
   pdl.pattern : benefit(1) {
     %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
     }
   }
 }
 
 // -----
 

mlir/test/Dialect/PDL/invalid.mlir

 //===----------------------------------------------------------------------===//
 // pdl::AttributeOp
 //===----------------------------------------------------------------------===//
 
 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"
 }
 
 // -----
 
 pdl.pattern : benefit(1) {

mlir/test/Dialect/PDL/ops.mlir

 // 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
 
 // -----
 
 pdl.pattern @operations : benefit(1) {
   // Operation with attributes and results.
   %attribute = attribute
   %type = type
   %op0 = operation {"attr" = %attribute} -> (%type : !pdl.type)
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"
+}

mlir/test/Integration/Dialect/PDL/CPU/multiroot.mlir

module @patterns {
   // fc_fwd
   pdl.pattern : benefit(1) {
     %in_type = pdl.type
     %out_type = pdl.type
     %weight_type = pdl.type
     %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 {
       %op1 = pdl.operation "kernel.FcFwd" (%rxact, %weight : !pdl.value, !pdl.value) -> (%out_type : !pdl.type)
       %val1 = pdl.result 0 of %op1  // txact
       pdl.replace %op0 with (%val1 : !pdl.value)  // tf.MatMul
     }
   }
 
   // fc_bwd
   pdl.pattern : benefit(4) {
     %in_type = pdl.type
     %out_type = pdl.type
     %weight_type = pdl.type
     %const_type = pdl.type
     %rxact = pdl.operand : %in_type
     %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)
     %val1 = pdl.result 0 of %op1
     %op2 = pdl.operation "tf.Mul" (%val0, %val1 : !pdl.value, !pdl.value) -> (%weight_type : !pdl.type)
     %val2 = pdl.result 0 of %op2
     %op3 = pdl.operation "tf.Sub" (%weight, %val2 : !pdl.value, !pdl.value) -> (%weight_type : !pdl.type)
 
pdl.pattern : benefit(8) {
     %out_type = pdl.type
     %weight_type = pdl.type
     %bias_type = pdl.type
     %rxact = pdl.operand : %in_type
     %rxdelta = pdl.operand : %out_type
     %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)
     %val1 = pdl.result 0 of %op1
     %op2 = pdl.operation "tf.Relu" (%val1 : !pdl.value) -> (%out_type : !pdl.type)
     %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)
     %op6 = pdl.operation "tf.BiasAddGrad" (%val3 : !pdl.value) -> (%bias_type : !pdl.type)
     %val6 = pdl.result 0 of %op6
     %op7 = pdl.operation "kernel.GD" (%bias, %val6 : !pdl.value, !pdl.value) -> (%bias_type : !pdl.type)
 
     pdl.rewrite {  // roots: %op2, %op5, %op7
module @patterns {
   pdl.pattern : benefit(4) {
     %in_type = pdl.type
     %out_type = pdl.type
     %weight_type = pdl.type
     %rxact = pdl.operand : %in_type
     %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
     %op3 = pdl.operation "kernel.GD" (%weight, %val2 : !pdl.value, !pdl.value) -> (%weight_type : !pdl.type)
 
     pdl.rewrite {  // roots: %op0, %op1, %op3
       %op4 = pdl.operation "kernel.Fc" (%rxact, %rxdelta, %weight : !pdl.value, !pdl.value, !pdl.value) -> (%out_type, %in_type, %weight_type : !pdl.type, !pdl.type, !pdl.type)
       %val4_0 = pdl.result 0 of %op4  // txact

mlir/test/mlir-pdll/CodeGen/MLIR/expr.pdll

 // RUN: mlir-pdll %s -I %S -I %S/../../../../include -split-input-file -x mlir | FileCheck %s
 
 //===----------------------------------------------------------------------===//
 // AttributeExpr
 //===----------------------------------------------------------------------===//
 
 // CHECK: pdl.pattern @AttrExpr
-// CHECK: %[[ATTR:.*]] = attribute 10
+// CHECK: %[[ATTR:.*]] = attribute = 10
 // CHECK: operation({{.*}}) {"attr" = %[[ATTR]]}
 Pattern AttrExpr => erase op<> { attr = attr<"10"> };
 
 // -----
 
 //===----------------------------------------------------------------------===//
 // CallExpr
 //===----------------------------------------------------------------------===//

mlir/test/python/dialects/pdl_ops.py

with InsertionPoint(pattern.body):
     rewrite = RewriteOp(root)
     with InsertionPoint(rewrite.add_body()):
       ApplyNativeRewriteOp([], "NativeRewrite", args=[root])
 
 # CHECK: module  {
 # 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:   }
 # CHECK: }
 @constructAndPrintInModule
 def test_attribute_with_value():
   pattern = PatternOp(1, "attribute_with_value")
   with InsertionPoint(pattern.body):