Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mlir][EmitC] Add custom assembly format to constant and variable ops #87618

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

simon-camp
Copy link
Contributor

@simon-camp simon-camp commented Apr 4, 2024

This mirrors the syntax for the llvm.mlir.constant operation.

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 4, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-emitc

Author: Simon Camphausen (simon-camp)

Changes

Patch is 26.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/87618.diff

17 Files Affected:

  • (modified) mlir/include/mlir/Dialect/EmitC/IR/EmitC.td (+13-11)
  • (modified) mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir (+5-10)
  • (modified) mlir/test/Conversion/MemRefToEmitC/memref-to-emitc.mlir (+3-3)
  • (modified) mlir/test/Conversion/SCFToEmitC/for.mlir (+8-8)
  • (modified) mlir/test/Conversion/SCFToEmitC/if.mlir (+2-2)
  • (modified) mlir/test/Dialect/EmitC/invalid_ops.mlir (+11-11)
  • (modified) mlir/test/Dialect/EmitC/invalid_types.mlir (+2-2)
  • (modified) mlir/test/Dialect/EmitC/ops.mlir (+3-3)
  • (modified) mlir/test/Dialect/EmitC/transforms.mlir (+2-2)
  • (modified) mlir/test/Target/Cpp/call.mlir (+1-1)
  • (modified) mlir/test/Target/Cpp/const.mlir (+12-12)
  • (modified) mlir/test/Target/Cpp/expressions.mlir (+4-4)
  • (modified) mlir/test/Target/Cpp/for.mlir (+13-13)
  • (modified) mlir/test/Target/Cpp/if.mlir (+3-3)
  • (modified) mlir/test/Target/Cpp/invalid.mlir (+1-1)
  • (modified) mlir/test/Target/Cpp/stdops.mlir (+3-3)
  • (modified) mlir/test/Target/Cpp/variable.mlir (+9-9)
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index e611fd2f0f15c4..6ea8e89ffbf601 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -344,17 +344,18 @@ def EmitC_ConstantOp : EmitC_Op<"constant", [ConstantLike]> {
 
     ```mlir
     // Integer constant
-    %0 = "emitc.constant"(){value = 42 : i32} : () -> i32
+    %0 = emitc.constant(42 : i32) : i32
 
-    // Constant emitted as `char = CHAR_MIN;`
-    %1 = "emitc.constant"() {value = #emitc.opaque<"CHAR_MIN">}
-      : () -> !emitc.opaque<"char">
+    // Constant emitted as `char c = CHAR_MIN;`
+    %1 = emitc.constant(#emitc.opaque<"CHAR_MIN">) : !emitc.opaque<"char">
     ```
   }];
 
   let arguments = (ins EmitC_OpaqueOrTypedAttr:$value);
   let results = (outs AnyType);
 
+  let assemblyFormat = "`(` $value `)` attr-dict `:` type(results)";
+
   let hasFolder = 1;
   let hasVerifier = 1;
 }
