From 95521c7eca582ace4626f38b1e58f047e6686b98 Mon Sep 17 00:00:00 2001 From: Matthias Gehre Date: Thu, 10 Oct 2024 12:49:19 +0200 Subject: [PATCH] Fix verbatim parsing to be unambiguous With the previous parsing, it would interpret ``` emitc.verbatim "#endif // PL_USE_XRT" %4 = "emitc.constant"() <{value = 1 : i32}> : () -> i32 ``` as if ``` emitc.verbatim "#endif // PL_USE_XRT" %4 = "emitc.constant"() <{value = 1 : i32}> : () -> i32 ``` and then complain that it expected a `:` after the `%4`. Fix this by introducing a `args` keyword to distinguish the case where the veratim has args from the case where the next operation starts. --- mlir/include/mlir/Dialect/EmitC/IR/EmitC.td | 2 +- mlir/test/Dialect/EmitC/invalid_ops.mlir | 12 ++++++------ mlir/test/Dialect/EmitC/ops.mlir | 9 +++++++-- mlir/test/Target/Cpp/verbatim.mlir | 10 +++++----- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td index 8d434d88e910a9..0de8787ba1dc8f 100644 --- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td +++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td @@ -1181,7 +1181,7 @@ def EmitC_VerbatimOp : EmitC_Op<"verbatim"> { let builders = [OpBuilder<(ins "::mlir::StringAttr":$value), [{ build($_builder, $_state, value, {}); }] >]; let builders = [OpBuilder<(ins "::llvm::StringRef":$value), [{ build($_builder, $_state, value, {}); }] >]; let hasVerifier = 1; - let assemblyFormat = "$value ($fmtArgs^ `:` type($fmtArgs))? attr-dict"; + let assemblyFormat = "$value (`args` $fmtArgs^ `:` type($fmtArgs))? attr-dict"; } def EmitC_AssignOp : EmitC_Op<"assign", []> { diff --git a/mlir/test/Dialect/EmitC/invalid_ops.mlir b/mlir/test/Dialect/EmitC/invalid_ops.mlir index 4d52dd39b02489..3b4c6046a08c5a 100644 --- a/mlir/test/Dialect/EmitC/invalid_ops.mlir +++ b/mlir/test/Dialect/EmitC/invalid_ops.mlir @@ -481,7 +481,7 @@ emitc.global const @myref : !emitc.array<2xi16> ref func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { // expected-error @+1 {{'emitc.verbatim' op requires operands for each placeholder in the format string}} - emitc.verbatim "" %arg0, %arg1 : !emitc.ptr, i32 + emitc.verbatim "" args %arg0, %arg1 : !emitc.ptr, i32 return } @@ -489,7 +489,7 @@ func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { // expected-error @+1 {{'emitc.verbatim' op requires operands for each placeholder in the format string}} - emitc.verbatim "abc" %arg0, %arg1 : !emitc.ptr, i32 + emitc.verbatim "abc" args %arg0, %arg1 : !emitc.ptr, i32 return } @@ -497,7 +497,7 @@ func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { // expected-error @+1 {{'emitc.verbatim' op requires operands for each placeholder in the format string}} - emitc.verbatim "{}" %arg0, %arg1 : !emitc.ptr, i32 + emitc.verbatim "{}" args %arg0, %arg1 : !emitc.ptr, i32 return } @@ -505,7 +505,7 @@ func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { // expected-error @+1 {{'emitc.verbatim' op requires operands for each placeholder in the format string}} - emitc.verbatim "{} {} {}" %arg0, %arg1 : !emitc.ptr, i32 + emitc.verbatim "{} {} {}" args %arg0, %arg1 : !emitc.ptr, i32 return } @@ -513,7 +513,7 @@ func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { // expected-error @+1 {{'emitc.verbatim' op expected '}' after unescaped '{'}} - emitc.verbatim "{ " %arg0, %arg1 : !emitc.ptr, i32 + emitc.verbatim "{ " args %arg0, %arg1 : !emitc.ptr, i32 return } @@ -521,6 +521,6 @@ func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { // expected-error @+1 {{'emitc.verbatim' op expected '}' after unescaped '{'}} - emitc.verbatim "{a} " %arg0, %arg1 : !emitc.ptr, i32 + emitc.verbatim "{a} " args %arg0, %arg1 : !emitc.ptr, i32 return } diff --git a/mlir/test/Dialect/EmitC/ops.mlir b/mlir/test/Dialect/EmitC/ops.mlir index 23b5421d860ff3..4e86642c2a3a95 100644 --- a/mlir/test/Dialect/EmitC/ops.mlir +++ b/mlir/test/Dialect/EmitC/ops.mlir @@ -244,10 +244,15 @@ emitc.verbatim "typedef float f32;" emitc.verbatim "{} { }" func.func @test_verbatim(%arg0 : !emitc.ptr, %arg1 : i32) { - emitc.verbatim "{} + {};" %arg0, %arg1 : !emitc.ptr, i32 + emitc.verbatim "{} + {};" args %arg0, %arg1 : !emitc.ptr, i32 // Trailing '{' are ok and don't start a placeholder. - emitc.verbatim "{} + {} {" %arg0, %arg1 : !emitc.ptr, i32 + emitc.verbatim "{} + {} {" args %arg0, %arg1 : !emitc.ptr, i32 + + // Check there is no ambiguity whether %a is the argument to the emitc.verbatim op. + emitc.verbatim "a" + %a = "emitc.constant"(){value = 42 : i32} : () -> i32 + return } diff --git a/mlir/test/Target/Cpp/verbatim.mlir b/mlir/test/Target/Cpp/verbatim.mlir index 1522cc32e79a52..bc687fbb0e31bf 100644 --- a/mlir/test/Target/Cpp/verbatim.mlir +++ b/mlir/test/Target/Cpp/verbatim.mlir @@ -25,20 +25,20 @@ emitc.func @func(%arg: f32) { %a = "emitc.variable"(){value = #emitc.opaque<"">} : () -> !emitc.array<3x7xi32> // CHECK: int32_t [[A:[^ ]*]][3][7]; - emitc.verbatim "{}" %arg : f32 + emitc.verbatim "{}" args %arg : f32 // CHECK: [[V0]] - emitc.verbatim "{} {{a" %arg : f32 + emitc.verbatim "{} {{a" args %arg : f32 // CHECK-NEXT: [[V0]] {a - emitc.verbatim "#pragma my var={} property" %arg : f32 + emitc.verbatim "#pragma my var={} property" args %arg : f32 // CHECK-NEXT: #pragma my var=[[V0]] property // Trailing '{' are printed as-is. - emitc.verbatim "#pragma my var={} {" %arg : f32 + emitc.verbatim "#pragma my var={} {" args %arg : f32 // CHECK-NEXT: #pragma my var=[[V0]] { - emitc.verbatim "#pragma my2 var={} property" %a : !emitc.array<3x7xi32> + emitc.verbatim "#pragma my2 var={} property" args %a : !emitc.array<3x7xi32> // CHECK-NEXT: #pragma my2 var=[[A]] property emitc.return }