Skip to content

Commit d4381b3

Browse files
committed
[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
1 parent 92a836d commit d4381b3

File tree

8 files changed

+31
-18
lines changed

8 files changed

+31
-18
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,14 @@ def PDL_AttributeOp : PDL_Op<"attribute"> {
118118
%attr = pdl.attribute : %type
119119

120120
// Define an attribute with a constant value:
121-
%attr = pdl.attribute "hello"
121+
%attr = pdl.attribute = "hello"
122122
```
123123
}];
124124

125125
let arguments = (ins Optional<PDL_Type>:$type,
126126
OptionalAttr<AnyAttr>:$value);
127127
let results = (outs PDL_Attribute:$attr);
128-
let assemblyFormat = "(`:` $type^)? ($value^)? attr-dict-with-keyword";
128+
let assemblyFormat = "(`:` $type^)? (`=` $value^)? attr-dict-with-keyword";
129129

130130
let builders = [
131131
OpBuilder<(ins CArg<"Value", "Value()">:$type), [{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ module @attributes {
4949
// CHECK-DAG: pdl_interp.check_type %[[ATTR1_TYPE]] is i64
5050
pdl.pattern : benefit(1) {
5151
%type = type : i64
52-
%attr = attribute 10 : i64
52+
%attr = attribute = 10 : i64
5353
%attr1 = attribute : %type
5454
%root = operation {"attr" = %attr, "attr1" = %attr1}
5555
rewrite %root with "rewriter"
@@ -583,7 +583,7 @@ module @attribute_literal {
583583

584584
// Check the correct lowering of an attribute that hasn't been bound.
585585
pdl.pattern : benefit(1) {
586-
%attr = attribute 10
586+
%attr = attribute = 10
587587
pdl.apply_native_constraint "constraint"(%attr: !pdl.attribute)
588588

589589
%root = operation

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ module @operation_attributes {
4242
%attr = attribute
4343
%root = operation "foo.op" {"attr" = %attr}
4444
rewrite %root {
45-
%attr1 = attribute true
45+
%attr1 = attribute = true
4646
%newOp = operation "foo.op" {"attr" = %attr, "attr1" = %attr1}
4747
erase %root
4848
}

mlir/test/Dialect/PDL/invalid.mlir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pdl.pattern : benefit(1) {
3636
%type = type
3737

3838
// expected-error@below {{expected only one of [`type`, `value`] to be set}}
39-
%attr = attribute : %type 10
39+
%attr = attribute : %type = 10
4040

4141
%op = operation "foo.op" {"attr" = %attr} -> (%type : !pdl.type)
4242
rewrite %op with "rewriter"

mlir/test/Dialect/PDL/ops.mlir

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// RUN: mlir-opt -split-input-file %s | mlir-opt
2-
// Verify the generic form can be parsed.
3-
// RUN: mlir-opt -split-input-file -mlir-print-op-generic %s | mlir-opt
2+
// RUN: mlir-opt -split-input-file -mlir-print-op-generic -mlir-print-local-scope %s | FileCheck %s --check-prefix=CHECK-GENERIC
43

54
// -----
65

@@ -138,7 +137,21 @@ pdl.pattern @apply_rewrite_with_no_results : benefit(1) {
138137
pdl.pattern @attribute_with_dict : benefit(1) {
139138
%root = operation
140139
rewrite %root {
141-
%attr = attribute {some_unit_attr} attributes {pdl.special_attribute}
140+
%attr = attribute = {some_unit_attr} attributes {pdl.special_attribute}
142141
apply_native_rewrite "NativeRewrite"(%attr : !pdl.attribute)
143142
}
144143
}
144+
145+
// -----
146+
147+
// Check that we don't treat the trailing location of a pdl.attribute as the
148+
// attribute value.
149+
150+
pdl.pattern @attribute_with_loc : benefit(1) {
151+
// CHECK-GENERIC: "pdl.attribute"
152+
// CHECK-GENERIC-NOT: value = loc
153+
%attr = attribute loc("bar")
154+
155+
%root = operation {"attribute" = %attr}
156+
rewrite %root with "rewriter"
157+
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ module @patterns {
1515
%rxact = pdl.operand : %in_type
1616
%weight = pdl.operand : %weight_type
1717

18-
%attr0 = pdl.attribute false
18+
%attr0 = pdl.attribute = false
1919
%op0 = pdl.operation "tf.MatMul" (%rxact, %weight : !pdl.value, !pdl.value) {"transpose_a" = %attr0, "transpose_b" = %attr0} -> (%out_type : !pdl.type)
2020

2121
pdl.rewrite %op0 {
@@ -35,8 +35,8 @@ module @patterns {
3535
%rxdelta = pdl.operand : %out_type
3636
%weight = pdl.operand : %weight_type
3737

38-
%attr0 = pdl.attribute true
39-
%attr1 = pdl.attribute false
38+
%attr0 = pdl.attribute = true
39+
%attr1 = pdl.attribute = false
4040
%op0 = pdl.operation "tf.MatMul" (%rxact, %rxdelta : !pdl.value, !pdl.value) {"transpose_a" = %attr0, "transpose_b" = %attr1} -> (%weight_type : !pdl.type)
4141
%val0 = pdl.result 0 of %op0
4242
%op1 = pdl.operation "tf.Const" -> (%const_type : !pdl.type)
@@ -156,7 +156,7 @@ module @patterns {
156156
%weight = pdl.operand : %weight_type
157157
%bias = pdl.operand : %bias_type
158158

159-
%attr0 = pdl.attribute false
159+
%attr0 = pdl.attribute = false
160160
%op0 = pdl.operation "tf.MatMul" (%rxact, %weight : !pdl.value, !pdl.value) {"transpose_a" = %attr0, "transpose_b" = %attr0} -> (%out_type : !pdl.type)
161161
%val0 = pdl.result 0 of %op0
162162
%op1 = pdl.operation "tf.BiasAdd" (%val0, %bias : !pdl.value, !pdl.value) -> (%out_type : !pdl.type)
@@ -165,7 +165,7 @@ module @patterns {
165165
%val2 = pdl.result 0 of %op2
166166
%op3 = pdl.operation "tf.ReluGrad" (%rxdelta, %val2 : !pdl.value, !pdl.value) -> (%out_type : !pdl.type)
167167
%val3 = pdl.result 0 of %op3
168-
%attr1 = pdl.attribute true
168+
%attr1 = pdl.attribute = true
169169
%op4 = pdl.operation "tf.MatMul" (%rxact, %val3 : !pdl.value, !pdl.value) {"transpose_a" = %attr1, "transpose_b" = %attr0} -> (%weight_type : !pdl.type)
170170
%val4 = pdl.result 0 of %op4
171171
%op5 = pdl.operation "kernel.GD" (%weight, %val4 : !pdl.value, !pdl.value) -> (%weight_type : !pdl.type)
@@ -198,9 +198,9 @@ module @patterns {
198198
%rxdelta = pdl.operand : %out_type
199199
%weight = pdl.operand : %weight_type
200200

201-
%attr0 = pdl.attribute false
201+
%attr0 = pdl.attribute = false
202202
%op0 = pdl.operation "tf.MatMul" (%rxact, %weight : !pdl.value, !pdl.value) {"transpose_a" = %attr0, "transpose_b" = %attr0} -> (%out_type : !pdl.type)
203-
%attr1 = pdl.attribute true
203+
%attr1 = pdl.attribute = true
204204
%op1 = pdl.operation "tf.MatMul" (%rxdelta, %weight : !pdl.value, !pdl.value) {"transpose_a" = %attr0, "transpose_b" = %attr1} -> (%in_type : !pdl.type)
205205
%op2 = pdl.operation "tf.MatMul" (%rxact, %rxdelta : !pdl.value, !pdl.value) {"transpose_a" = %attr1, "transpose_b" = %attr0} -> (%weight_type : !pdl.type)
206206
%val2 = pdl.result 0 of %op2

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
//===----------------------------------------------------------------------===//
66

77
// CHECK: pdl.pattern @AttrExpr
8-
// CHECK: %[[ATTR:.*]] = attribute 10
8+
// CHECK: %[[ATTR:.*]] = attribute = 10
99
// CHECK: operation({{.*}}) {"attr" = %[[ATTR]]}
1010
Pattern AttrExpr => erase op<> { attr = attr<"10"> };
1111

mlir/test/python/dialects/pdl_ops.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ def test_native_rewrite():
220220
# CHECK: pdl.pattern @attribute_with_value : benefit(1) {
221221
# CHECK: %0 = operation
222222
# CHECK: rewrite %0 {
223-
# CHECK: %1 = attribute "value"
223+
# CHECK: %1 = attribute = "value"
224224
# CHECK: apply_native_rewrite "NativeRewrite"(%1 : !pdl.attribute)
225225
# CHECK: }
226226
# CHECK: }

0 commit comments

Comments
 (0)