@@ -988,19 +989,19 @@ def EmitC_VariableOp : EmitC_Op<"variable", []> {
 
     ```mlir
     // Integer variable
-    %0 = "emitc.variable"(){value = 42 : i32} : () -> i32
+    %0 = emitc.variable(42 : i32) : i32
 
-    // Variable emitted as `int32_t* = NULL;`
-    %1 = "emitc.variable"() {value = #emitc.opaque<"NULL">} 
-      : () -> !emitc.ptr<!emitc.opaque<"int32_t">>
+    // Variable emitted as `int32_t* p = NULL;`
+    %1 = emitc.variable(#emitc.opaque<"NULL">)
+      : !emitc.ptr<!emitc.opaque<"int32_t">>
     ```
 
     Since folding is not supported, it can be used with pointers.
     As an example, it is valid to create pointers to `variable` operations
     by using `apply` operations and pass these to a `call` operation.
     ```mlir
-    %0 = "emitc.variable"() {value = 0 : i32} : () -> i32
-    %1 = "emitc.variable"() {value = 0 : i32} : () -> i32
+    %0 = emitc.variable(0 : i32) : i32
+    %1 = emitc.variable(0 : i32) : i32
     %2 = emitc.apply "&"(%0) : (i32) -> !emitc.ptr<i32>
     %3 = emitc.apply "&"(%1) : (i32) -> !emitc.ptr<i32>
     emitc.call_opaque "write"(%2, %3)
@@ -1011,6 +1012,7 @@ def EmitC_VariableOp : EmitC_Op<"variable", []> {
   let arguments = (ins EmitC_OpaqueOrTypedAttr:$value);
   let results = (outs AnyType);
 
+  let assemblyFormat = "`(` $value `)` attr-dict `:` type(results)";
   let hasVerifier = 1;
 }
 
@@ -1060,7 +1062,7 @@ def EmitC_AssignOp : EmitC_Op<"assign", []> {
 
     ```mlir
     // Integer variable
-    %0 = "emitc.variable"(){value = 42 : i32} : () -> i32
+    %0 = emitc.variable(42 : i32) : i32
     %1 = emitc.call_opaque "foo"() : () -> (i32)
 
     // Assign emitted as `... = ...;`
diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
index 76ba518577ab8e..efcc3205eeb227 100644
--- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
+++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
@@ -2,20 +2,15 @@
 
 // CHECK-LABEL: arith_constants
 func.func @arith_constants() {
-  // CHECK: emitc.constant
-  // CHECK-SAME: value = 0 : index
+  // CHECK: emitc.constant(0 : index)
   %c_index = arith.constant 0 : index
-  // CHECK: emitc.constant
-  // CHECK-SAME: value = 0 : i32
+  // CHECK: emitc.constant(0 : i32)
   %c_signless_int_32 = arith.constant 0 : i32
-  // CHECK: emitc.constant
-  // CHECK-SAME: value = 0.{{0+}}e+00 : f32
+  // CHECK: emitc.constant(0.{{0+}}e+00 : f32)
   %c_float_32 = arith.constant 0.0 : f32
-  // CHECK: emitc.constant
-  // CHECK-SAME: value = dense<0> : tensor<i32>
+  // CHECK: emitc.constant(dense<0> : tensor<i32>)
   %c_tensor_single_value = arith.constant dense<0> : tensor<i32>
-  // CHECK: emitc.constant
-  // CHECK-SAME: value{{.*}}[1, 2], [-3, 9], [0, 0], [2, -1]{{.*}}tensor<4x2xi64>
+  // CHECK: emitc.constant({{.*}}[1, 2], [-3, 9], [0, 0], [2, -1]{{.*}}tensor<4x2xi64>)
   %c_tensor_value = arith.constant dense<[[1, 2], [-3, 9], [0, 0], [2, -1]]> : tensor<4x2xi64>
   return
 }
diff --git a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc.mlir b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc.mlir
index 7aa2ba88843a2a..977f556d4855f6 100644
--- a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc.mlir
+++ b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc.mlir
@@ -3,7 +3,7 @@
 // CHECK-LABEL: memref_store
 // CHECK-SAME:  %[[v:.*]]: f32, %[[i:.*]]: index, %[[j:.*]]: index
 func.func @memref_store(%v : f32, %i: index, %j: index) {
-  // CHECK: %[[ALLOCA:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.array<4x8xf32>
+  // CHECK: %[[ALLOCA:.*]] = emitc.variable(#emitc.opaque<"">) : !emitc.array<4x8xf32>
   %0 = memref.alloca() : memref<4x8xf32>
 
   // CHECK: %[[SUBSCRIPT:.*]] = emitc.subscript %[[ALLOCA]][%[[i]], %[[j]]] : (!emitc.array<4x8xf32>, index, index) -> f32
@@ -16,11 +16,11 @@ func.func @memref_store(%v : f32, %i: index, %j: index) {
 // CHECK-LABEL: memref_load
 // CHECK-SAME:  %[[i:.*]]: index, %[[j:.*]]: index
 func.func @memref_load(%i: index, %j: index) -> f32 {
-  // CHECK: %[[ALLOCA:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.array<4x8xf32>
+  // CHECK: %[[ALLOCA:.*]] = emitc.variable(#emitc.opaque<"">) : !emitc.array<4x8xf32>
   %0 = memref.alloca() : memref<4x8xf32>
 
   // CHECK: %[[LOAD:.*]] = emitc.subscript %[[ALLOCA]][%[[i]], %[[j]]] : (!emitc.array<4x8xf32>, index, index) -> f32
-  // CHECK: %[[VAR:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f32
+  // CHECK: %[[VAR:.*]] = emitc.variable(#emitc.opaque<"">) : f32
   // CHECK: emitc.assign %[[LOAD]] : f32 to %[[VAR]] : f32
   %1 = memref.load %0[%i, %j] : memref<4x8xf32>
   // CHECK: return %[[VAR]] : f32
diff --git a/mlir/test/Conversion/SCFToEmitC/for.mlir b/mlir/test/Conversion/SCFToEmitC/for.mlir
index 7f90310af21894..24a8ed92ed10c3 100644
--- a/mlir/test/Conversion/SCFToEmitC/for.mlir
+++ b/mlir/test/Conversion/SCFToEmitC/for.mlir
@@ -47,10 +47,10 @@ func.func @for_yield(%arg0 : index, %arg1 : index, %arg2 : index) -> (f32, f32)
 // CHECK-SAME:      %[[VAL_0:.*]]: index, %[[VAL_1:.*]]: index, %[[VAL_2:.*]]: index) -> (f32, f32) {
 // CHECK-NEXT:    %[[VAL_3:.*]] = arith.constant 0.000000e+00 : f32
 // CHECK-NEXT:    %[[VAL_4:.*]] = arith.constant 1.000000e+00 : f32
-// CHECK-NEXT:    %[[VAL_5:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f32
-// CHECK-NEXT:    %[[VAL_6:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f32
-// CHECK-NEXT:    %[[VAL_7:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f32
-// CHECK-NEXT:    %[[VAL_8:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f32
+// CHECK-NEXT:    %[[VAL_5:.*]] = emitc.variable(#emitc.opaque<"">) : f32
+// CHECK-NEXT:    %[[VAL_6:.*]] = emitc.variable(#emitc.opaque<"">) : f32
+// CHECK-NEXT:    %[[VAL_7:.*]] = emitc.variable(#emitc.opaque<"">) : f32
+// CHECK-NEXT:    %[[VAL_8:.*]] = emitc.variable(#emitc.opaque<"">) : f32
 // CHECK-NEXT:    emitc.assign %[[VAL_3]] : f32 to %[[VAL_7]] : f32
 // CHECK-NEXT:    emitc.assign %[[VAL_4]] : f32 to %[[VAL_8]] : f32
 // CHECK-NEXT:    emitc.for %[[VAL_9:.*]] = %[[VAL_0]] to %[[VAL_1]] step %[[VAL_2]] {
@@ -77,12 +77,12 @@ func.func @nested_for_yield(%arg0 : index, %arg1 : index, %arg2 : index) -> f32
 // CHECK-LABEL: func.func @nested_for_yield(
 // CHECK-SAME:      %[[VAL_0:.*]]: index, %[[VAL_1:.*]]: index, %[[VAL_2:.*]]: index) -> f32 {
 // CHECK-NEXT:    %[[VAL_3:.*]] = arith.constant 1.000000e+00 : f32
-// CHECK-NEXT:    %[[VAL_4:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f32
-// CHECK-NEXT:    %[[VAL_5:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f32
+// CHECK-NEXT:    %[[VAL_4:.*]] = emitc.variable(#emitc.opaque<"">) : f32
+// CHECK-NEXT:    %[[VAL_5:.*]] = emitc.variable(#emitc.opaque<"">) : f32
 // CHECK-NEXT:    emitc.assign %[[VAL_3]] : f32 to %[[VAL_5]] : f32
 // CHECK-NEXT:    emitc.for %[[VAL_6:.*]] = %[[VAL_0]] to %[[VAL_1]] step %[[VAL_2]] {
-// CHECK-NEXT:      %[[VAL_7:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f32
-// CHECK-NEXT:      %[[VAL_8:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f32
+// CHECK-NEXT:      %[[VAL_7:.*]] = emitc.variable(#emitc.opaque<"">) : f32
+// CHECK-NEXT:      %[[VAL_8:.*]] = emitc.variable(#emitc.opaque<"">) : f32
 // CHECK-NEXT:      emitc.assign %[[VAL_5]] : f32 to %[[VAL_8]] : f32
 // CHECK-NEXT:      emitc.for %[[VAL_9:.*]] = %[[VAL_0]] to %[[VAL_1]] step %[[VAL_2]] {
 // CHECK-NEXT:        %[[VAL_10:.*]] = arith.addf %[[VAL_8]], %[[VAL_8]] : f32
diff --git a/mlir/test/Conversion/SCFToEmitC/if.mlir b/mlir/test/Conversion/SCFToEmitC/if.mlir
index afc9abc761eb4c..35e3120290cd71 100644
--- a/mlir/test/Conversion/SCFToEmitC/if.mlir
+++ b/mlir/test/Conversion/SCFToEmitC/if.mlir
@@ -53,8 +53,8 @@ func.func @test_if_yield(%arg0: i1, %arg1: f32) {
 // CHECK-SAME:                           %[[VAL_0:.*]]: i1,
 // CHECK-SAME:                           %[[VAL_1:.*]]: f32) {
 // CHECK-NEXT:    %[[VAL_2:.*]] = arith.constant 0 : i8
-// CHECK-NEXT:    %[[VAL_3:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> i32
-// CHECK-NEXT:    %[[VAL_4:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f64
+// CHECK-NEXT:    %[[VAL_3:.*]] = emitc.variable(#emitc.opaque<"">) : i32
+// CHECK-NEXT:    %[[VAL_4:.*]] = emitc.variable(#emitc.opaque<"">) : f64
 // CHECK-NEXT:    emitc.if %[[VAL_0]] {
 // CHECK-NEXT:      %[[VAL_5:.*]] = emitc.call_opaque "func_true_1"(%[[VAL_1]]) : (f32) -> i32
 // CHECK-NEXT:      %[[VAL_6:.*]] = emitc.call_opaque "func_true_2"(%[[VAL_1]]) : (f32) -> f64
diff --git a/mlir/test/Dialect/EmitC/invalid_ops.mlir b/mlir/test/Dialect/EmitC/invalid_ops.mlir
index bbaab0d5b6f3a9..2284a732ddf69d 100644
--- a/mlir/test/Dialect/EmitC/invalid_ops.mlir
+++ b/mlir/test/Dialect/EmitC/invalid_ops.mlir
@@ -2,7 +2,7 @@
 
 func.func @const_attribute_str() {
     // expected-error @+1 {{'emitc.constant' op string attributes are not supported, use #emitc.opaque instead}}                 
-    %c0 = "emitc.constant"(){value = "NULL"} : () -> !emitc.ptr<i32>
+    %c0 = emitc.constant("NULL") : !emitc.ptr<i32>
     return
 }
 
@@ -10,7 +10,7 @@ func.func @const_attribute_str() {
 
 func.func @const_attribute_return_type_1() {
     // expected-error @+1 {{'emitc.constant' op requires attribute to either be an #emitc.opaque attribute or it's type ('i64') to match the op's result type ('i32')}}
-    %c0 = "emitc.constant"(){value = 42: i64} : () -> i32
+    %c0 = emitc.constant(42: i64) : i32
     return
 }
 
@@ -18,7 +18,7 @@ func.func @const_attribute_return_type_1() {
 
 func.func @const_attribute_return_type_2() {
     // expected-error @+1 {{'emitc.constant' op attribute 'value' failed to satisfy constraint: An opaque attribute or TypedAttr instance}}
-    %c0 = "emitc.constant"(){value = unit} : () -> i32
+    %c0 = emitc.constant(unit) : i32
     return
 }
 
@@ -26,7 +26,7 @@ func.func @const_attribute_return_type_2() {
 
 func.func @empty_constant() {
     // expected-error @+1 {{'emitc.constant' op value must not be empty}}
-    %c0 = "emitc.constant"(){value = #emitc.opaque<"">} : () -> i32
+    %c0 = emitc.constant(#emitc.opaque<"">) : i32
     return
 }
 
@@ -105,7 +105,7 @@ func.func @illegal_operator(%arg : i32) {
 // -----
 
 func.func @illegal_operand() {
-    %1 = "emitc.constant"(){value = 42: i32} : () -> i32
+    %1 = emitc.constant(42: i32) : i32
     // expected-error @+1 {{'emitc.apply' op cannot apply to constant}}
     %2 = emitc.apply "&"(%1) : (i32) -> !emitc.ptr<i32>
     return
@@ -115,7 +115,7 @@ func.func @illegal_operand() {
 
 func.func @var_attribute_return_type_1() {
     // expected-error @+1 {{'emitc.variable' op requires attribute to either be an #emitc.opaque attribute or it's type ('i64') to match the op's result type ('i32')}}
-    %c0 = "emitc.variable"(){value = 42: i64} : () -> i32
+    %c0 = emitc.variable(42: i64) : i32
     return
 }
 
@@ -123,7 +123,7 @@ func.func @var_attribute_return_type_1() {
 
 func.func @var_attribute_return_type_2() {
     // expected-error @+1 {{'emitc.variable' op attribute 'value' failed to satisfy constraint: An opaque attribute or TypedAttr instance}}
-    %c0 = "emitc.variable"(){value = unit} : () -> i32
+    %c0 = emitc.variable(unit) : i32
     return
 }
 
@@ -243,7 +243,7 @@ func.func @test_assign_to_non_variable(%arg1: f32, %arg2: f32) {
 // -----
 
 func.func @test_assign_type_mismatch(%arg1: f32) {
-  %v = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> i32
+  %v = emitc.variable(#emitc.opaque<"">) : i32
   // expected-error @+1 {{'emitc.assign' op requires value's type ('f32') to match variable's type ('i32')}}
   emitc.assign %arg1 : f32 to %v : i32
   return
@@ -252,7 +252,7 @@ func.func @test_assign_type_mismatch(%arg1: f32) {
 // -----
 
 func.func @test_assign_to_array(%arg1: !emitc.array<4xi32>) {
-  %v = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.array<4xi32>
+  %v = emitc.variable(#emitc.opaque<"">) : !emitc.array<4xi32>
   // expected-error @+1 {{'emitc.assign' op cannot assign to array type}}
   emitc.assign %arg1 : !emitc.array<4xi32> to %v : !emitc.array<4xi32>
   return
@@ -263,7 +263,7 @@ func.func @test_assign_to_array(%arg1: !emitc.array<4xi32>) {
 func.func @test_expression_no_yield() -> i32 {
   // expected-error @+1 {{'emitc.expression' op must yield a value at termination}}
   %r = emitc.expression : i32 {
-    %c7 = "emitc.constant"(){value = 7 : i32} : () -> i32
+    %c7 = emitc.constant(7 : i32) : i32
   }
   return %r : i32
 }
@@ -273,7 +273,7 @@ func.func @test_expression_no_yield() -> i32 {
 func.func @test_expression_illegal_op(%arg0 : i1) -> i32 {
   // expected-error @+1 {{'emitc.expression' op contains an unsupported operation}}
   %r = emitc.expression : i32 {
-    %x = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> i32
+    %x = emitc.variable(#emitc.opaque<"">) : i32
     emitc.yield %x : i32
   }
   return %r : i32
diff --git a/mlir/test/Dialect/EmitC/invalid_types.mlir b/mlir/test/Dialect/EmitC/invalid_types.mlir
index f9d517bf689b95..a8b7b97aad24aa 100644
--- a/mlir/test/Dialect/EmitC/invalid_types.mlir
+++ b/mlir/test/Dialect/EmitC/invalid_types.mlir
@@ -2,14 +2,14 @@
 
 func.func @illegal_opaque_type_1() {
     // expected-error @+1 {{expected non empty string in !emitc.opaque type}}
-    %1 = "emitc.variable"(){value = "42" : !emitc.opaque<"">} : () -> !emitc.opaque<"mytype">
+    %1 = emitc.variable("42" : !emitc.opaque<"">) : !emitc.opaque<"mytype">
 }
 
 // -----
 
 func.func @illegal_opaque_type_2() {
     // expected-error @+1 {{pointer not allowed as outer type with !emitc.opaque, use !emitc.ptr instead}}
-    %1 = "emitc.variable"(){value = "nullptr" : !emitc.opaque<"int32_t*">} : () -> !emitc.opaque<"int32_t*">
+    %1 = emitc.variable("nullptr" : !emitc.opaque<"int32_t*">) : !emitc.opaque<"int32_t*">
 }
 
 // -----
diff --git a/mlir/test/Dialect/EmitC/ops.mlir b/mlir/test/Dialect/EmitC/ops.mlir
index ace3670426afa5..d41380106a4a30 100644
--- a/mlir/test/Dialect/EmitC/ops.mlir
+++ b/mlir/test/Dialect/EmitC/ops.mlir
@@ -40,7 +40,7 @@ func.func @cast(%arg0: i32) {
 }
 
 func.func @c() {
-  %1 = "emitc.constant"(){value = 42 : i32} : () -> i32
+  %1 = emitc.constant(42 : i32) : i32
   return
 }
 
@@ -170,13 +170,13 @@ func.func @test_if_else(%arg0: i1, %arg1: f32) {
 }
 
 func.func @test_assign(%arg1: f32) {
-  %v = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> f32
+  %v = emitc.variable(#emitc.opaque<"">) : f32
   emitc.assign %arg1 : f32 to %v : f32
   return
 }
 
 func.func @test_expression(%arg0: i32, %arg1: i32, %arg2: i32, %arg3: f32, %arg4: f32) -> i32 {
-  %c7 = "emitc.constant"() {value = 7 : i32} : () -> i32
+  %c7 = emitc.constant(7 : i32) : i32
   %q = emitc.expression : i32 {
     %a = emitc.rem %arg1, %c7 : (i32, i32) -> i32
     emitc.yield %a : i32
diff --git a/mlir/test/Dialect/EmitC/transforms.mlir b/mlir/test/Dialect/EmitC/transforms.mlir
index 8ac606a2c8c0a1..9d496c51996fb3 100644
--- a/mlir/test/Dialect/EmitC/transforms.mlir
+++ b/mlir/test/Dialect/EmitC/transforms.mlir
@@ -2,7 +2,7 @@
 
 // CHECK-LABEL: func.func @single_expression(
 // CHECK-SAME:                               %[[VAL_0:.*]]: i32, %[[VAL_1:.*]]: i32, %[[VAL_2:.*]]: i32, %[[VAL_3:.*]]: i32) -> i1 {
-// CHECK:           %[[VAL_4:.*]] = "emitc.constant"() <{value = 42 : i32}> : () -> i32
+// CHECK:           %[[VAL_4:.*]] = emitc.constant(42 : i32) : i32
 // CHECK:           %[[VAL_5:.*]] = emitc.expression : i1 {
 // CHECK:             %[[VAL_6:.*]] = emitc.mul %[[VAL_0]], %[[VAL_4]] : (i32, i32) -> i32
 // CHECK:             %[[VAL_7:.*]] = emitc.sub %[[VAL_6]], %[[VAL_2]] : (i32, i32) -> i32
@@ -13,7 +13,7 @@
 // CHECK:       }
 
 func.func @single_expression(%arg0: i32, %arg1: i32, %arg2: i32, %arg3: i32) -> i1 {
-  %c42 = "emitc.constant"(){value = 42 : i32} : () -> i32
+  %c42 = emitc.constant (42 : i32) : i32
   %a = emitc.mul %arg0, %c42 : (i32, i32) -> i32
   %b = emitc.sub %a, %arg2 : (i32, i32) -> i32
   %c = emitc.cmp lt, %b, %arg3 :(i32, i32) -> i1
diff --git a/mlir/test/Target/Cpp/call.mlir b/mlir/test/Target/Cpp/call.mlir
index e3ac392f30b625..5481580e54dcdf 100644
--- a/mlir/test/Target/Cpp/call.mlir
+++ b/mlir/test/Target/Cpp/call.mlir
@@ -18,7 +18,7 @@ func.func @emitc_call_opaque() {
 
 
 func.func @emitc_call_opaque_two_results() {
-  %0 = "emitc.constant"() <{value = 0 : index}> : () -> index
+  %0 = emitc.constant(0 : index) : index
   %1:2 = emitc.call_opaque "two_results" () : () -> (i32, i32)
   return
 }
diff --git a/mlir/test/Target/Cpp/const.mlir b/mlir/test/Target/Cpp/const.mlir
index 3658455d669438..a01c9c3445d197 100644
--- a/mlir/test/Target/Cpp/const.mlir
+++ b/mlir/test/Target/Cpp/const.mlir
@@ -2,18 +2,18 @@
 // RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck %s -check-prefix=CPP-DECLTOP
 
 func.func @emitc_constant() {
-  %c0 = "emitc.constant"(){value = #emitc.opaque<"INT_MAX">} : () -> i32
-  %c1 = "emitc.constant"(){value = 42 : i32} : () -> i32
-  %c2 = "emitc.constant"(){value = -1 : i32} : () -> i32
-  %c3 = "emitc.constant"(){value = -1 : si8} : () -> si8
-  %c4 = "emitc.constant"(){value = 255 : ui8} : () -> ui8
-  %c5 = "emitc.constant"(){value = #emitc.opaque<"CHAR_MIN">} : () -> !emitc.opaque<"char">
-  %c6 = "emitc.constant"(){value = 2 : index} : () -> index
-  %c7 = "emitc.constant"(){value = 2.0 : f32} : () -> f32
-  %f64 = "emitc.constant"(){value = 4.0 : f64} : () -> f64
-  %c8 = "emitc.constant"(){value = dense<0> : tensor<i32>} : () -> tensor<i32>
-  %c9 = "emitc.constant"(){value = dense<[0, 1]> : tensor<2xindex>} : () -> tensor<2xindex>
-  %c10 = "emitc.constant"(){value = dense<[[0.0, 1.0], [2.0, 3.0]]> : tensor<2x2xf32>} : () -> tensor<2x2xf32>
+  %c0 = emitc.constant(#emitc.opaque<"INT_MAX">) : i32
+  %c1 = emitc.constant(42 : i32) : i32
+  %c2 = emitc.constant(-1 : i32) : i32
+  %c3 = emitc.constant(-1 : si8) : si8
+  %c4 = emitc.constant(255 : ui8) : ui8
+  %c5 = emitc.constant(#emitc.opaque<"CHAR_MIN">) : !emitc.opaque<"char">
+  %c6 = emitc.constant(2 : index) : index
+  %c7 = emitc.constant(2.0 : f32) : f32
+  %f64 = emitc.constant(4.0 : f64) : f64
+  %c8 = emitc.constant(dense<0> : tensor<i32>) : tensor<i32>
+  %c9 = emitc.constant(dense<[0, 1]> : tensor<2xindex>) : tensor<2xindex>
+  %c10 = emitc.constant(dense<[[0.0, 1.0], [2.0, 3.0]]> : tensor<2x2xf32>) : tensor<2x2xf32>
   return
 }
 // CPP-DEFAULT: void emitc_constant() {
diff --git a/mlir/test/Target/Cpp/expressions.mlir b/mlir/test/Target/Cpp/expressions.mlir
index 9ec9dcc3c6a84b..5e54946480c693 100644
--- a/mlir/test/Target/Cpp/expressions.mlir
+++ b/mlir/test/Target/Cpp/expressions.mlir
@@ -34,7 +34,7 @@ func.func @single_use(%arg0: i32, %arg1: i32, %arg2: i32, %arg3: i32) -> i32 {
     %d = emitc.cmp lt, %c, %arg1 :(i32, i32) -> i1
     emitc.yield %d : i1
   }
-  %v = "emitc.variable"(){value = #emitc.opaque<"">} : () -> i32
+  %v = emitc.variable(#emitc.opaque<"">) : i32
   emitc.if %e {
     emitc.assign %arg0 : i32 to %v : i32
     emitc.yield
@@ -120,7 +120,7 @@ func.func @multiple_uses(%arg0: i32, %arg1: i32, %arg2: i32, %arg3: i32) -> i32
     %d = emitc.cmp lt, %c, %arg1 :(i32, i32) -> i1
     emitc.yield %d : i1
   }
-  %v = "emitc.variable"(){value = #emitc.opaque<"">} : () -> i32
+  %v = emitc.variable(#emitc.opaque<"">) : i32
   emitc.if %e {
     emitc.assign %arg0 : i32 to %v : i32
     emitc.yield
@...
[truncated]

@simon-camp simon-camp marked this pull request as draft April 4, 2024 11:27
@simon-camp
Copy link
Contributor Author

I first tried to mirror the syntax from the arith dialect, let assemblyFormat = "attr-dict $value";.
There are two different cases for the value attribute.

For typed attributes we can parse the attribute and infer the result type from it, just like for the arith dialect.

For emitc opaque attributes we could write a custom parser, but I didn't manage to parse #emitc.opaque<"a"> : i32 as attribute, colon, type, because the type get's consumed and discarded by the attribute parser.

Copy link
Member

@marbre marbre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really appreciate any effort to improve the / add a custom assembly format.
While

  %c5 = emitc.variable(#emitc.opaque<"">) : !emitc.ptr<i32>
  %c6 = emitc.variable(#emitc.opaque<"NULL">) : !emitc.ptr<i32>

looks (nearly) good to me, I would appreciate something similar for non-opaque attributes so we have

  %c3 = emitc.variable -1 : si8
  %c4 = emitc.variable 255 : ui8

instead of

  %c3 = emitc.variable(-1 : si8) : si8
  %c4 = emitc.variable(255 : ui8) : ui8

similar to arith.constant. As discussed offline, this isn't straight forward as opaque attributes have no type and arith.constant uses a TypedAttrInterface.

I am being cautious here, because if we adjust the IR then it should be stable and we won't break it by further updates. So we should see if there really is no better way.

@mgehre-amd
Copy link
Contributor

I really appreciate any effort to improve the / add a custom assembly format. While

  %c5 = emitc.variable(#emitc.opaque<"">) : !emitc.ptr<i32>
  %c6 = emitc.variable(#emitc.opaque<"NULL">) : !emitc.ptr<i32>

looks (nearly) good to me, I would appreciate something similar for non-opaque attributes so we have

  %c3 = emitc.variable -1 : si8
  %c4 = emitc.variable 255 : ui8

instead of

  %c3 = emitc.variable(-1 : si8) : si8
  %c4 = emitc.variable(255 : ui8) : ui8

similar to arith.constant. As discussed offline, this isn't straight forward as opaque attributes have no type and arith.constant uses a TypedAttrInterface.

I am being cautious here, because if we adjust the IR then it should be stable and we won't break it by further updates. So we should see if there really is no better way.

Would this be possible with a custom parse function like in https://github.com/llvm/llvm-project/blob/main/mlir/lib/Dialect/EmitC/IR/EmitC.cpp#L325?

@simon-camp
Copy link
Contributor Author

Would this be possible with a custom parse function like in https://github.com/llvm/llvm-project/blob/main/mlir/lib/Dialect/EmitC/IR/EmitC.cpp#L325?

Altering the syntax by attribute kind should be doable if this is what's suggested here. We could print typed attributes in arith syntax and opaque attributes in llvm syntax (or some other syntax that separates the opaque attribute from the type). Just the arith syntax for opaque attributes won't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants