From 2951f3220cdb65ba1975d09ae3331ad3f247ce80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20M=C3=BCller?= Date: Tue, 4 Jun 2024 11:15:28 +0000 Subject: [PATCH] [Substrait] Simplify assembly format of `field_reference` op. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR removes a pair of `[.]` around the integer array `position` attribute of the `field_reference` op, which isn't necessary. The original format had used the additional pair of brackets out of ignorance and was misleading as it suggested that there is an additional nesting level, which actually doesn't exist. Signed-off-by: Ingo Müller --- .../Dialect/Substrait/IR/SubstraitOps.td | 2 +- .../Substrait/field-reference-invalid.mlir | 4 +- test/Dialect/Substrait/field-reference.mlir | 16 +++---- test/Dialect/Substrait/project-invalid.mlir | 2 +- .../SubstraitPB/Export/field-reference.mlir | 6 +-- .../SubstraitPB/Import/field-reference.textpb | 6 +-- .../Substrait/emit-deduplication.mlir | 42 +++++++++---------- 7 files changed, 39 insertions(+), 39 deletions(-) diff --git a/include/structured/Dialect/Substrait/IR/SubstraitOps.td b/include/structured/Dialect/Substrait/IR/SubstraitOps.td index 6078c1c4f5e4..9729a25cd6b5 100644 --- a/include/structured/Dialect/Substrait/IR/SubstraitOps.td +++ b/include/structured/Dialect/Substrait/IR/SubstraitOps.td @@ -189,7 +189,7 @@ def Substrait_FieldReferenceOp : Substrait_ExpressionOp<"field_reference", [ ); let results = (outs Substrait_FieldType:$result); let assemblyFormat = [{ - $container `[` $position `]` attr-dict `:` type($container) + $container `` $position attr-dict `:` type($container) }]; } diff --git a/test/Dialect/Substrait/field-reference-invalid.mlir b/test/Dialect/Substrait/field-reference-invalid.mlir index dc59681f21ea..f248566aa442 100644 --- a/test/Dialect/Substrait/field-reference-invalid.mlir +++ b/test/Dialect/Substrait/field-reference-invalid.mlir @@ -7,7 +7,7 @@ substrait.plan version 0 : 42 : 1 { ^bb0(%arg : tuple): // expected-error@+2 {{can't extract element from type 'si32'}} // expected-error@+1 {{mismatching position and type (position: array, type: 'tuple')}} - %2 = field_reference %arg[[0, 0]] : tuple + %2 = field_reference %arg[0, 0] : tuple %3 = literal 0 : si1 yield %3 : si1 } @@ -24,7 +24,7 @@ substrait.plan version 0 : 42 : 1 { ^bb0(%arg : tuple): // expected-error@+2 {{2 is not a valid index for 'tuple'}} // expected-error@+1 {{mismatching position and type (position: array, type: 'tuple')}} - %2 = field_reference %arg[[2]] : tuple + %2 = field_reference %arg[2] : tuple %3 = literal 0 : si1 yield %3 : si1 } diff --git a/test/Dialect/Substrait/field-reference.mlir b/test/Dialect/Substrait/field-reference.mlir index 752f8b1590fd..c3fb596513f6 100644 --- a/test/Dialect/Substrait/field-reference.mlir +++ b/test/Dialect/Substrait/field-reference.mlir @@ -6,7 +6,7 @@ // CHECK-NEXT: named_table // CHECK-NEXT: filter // CHECK-NEXT: (%[[ARG0:.*]]: tuple): -// CHECK-NEXT: %[[V0:.*]] = field_reference %[[ARG0]]{{\[}}[0]] : tuple +// CHECK-NEXT: %[[V0:.*]] = field_reference %[[ARG0]][0] : tuple // CHECK-NEXT: yield %[[V0]] : si1 substrait.plan version 0 : 42 : 1 { @@ -14,7 +14,7 @@ substrait.plan version 0 : 42 : 1 { %0 = named_table @t1 as ["a"] : tuple %1 = filter %0 : tuple { ^bb0(%arg : tuple): - %2 = field_reference %arg[[0]] : tuple + %2 = field_reference %arg[0] : tuple yield %2 : si1 } yield %1 : tuple @@ -28,7 +28,7 @@ substrait.plan version 0 : 42 : 1 { // CHECK-NEXT: named_table // CHECK-NEXT: filter // CHECK-NEXT: (%[[ARG0:.*]]: tuple>): -// CHECK-NEXT: %[[V0:.*]] = field_reference %[[ARG0]]{{\[}}[1, 0]] : tuple> +// CHECK-NEXT: %[[V0:.*]] = field_reference %[[ARG0]][1, 0] : tuple> // CHECK-NEXT: yield %[[V0]] : si1 substrait.plan version 0 : 42 : 1 { @@ -36,7 +36,7 @@ substrait.plan version 0 : 42 : 1 { %0 = named_table @t1 as ["a", "b", "c"] : tuple> %1 = filter %0 : tuple> { ^bb0(%arg : tuple>): - %2 = field_reference %arg[[1, 0]] : tuple> + %2 = field_reference %arg[1, 0] : tuple> yield %2 : si1 } yield %1 : tuple> @@ -50,8 +50,8 @@ substrait.plan version 0 : 42 : 1 { // CHECK-NEXT: named_table // CHECK-NEXT: filter // CHECK-NEXT: (%[[ARG0:.*]]: tuple>): -// CHECK-NEXT: %[[V0:.*]] = field_reference %[[ARG0]]{{\[}}[1]] : tuple> -// CHECK-NEXT: %[[V1:.*]] = field_reference %[[V0]]{{\[}}[0]] : tuple +// CHECK-NEXT: %[[V0:.*]] = field_reference %[[ARG0]][1] : tuple> +// CHECK-NEXT: %[[V1:.*]] = field_reference %[[V0]][0] : tuple // CHECK-NEXT: yield %[[V1]] : si1 substrait.plan version 0 : 42 : 1 { @@ -59,8 +59,8 @@ substrait.plan version 0 : 42 : 1 { %0 = named_table @t1 as ["a", "b", "c"] : tuple> %1 = filter %0 : tuple> { ^bb0(%arg : tuple>): - %2 = field_reference %arg[[1]] : tuple> - %3 = field_reference %2[[0]] : tuple + %2 = field_reference %arg[1] : tuple> + %3 = field_reference %2[0] : tuple yield %3 : si1 } yield %1 : tuple> diff --git a/test/Dialect/Substrait/project-invalid.mlir b/test/Dialect/Substrait/project-invalid.mlir index 694b012578dc..1166ecdb793f 100644 --- a/test/Dialect/Substrait/project-invalid.mlir +++ b/test/Dialect/Substrait/project-invalid.mlir @@ -50,7 +50,7 @@ substrait.plan version 0 : 42 : 1 { // expected-error@+1 {{'substrait.project' op has 'expressions' region with mismatching argument type (has: 'tuple', expected: 'tuple')}} %1 = project %0 : tuple -> tuple { ^bb0(%arg : tuple): - %3 = field_reference %arg[[0]] : tuple + %3 = field_reference %arg[0] : tuple yield %3 : si1 } yield %1 : tuple diff --git a/test/Target/SubstraitPB/Export/field-reference.mlir b/test/Target/SubstraitPB/Export/field-reference.mlir index 41e038ffd5fc..d35daceff83d 100644 --- a/test/Target/SubstraitPB/Export/field-reference.mlir +++ b/test/Target/SubstraitPB/Export/field-reference.mlir @@ -30,7 +30,7 @@ substrait.plan version 0 : 42 : 1 { %0 = named_table @t1 as ["a", "b", "c"] : tuple> %1 = filter %0 : tuple> { ^bb0(%arg : tuple>): - %2 = field_reference %arg[[1, 0]] : tuple> + %2 = field_reference %arg[1, 0] : tuple> yield %2 : si1 } yield %1 : tuple> @@ -67,8 +67,8 @@ substrait.plan version 0 : 42 : 1 { %0 = named_table @t1 as ["a", "b", "c"] : tuple> %1 = filter %0 : tuple> { ^bb0(%arg : tuple>): - %2 = field_reference %arg[[1]] : tuple> - %3 = field_reference %2[[0]] : tuple + %2 = field_reference %arg[1] : tuple> + %3 = field_reference %2[0] : tuple yield %3 : si1 } yield %1 : tuple> diff --git a/test/Target/SubstraitPB/Import/field-reference.textpb b/test/Target/SubstraitPB/Import/field-reference.textpb index 63e84c8170f1..f6c176a7ca41 100644 --- a/test/Target/SubstraitPB/Import/field-reference.textpb +++ b/test/Target/SubstraitPB/Import/field-reference.textpb @@ -15,7 +15,7 @@ # CHECK-NEXT: named_table # CHECK-NEXT: filter # CHECK-NEXT: (%[[ARG0:.*]]: tuple>) -# CHECK-NEXT: %[[V0:.*]] = field_reference %[[ARG0]]{{\[}}[1, 0]] +# CHECK-NEXT: %[[V0:.*]] = field_reference %[[ARG0]][1, 0] # CHECK-SAME: : tuple> # CHECK-NEXT: yield %[[V0]] : si1 @@ -89,9 +89,9 @@ version { # CHECK-NEXT: named_table # CHECK-NEXT: filter # CHECK-NEXT: (%[[ARG0:.*]]: tuple>) -# CHECK-NEXT: %[[V0:.*]] = field_reference %[[ARG0]]{{\[}}[1]] +# CHECK-NEXT: %[[V0:.*]] = field_reference %[[ARG0]][1] # CHECK-SAME: : tuple> -# CHECK-NEXT: %[[V1:.*]] = field_reference %[[V0]]{{\[}}[0]] +# CHECK-NEXT: %[[V1:.*]] = field_reference %[[V0]][0] # CHECK-SAME: : tuple # CHECK-NEXT: yield %[[V1]] : si1 diff --git a/test/Transforms/Substrait/emit-deduplication.mlir b/test/Transforms/Substrait/emit-deduplication.mlir index a353d22a74db..68233beb1ae3 100644 --- a/test/Transforms/Substrait/emit-deduplication.mlir +++ b/test/Transforms/Substrait/emit-deduplication.mlir @@ -160,11 +160,11 @@ substrait.plan version 0 : 42 : 1 { // CHECK-NEXT: %[[V1:.*]] = emit [1, 2, 0] from %[[V0]] : // CHECK-NEXT: %[[V2:.*]] = filter %[[V1]] : {{.*}} { // CHECK-NEXT: ^{{.*}}(%[[ARG0:.*]]: [[TYPE:.*]]): -// CHECK-NEXT: %[[V3:.*]] = field_reference %[[ARG0]]{{\[}}[0]] : [[TYPE]] -// CHECK-NEXT: %[[V5:.*]] = field_reference %[[ARG0]]{{\[}}[1, 0]] : [[TYPE]] -// CHECK-NEXT: %[[V6:.*]] = field_reference %[[ARG0]]{{\[}}[1]] : [[TYPE]] -// CHECK-NEXT: %[[V7:.*]] = field_reference %[[V6]]{{\[}}[1]] : -// CHECK-NEXT: %[[V9:.*]] = field_reference %[[ARG0]]{{\[}}[2]] : [[TYPE]] +// CHECK-NEXT: %[[V3:.*]] = field_reference %[[ARG0]][0] : [[TYPE]] +// CHECK-NEXT: %[[V5:.*]] = field_reference %[[ARG0]][1, 0] : [[TYPE]] +// CHECK-NEXT: %[[V6:.*]] = field_reference %[[ARG0]][1] : [[TYPE]] +// CHECK-NEXT: %[[V7:.*]] = field_reference %[[V6]][1] : +// CHECK-NEXT: %[[V9:.*]] = field_reference %[[ARG0]][2] : [[TYPE]] // CHECK-NEXT: %[[Va:.*]] = func.call @f(%[[V3]], %[[V3]], %[[V5]], %[[V7]], %[[V3]], %[[V9]]) // CHECK-NEXT: yield %[[Va]] : si1 // CHECK-NEXT: } @@ -182,13 +182,13 @@ substrait.plan version 0 : 42 : 1 { : tuple> -> tuple, si1, si1> %2 = filter %1 : tuple, si1, si1> { ^bb0(%arg0: tuple, si1, si1>): - %3 = field_reference %arg0[[0]] : tuple, si1, si1> - %4 = field_reference %arg0[[1]] : tuple, si1, si1> - %5 = field_reference %arg0[[2, 0]] : tuple, si1, si1> - %6 = field_reference %arg0[[2]] : tuple, si1, si1> - %7 = field_reference %6[[1]] : tuple - %8 = field_reference %arg0[[3]] : tuple, si1, si1> - %9 = field_reference %arg0[[4]] : tuple, si1, si1> + %3 = field_reference %arg0[0] : tuple, si1, si1> + %4 = field_reference %arg0[1] : tuple, si1, si1> + %5 = field_reference %arg0[2, 0] : tuple, si1, si1> + %6 = field_reference %arg0[2] : tuple, si1, si1> + %7 = field_reference %6[1] : tuple + %8 = field_reference %arg0[3] : tuple, si1, si1> + %9 = field_reference %arg0[4] : tuple, si1, si1> %a = func.call @f(%3, %4, %5, %7, %8, %9) : (si1, si1, si1, si32, si1, si1) -> si1 yield %a : si1 } @@ -206,7 +206,7 @@ substrait.plan version 0 : 42 : 1 { // CHECK-NEXT: %[[V1:.*]] = emit [1] from %[[V0]] : // CHECK-NEXT: %[[V2:.*]] = project %[[V1]] : tuple -> tuple { // CHECK-NEXT: ^{{.*}}(%[[ARG0:.*]]: [[TYPE:.*]]): -// CHECK-NEXT: %[[V3:.*]] = field_reference %[[ARG0]]{{\[}}[0]] : [[TYPE]] +// CHECK-NEXT: %[[V3:.*]] = field_reference %[[ARG0]][0] : [[TYPE]] // CHECK-NEXT: %[[V5:.*]] = func.call @f(%[[V3]], %[[V3]]) : // CHECK-NEXT: yield %[[V5]] : si1 // CHECK-NEXT: } @@ -220,8 +220,8 @@ substrait.plan version 0 : 42 : 1 { %1 = emit [1, 1] from %0 : tuple -> tuple %2 = project %1 : tuple -> tuple { ^bb0(%arg : tuple): - %3 = field_reference %arg[[0]] : tuple - %4 = field_reference %arg[[1]] : tuple + %3 = field_reference %arg[0] : tuple + %4 = field_reference %arg[1] : tuple %5 = func.call @f(%3, %4) : (si32, si32) -> si1 yield %5 : si1 } @@ -238,7 +238,7 @@ substrait.plan version 0 : 42 : 1 { // CHECK-NEXT: %[[V0:.*]] = named_table // CHECK-NEXT: %[[V1:.*]] = project %[[V0]] : {{.*}} { // CHECK-NEXT: ^{{.*}}(%[[ARG0:.*]]: [[TYPE:.*]]): -// CHECK-NEXT: %[[V2:.*]] = field_reference %[[ARG0]]{{\[}}[0]] : [[TYPE]] +// CHECK-NEXT: %[[V2:.*]] = field_reference %[[ARG0]][0] : [[TYPE]] // CHECK-NEXT: %[[V3:.*]] = func.call @f(%[[V2]]) : // CHECK-NEXT: yield %[[V3]] : si1 // CHECK-NEXT: } @@ -251,7 +251,7 @@ substrait.plan version 0 : 42 : 1 { %0 = named_table @t1 as ["a"] : tuple %1 = project %0 : tuple -> tuple { ^bb0(%arg : tuple): - %2 = field_reference %arg[[0]] : tuple + %2 = field_reference %arg[0] : tuple %3 = func.call @f(%2) : (si32) -> si1 // We yield two times the same value. This pattern should remove one of // the two and re-establish the duplicate with an `amit` after the @@ -271,7 +271,7 @@ substrait.plan version 0 : 42 : 1 { // CHECK-NEXT: %[[V0:.*]] = named_table // CHECK-NEXT: %[[V1:.*]] = project %[[V0]] : {{.*}} { // CHECK-NEXT: ^{{.*}}(%[[ARG0:.*]]: [[TYPE:.*]]): -// CHECK-NEXT: %[[V2:.*]] = field_reference %[[ARG0]]{{\[}}[0]] : [[TYPE]] +// CHECK-NEXT: %[[V2:.*]] = field_reference %[[ARG0]][0] : [[TYPE]] // CHECK-NEXT: %[[V3:.*]] = func.call @f(%[[V2]]) : // CHECK-NEXT: yield %[[V3]] : si1 // CHECK-NEXT: } @@ -284,7 +284,7 @@ substrait.plan version 0 : 42 : 1 { %0 = named_table @t1 as ["a", "b"] : tuple %1 = project %0 : tuple -> tuple { ^bb0(%arg0: tuple): - %2 = field_reference %arg0[[0]] : tuple + %2 = field_reference %arg0[0] : tuple %3 = func.call @f(%2) : (si32) -> si1 // `%2` yields an input field without modifications. This pattern removes // that yielding and re-establishes the duplicated field with an `emit` @@ -319,8 +319,8 @@ substrait.plan version 0 : 42 : 1 { %1 = emit [1, 1] from %0 : tuple -> tuple %2 = project %1 : tuple -> tuple { ^bb0(%arg : tuple): - %3 = field_reference %arg[[0]] : tuple - %4 = field_reference %arg[[1]] : tuple + %3 = field_reference %arg[0] : tuple + %4 = field_reference %arg[1] : tuple yield %3, %4 : si32, si32 } yield %2 : tuple