Skip to content

[mlir][emitc] Clean up EmitC tests #152327

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

Merged
merged 1 commit into from
Aug 11, 2025
Merged

[mlir][emitc] Clean up EmitC tests #152327

merged 1 commit into from
Aug 11, 2025

Conversation

EtoAndruwa
Copy link
Contributor

@EtoAndruwa EtoAndruwa commented Aug 6, 2025

Changes:

  • Unnecessary -verify-diagnostics flags were removed from tests.
  • mlir/test/mlir-translate/emitc_classops.mlir was moved to mlir/test/Target/Cpp/class.mlir.
  • The transfrom.mlir test was renamed to form-expressions.mlir for clarity.
  • Test for emitc.class, emitc.field and emitc.get_field was added to mlir/test/Dialect/EmitC/ops.mlir.
  • wrap_emitc_func_in_class.mlir and wrap_emitc_func_in_class_noAttr.mlir were combined into wrap-func-in-class.mlir.

---------------------------OLD---------------------------
This MR cleans up EmitC source and test files.

Changes:

  • mlir/test/mlir-translate/emitc_classops.mlir was moved to mlir/test/Target/Cpp/class.mlir.
  • mlir/test/Dialect/EmitC/wrap_emitc_func_in_class.mlir and mlir/test/Dialect/EmitC/wrap_emitc_func_in_class_noAttr.mlir were moved to mlir/test/Dialect/EmitC/transforms.mlir.
  • Test for emitc.class, emitc.field and emitc.get_field was added to mlir/test/Dialect/EmitC/ops.mlir.
  • Unnecessary -verify-diagnostics flags were removed from tests.
  • Alphabetical order was restored in mlir/include/mlir/Dialect/EmitC/IR/EmitC.td and mlir/lib/Dialect/EmitC/IR/EmitC.cpp.
  • Case 16 in bool mlir::emitc::isSupportedFloatType(Type type) in EmitC.cpp was refactored to:
case 16:
      if (!llvm::isa<Float16Type, BFloat16Type>(type))
        return false;
      LLVM_FALLTHROUGH;

---------------------------OLD---------------------------

@llvmbot
Copy link
Member

llvmbot commented Aug 6, 2025

@llvm/pr-subscribers-mlir

Author: Andrey Timonin (EtoAndruwa)

Changes

This MR cleans up EmitC source and test files.

Changes:

  • mlir/test/mlir-translate/emitc_classops.mlir was moved to mlir/test/Target/Cpp/class.mlir.
  • mlir/test/Dialect/EmitC/wrap_emitc_func_in_class.mlir and mlir/test/Dialect/EmitC/wrap_emitc_func_in_class_noAttr.mlir were moved to mlir/test/Dialect/EmitC/transforms.mlir.
  • Test for emitc.class, emitc.field and emitc.get_field was added to mlir/test/Dialect/EmitC/ops.mlir.
  • Unnecessary -verify-diagnostics flags were removed from tests.
  • Alphabetical order was restored in mlir/include/mlir/Dialect/EmitC/IR/EmitC.td and mlir/lib/Dialect/EmitC/IR/EmitC.cpp.
  • Case 16 in bool mlir::emitc::isSupportedFloatType(Type type) in EmitC.cpp was refactored to:
case 16:
      if (!llvm::isa&lt;Float16Type, BFloat16Type&gt;(type))
        return false;
      LLVM_FALLTHROUGH;

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

10 Files Affected:

  • (modified) mlir/include/mlir/Dialect/EmitC/IR/EmitC.td (+743-743)
  • (modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+502-498)
  • (modified) mlir/test/Dialect/EmitC/attrs.mlir (+2-2)
  • (modified) mlir/test/Dialect/EmitC/ops.mlir (+12)
  • (modified) mlir/test/Dialect/EmitC/transforms.mlir (+152-95)
  • (modified) mlir/test/Dialect/EmitC/types.mlir (+3-3)
  • (removed) mlir/test/Dialect/EmitC/wrap_emitc_func_in_class.mlir (-40)
  • (removed) mlir/test/Dialect/EmitC/wrap_emitc_func_in_class_noAttr.mlir (-17)
  • (added) mlir/test/Target/Cpp/class.mlir (+78)
  • (removed) mlir/test/mlir-translate/emitc_classops.mlir (-78)
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 937b34a625628..8d5ae60c80452 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -67,52 +67,6 @@ def IntegerIndexOrOpaqueType : Type<CPred<"emitc::isIntegerIndexOrOpaqueType($_s
 "integer, index or opaque type supported by EmitC">;
 def FloatIntegerIndexOrOpaqueType : AnyTypeOf<[EmitCFloatType, IntegerIndexOrOpaqueType]>;
 
-def EmitC_FileOp
-    : EmitC_Op<"file", [IsolatedFromAbove, NoRegionArguments, SymbolTable,
-                        OpAsmOpInterface]#GraphRegionNoTerminator.traits> {
-  let summary = "A file container operation";
-  let description = [{
-    A `file` represents a single C/C++ file.
-
-    `mlir-translate` ignores the body of all `emitc.file` ops
-    unless the `-file-id=id` flag is used. With that flag, all `emitc.file` ops
-    with matching id are emitted.
-
-    Example:
-
-    ```mlir
-    emitc.file "main" {
-      emitc.func @func_one() {
-        emitc.return
-      }
-    }
-    ```
-  }];
-
-  let arguments = (ins Builtin_StringAttr:$id);
-  let regions = (region SizedRegion<1>:$bodyRegion);
-
-  let assemblyFormat = "$id attr-dict-with-keyword $bodyRegion";
-  let builders = [OpBuilder<(ins CArg<"StringRef">:$id)>];
-  let extraClassDeclaration = [{
-    /// Construct a file op from the given location with a name.
-    static FileOp create(Location loc, StringRef name);
-
-    //===------------------------------------------------------------------===//
-    // OpAsmOpInterface Methods
-    //===------------------------------------------------------------------===//
-
-    /// EmitC ops in the body can omit their 'emitc.' prefix in the assembly.
-    static ::llvm::StringRef getDefaultDialect() {
-      return "emitc";
-    }
-  }];
-
-  // We need to ensure that the body region has a block;
-  // the auto-generated builders do not guarantee that.
-  let skipDefaultBuilders = 1;
-}
-
 def EmitC_AddOp : EmitC_BinaryOp<"add", []> {
   let summary = "Addition operation";
   let description = [{
@@ -172,6 +126,35 @@ def EmitC_ApplyOp : EmitC_Op<"apply", [CExpressionInterface]> {
   let hasVerifier = 1;
 }
 
+def EmitC_AssignOp : EmitC_Op<"assign", []> {
+  let summary = "Assign operation";
+  let description = [{
+    The `emitc.assign` operation stores an SSA value to the location designated by an
+    EmitC variable. This operation doesn't return any value. The assigned value
+    must be of the same type as the variable being assigned. The operation is
+    emitted as a C/C++ '=' operator.
+
+    Example:
+
+    ```mlir
+    // Integer variable
+    %0 = "emitc.variable"(){value = 42 : i32} : () -> !emitc.lvalue<i32>
+    %1 = emitc.call_opaque "foo"() : () -> (i32)
+
+    // Assign emitted as `... = ...;`
+    "emitc.assign"(%0, %1) : (!emitc.lvalue<i32>, i32) -> ()
+    ```
+  }];
+
+  let arguments = (ins
+      Res<EmitC_LValueType, "", [MemWrite<DefaultResource, 1, FullEffect>]>:$var,
+      EmitCType:$value);
+  let results = (outs);
+
+  let hasVerifier = 1;
+  let assemblyFormat = "$value `:` type($value) `to` $var `:` type($var) attr-dict";
+}
+
 def EmitC_BitwiseAndOp : EmitC_BinaryOp<"bitwise_and", []> {
   let summary = "Bitwise and operation";
   let description = [{
@@ -280,6 +263,88 @@ def EmitC_BitwiseXorOp : EmitC_BinaryOp<"bitwise_xor", []> {
   }];
 }
 
+def EmitC_CallOp : EmitC_Op<"call",
+    [CallOpInterface, CExpressionInterface,
+     DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
+  let summary = "Call operation";
+  let description = [{
+    The `emitc.call` operation represents a direct call to an `emitc.func`
+    that is within the same symbol scope as the call. The operands and result type
+    of the call must match the specified function type. The callee is encoded as a
+    symbol reference attribute named "callee".
+
+    Example:
+
+    ```mlir
+    %2 = emitc.call @my_add(%0, %1) : (f32, f32) -> f32
+    ```
+  }];
+  let arguments = (ins
+    FlatSymbolRefAttr:$callee,
+    Variadic<EmitCType>:$operands,
+    OptionalAttr<DictArrayAttr>:$arg_attrs,
+    OptionalAttr<DictArrayAttr>:$res_attrs
+  );
+
+  let results = (outs Variadic<EmitCType>);
+
+  let builders = [
+    OpBuilder<(ins "FuncOp":$callee, CArg<"ValueRange", "{}">:$operands), [{
+      $_state.addOperands(operands);
+      $_state.addAttribute("callee", SymbolRefAttr::get(callee));
+      $_state.addTypes(callee.getFunctionType().getResults());
+    }]>,
+    OpBuilder<(ins "SymbolRefAttr":$callee, "TypeRange":$results,
+      CArg<"ValueRange", "{}">:$operands), [{
+      $_state.addOperands(operands);
+      $_state.addAttribute("callee", callee);
+      $_state.addTypes(results);
+    }]>,
+    OpBuilder<(ins "StringAttr":$callee, "TypeRange":$results,
+      CArg<"ValueRange", "{}">:$operands), [{
+      build($_builder, $_state, SymbolRefAttr::get(callee), results, operands);
+    }]>,
+    OpBuilder<(ins "StringRef":$callee, "TypeRange":$results,
+      CArg<"ValueRange", "{}">:$operands), [{
+      build($_builder, $_state, StringAttr::get($_builder.getContext(), callee),
+            results, operands);
+    }]>];
+
+  let extraClassDeclaration = [{
+    FunctionType getCalleeType();
+
+    /// Get the argument operands to the called function.
+    operand_range getArgOperands() {
+      return {arg_operand_begin(), arg_operand_end()};
+    }
+
+    MutableOperandRange getArgOperandsMutable() {
+      return getOperandsMutable();
+    }
+
+    operand_iterator arg_operand_begin() { return operand_begin(); }
+    operand_iterator arg_operand_end() { return operand_end(); }
+
+    /// Return the callee of this operation.
+    CallInterfaceCallable getCallableForCallee() {
+      return (*this)->getAttrOfType<SymbolRefAttr>("callee");
+    }
+
+    /// Set the callee for this operation.
+    void setCalleeFromCallable(CallInterfaceCallable callee) {
+      (*this)->setAttr("callee", cast<SymbolRefAttr>(callee));
+    }
+
+    bool hasSideEffects() {
+      return false;
+    }
+  }];
+
+  let assemblyFormat = [{
+    $callee `(` $operands `)` attr-dict `:` functional-type($operands, results)
+  }];
+}
+
 def EmitC_CallOpaqueOp : EmitC_Op<"call_opaque", [CExpressionInterface]> {
   let summary = "Opaque call operation";
   let description = [{
@@ -399,6 +464,42 @@ def EmitC_CmpOp : EmitC_BinaryOp<"cmp", []> {
   let assemblyFormat = "$predicate `,` operands attr-dict `:` functional-type(operands, results)";
 }
 
+def EmitC_ConditionalOp : EmitC_Op<"conditional",
+    [AllTypesMatch<["true_value", "false_value", "result"]>, CExpressionInterface]> {
+  let summary = "Conditional (ternary) operation";
+  let description = [{
+    With the `emitc.conditional` operation the ternary conditional operator can
+    be applied.
+
+    Example:
+
+    ```mlir
+    %0 = emitc.cmp gt, %arg0, %arg1 : (i32, i32) -> i1
+
+    %c0 = "emitc.constant"() {value = 10 : i32} : () -> i32
+    %c1 = "emitc.constant"() {value = 11 : i32} : () -> i32
+
+    %1 = emitc.conditional %0, %c0, %c1 : i32
+    ```
+    ```c++
+    // Code emitted for the operations above.
+    bool v3 = v1 > v2;
+    int32_t v4 = 10;
+    int32_t v5 = 11;
+    int32_t v6 = v3 ? v4 : v5;
+    ```
+  }];
+  let arguments = (ins I1:$condition, EmitCType:$true_value, EmitCType:$false_value);
+  let results = (outs EmitCType:$result);
+  let assemblyFormat = "operands attr-dict `:` type($result)";
+
+  let extraClassDeclaration = [{
+    bool hasSideEffects() {
+      return false;
+    }
+  }];
+}
+
 def EmitC_ConstantOp : EmitC_Op<"constant", [ConstantLike]> {
   let summary = "Constant operation";
   let description = [{
@@ -428,43 +529,138 @@ def EmitC_ConstantOp : EmitC_Op<"constant", [ConstantLike]> {
   let hasVerifier = 1;
 }
 
-def EmitC_DivOp : EmitC_BinaryOp<"div", []> {
-  let summary = "Division operation";
+def EmitC_ClassOp
+    : EmitC_Op<"class", [AutomaticAllocationScope, IsolatedFromAbove,
+                         OpAsmOpInterface, SymbolTable,
+                         Symbol]#GraphRegionNoTerminator.traits> {
+  let summary =
+      "Represents a C++ class definition, encapsulating fields and methods.";
+
   let description = [{
-    With the `emitc.div` operation the arithmetic operator / (division) can
-    be applied.
+    The `emitc.class` operation defines a C++ class, acting as a container
+    for its data fields (`emitc.field`) and methods (`emitc.func`).
+    It creates a distinct scope, isolating its contents from the surrounding
+    MLIR region, similar to how C++ classes encapsulate their internals.
 
     Example:
 
     ```mlir
-    // Custom form of the division operation.
-    %0 = emitc.div %arg0, %arg1 : (i32, i32) -> i32
-    %1 = emitc.div %arg2, %arg3 : (f32, f32) -> f32
-    ```
-    ```c++
-    // Code emitted for the operations above.
-    int32_t v5 = v1 / v2;
-    float v6 = v3 / v4;
+    emitc.class @modelClass {
+      emitc.field @fieldName0 : !emitc.array<1xf32> = {emitc.opaque = "input_tensor"}
+      emitc.func @execute() {
+        %0 = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
+        %1 = get_field @fieldName0 : !emitc.array<1xf32>
+        %2 = subscript %1[%0] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
+        return
+      }
+    }
+    // Class with a final speciferAdd commentMore actions
+    emitc.class final @modelClass {
+      emitc.field @fieldName0 : !emitc.array<1xf32> = {emitc.opaque = "input_tensor"}
+      emitc.func @execute() {
+        %0 = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
+        %1 = get_field @fieldName0 : !emitc.array<1xf32>
+        %2 = subscript %1[%0] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
+        return
+      }
+    }
     ```
   }];
 
-  let arguments = (ins FloatIntegerIndexOrOpaqueType, FloatIntegerIndexOrOpaqueType);
-  let results = (outs FloatIntegerIndexOrOpaqueType);
-}
+  let arguments = (ins SymbolNameAttr:$sym_name, UnitAttr:$final_specifier);
 
-def EmitC_ExpressionOp : EmitC_Op<"expression",
-      [HasOnlyGraphRegion, OpAsmOpInterface,
-       SingleBlockImplicitTerminator<"emitc::YieldOp">, NoRegionArguments]> {
-  let summary = "Expression operation";
-  let description = [{
-    The `emitc.expression` operation returns a single SSA value which is yielded by
-    its single-basic-block region. The operation doesn't take any arguments.
+  let regions = (region AnyRegion:$body);
 
-    As the operation is to be emitted as a C expression, the operations within
-    its body must form a single Def-Use tree of emitc ops whose result is
-    yielded by a terminating `emitc.yield`.
+  let extraClassDeclaration = [{
+    // Returns the body block containing class members and methods.
+    Block &getBlock();
+  }];
 
-    Example:
+  let hasCustomAssemblyFormat = 1;
+
+  let assemblyFormat =
+      [{ (`final` $final_specifier^)? $sym_name attr-dict-with-keyword $body }];
+}
+
+def EmitC_DeclareFuncOp : EmitC_Op<"declare_func", [
+  DeclareOpInterfaceMethods<SymbolUserOpInterface>
+]> {
+  let summary = "An operation to declare a function";
+  let description = [{
+    The `emitc.declare_func` operation allows to insert a function declaration for an
+    `emitc.func` at a specific position. The operation only requires the "callee"
+    of the `emitc.func` to be specified as an attribute.
+
+    Example:
+
+    ```mlir
+    emitc.declare_func @bar
+    emitc.func @foo(%arg0: i32) -> i32 {
+      %0 = emitc.call @bar(%arg0) : (i32) -> (i32)
+      emitc.return %0 : i32
+    }
+
+    emitc.func @bar(%arg0: i32) -> i32 {
+      emitc.return %arg0 : i32
+    }
+    ```
+
+    ```c++
+    // Code emitted for the operations above.
+    int32_t bar(int32_t v1);
+    int32_t foo(int32_t v1) {
+      int32_t v2 = bar(v1);
+      return v2;
+    }
+
+    int32_t bar(int32_t v1) {
+      return v1;
+    }
+    ```
+  }];
+  let arguments = (ins FlatSymbolRefAttr:$sym_name);
+  let assemblyFormat = [{
+    $sym_name attr-dict
+  }];
+}
+
+def EmitC_DivOp : EmitC_BinaryOp<"div", []> {
+  let summary = "Division operation";
+  let description = [{
+    With the `emitc.div` operation the arithmetic operator / (division) can
+    be applied.
+
+    Example:
+
+    ```mlir
+    // Custom form of the division operation.
+    %0 = emitc.div %arg0, %arg1 : (i32, i32) -> i32
+    %1 = emitc.div %arg2, %arg3 : (f32, f32) -> f32
+    ```
+    ```c++
+    // Code emitted for the operations above.
+    int32_t v5 = v1 / v2;
+    float v6 = v3 / v4;
+    ```
+  }];
+
+  let arguments = (ins FloatIntegerIndexOrOpaqueType, FloatIntegerIndexOrOpaqueType);
+  let results = (outs FloatIntegerIndexOrOpaqueType);
+}
+
+def EmitC_ExpressionOp : EmitC_Op<"expression",
+      [HasOnlyGraphRegion, OpAsmOpInterface,
+       SingleBlockImplicitTerminator<"emitc::YieldOp">, NoRegionArguments]> {
+  let summary = "Expression operation";
+  let description = [{
+    The `emitc.expression` operation returns a single SSA value which is yielded by
+    its single-basic-block region. The operation doesn't take any arguments.
+
+    As the operation is to be emitted as a C expression, the operations within
+    its body must form a single Def-Use tree of emitc ops whose result is
+    yielded by a terminating `emitc.yield`.
+
+    Example:
 
     ```mlir
     %r = emitc.expression : i32 {
@@ -519,6 +715,85 @@ def EmitC_ExpressionOp : EmitC_Op<"expression",
   }];
 }
 
+def EmitC_FieldOp : EmitC_Op<"field", [Symbol]> {
+  let summary = "A field within a class";
+  let description = [{
+    The `emitc.field` operation declares a named field within an `emitc.class`
+    operation. The field's type must be an EmitC type.
+
+    Example:
+
+    ```mlir
+    // Example with an attribute:
+    emitc.field @fieldName0 : !emitc.array<1xf32>  {emitc.opaque = "another_feature"}
+    // Example with no attribute:
+    emitc.field @fieldName0 : !emitc.array<1xf32>
+    // Example with an initial value:
+    emitc.field @fieldName0 : !emitc.array<1xf32> = dense<0.0>
+    // Example with an initial value and attributes:
+    emitc.field @fieldName0 : !emitc.array<1xf32> = dense<0.0> {
+      emitc.opaque = "input_tensor"}
+    ```
+  }];
+
+  let arguments = (ins SymbolNameAttr:$sym_name, TypeAttr:$type,
+      OptionalAttr<EmitC_OpaqueOrTypedAttr>:$initial_value);
+
+  let assemblyFormat = [{
+       $sym_name
+       `:` custom<EmitCFieldOpTypeAndInitialValue>($type, $initial_value)
+       attr-dict
+  }];
+
+  let hasVerifier = 1;
+}
+
+def EmitC_FileOp
+    : EmitC_Op<"file", [IsolatedFromAbove, NoRegionArguments, SymbolTable,
+                        OpAsmOpInterface]#GraphRegionNoTerminator.traits> {
+  let summary = "A file container operation";
+  let description = [{
+    A `file` represents a single C/C++ file.
+
+    `mlir-translate` ignores the body of all `emitc.file` ops
+    unless the `-file-id=id` flag is used. With that flag, all `emitc.file` ops
+    with matching id are emitted.
+
+    Example:
+
+    ```mlir
+    emitc.file "main" {
+      emitc.func @func_one() {
+        emitc.return
+      }
+    }
+    ```
+  }];
+
+  let arguments = (ins Builtin_StringAttr:$id);
+  let regions = (region SizedRegion<1>:$bodyRegion);
+
+  let assemblyFormat = "$id attr-dict-with-keyword $bodyRegion";
+  let builders = [OpBuilder<(ins CArg<"StringRef">:$id)>];
+  let extraClassDeclaration = [{
+    /// Construct a file op from the given location with a name.
+    static FileOp create(Location loc, StringRef name);
+
+    //===------------------------------------------------------------------===//
+    // OpAsmOpInterface Methods
+    //===------------------------------------------------------------------===//
+
+    /// EmitC ops in the body can omit their 'emitc.' prefix in the assembly.
+    static ::llvm::StringRef getDefaultDialect() {
+      return "emitc";
+    }
+  }];
+
+  // We need to ensure that the body region has a block;
+  // the auto-generated builders do not guarantee that.
+  let skipDefaultBuilders = 1;
+}
+
 def EmitC_ForOp : EmitC_Op<"for",
       [AllTypesMatch<["lowerBound", "upperBound", "step"]>,
        OpAsmOpInterface, SingleBlockImplicitTerminator<"emitc::YieldOp">,
@@ -589,172 +864,48 @@ def EmitC_ForOp : EmitC_Op<"for",
   let hasRegionVerifier = 1;
 }
 
-def EmitC_CallOp : EmitC_Op<"call",
-    [CallOpInterface, CExpressionInterface,
-     DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
-  let summary = "Call operation";
+def EmitC_FuncOp : EmitC_Op<"func", [
+  AutomaticAllocationScope,
+  FunctionOpInterface, IsolatedFromAbove, OpAsmOpInterface
+]> {
+  let summary = "An operation with a name containing a single `SSACFG` region";
   let description = [{
-    The `emitc.call` operation represents a direct call to an `emitc.func`
-    that is within the same symbol scope as the call. The operands and result type
-    of the call must match the specified function type. The callee is encoded as a
-    symbol reference attribute named "callee".
+    Operations within the function cannot implicitly capture values defined
+    outside of the function, i.e. Functions are `IsolatedFromAbove`. All
+    external references must use function arguments or attributes that establish
+    a symbolic connection (e.g. symbols referenced by name via a string
+    attribute like SymbolRefAttr). While the MLIR textual form provides a nice
+    inline syntax for function arguments, they are internally represented as
+    “block arguments” to the first block in the region.
+
+    Only dialect attribute names may be specified in the attribute dictionaries
+    for function arguments, results, or the function itself.
 
     Example:
 
     ```mlir
-    %2 = emitc.call @my_add(%0, %1) : (f32, f32) -> f32
-    ```
-  }];
-  let arguments = (ins 
-    FlatSymbolRefAttr:$callee,
-    Variadic<EmitCType>:$operands,
-    OptionalAttr<DictArrayAttr>:$arg_attrs,
-    OptionalAttr<DictArrayAttr>:$res_attrs
-  );
-
-  let results = (outs Variadic<EmitCType>);
-
-  let builders = [
-    OpBuilder<(ins "FuncOp":$callee, CArg<"ValueRange", "{}">:$operands), [{
-      $_state.addOperands(operands);
-      $_state.addAttribute("callee", SymbolRefAttr::get(callee));
-      $_state.addTypes(callee.getFunctionType().getResults());
-    }]>,
-    OpBuilder<(ins "SymbolRefAttr":$callee, "TypeRange":$results,
-      CArg<"ValueRange", "{}">:$operands), [{
-      $_state.addOperands(operands);
-      $_state.addAttribute("callee", callee);
-      $_state.addTypes(results);
-    }]>,
-    OpBuilder<(ins "StringAttr":$callee, "TypeRange":$results,
-      CArg<"ValueRange", "{}">:$operands), [{
-      build($_builder, $_state, SymbolRefAttr::get(callee), results, operands);
-    }]>,
-    OpBuilder<(ins "StringRef":$callee, "TypeRange":$results,
-      CArg<"ValueRange", "{}">:$operands), [{
-      build($_builder, $_state, StringAttr::get($_builder.getContext(), callee),
-            results, operands);
-    }]>];
-
-  let extraClassDeclaration = [{
-    FunctionType getCalleeType();
-
-    /// Get the argument operands to the called function.
-    operand_range getArgOperands() {
-      return {arg_operand_begin(), arg_operand_end()};
-    }
-
-    MutableOperandRange getArgOperandsMutable() {
-      return getOperandsMutable();
+    // A function with no results:
+    emitc.func @foo(%arg0 : i32) {
+      emitc.call_opaque "bar" (%arg0) : (i32) -> ()
+      emitc.return
     }
 
-    operand_iterator arg_operand_begin() { return operand_begin(); }
-    operand_iterator arg_operand_end() { return operand_end(); }
-
-    /// Return the callee of this operation.
-    CallInterfaceCallable getCallableForCallee() {
-      return (*this)->getAttrOfType<SymbolRefAttr>("callee");
+    // A function with its argument as single result:
+    emitc.func @foo(%arg0 : i32) -> i32 {
+      emitc.return %arg0 : i32
     }
 
-    /// Set the callee for this operation.
-    void setCalleeFromCallable(CallInterfaceCallable callee) {
-      (*this)->setAttr("callee", cast<SymbolRefAttr>(callee));
+    // A function with specifiers attribute:
+    emitc.func @example_specifiers_fn_attr() -> i32
+                attributes {specifiers = ["static","inline"]} {
+      %0 = emitc.call_opaque "foo" (): () -> i32
+      emitc.return %0 : i32
     }
 
-    bool hasSideEffects() {
-      return false;
-    }
-  }];
-
-  let assemblyFormat...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Aug 6, 2025

@llvm/pr-subscribers-mlir-emitc

Author: Andrey Timonin (EtoAndruwa)

Changes

This MR cleans up EmitC source and test files.

Changes:

  • mlir/test/mlir-translate/emitc_classops.mlir was moved to mlir/test/Target/Cpp/class.mlir.
  • mlir/test/Dialect/EmitC/wrap_emitc_func_in_class.mlir and mlir/test/Dialect/EmitC/wrap_emitc_func_in_class_noAttr.mlir were moved to mlir/test/Dialect/EmitC/transforms.mlir.
  • Test for emitc.class, emitc.field and emitc.get_field was added to mlir/test/Dialect/EmitC/ops.mlir.
  • Unnecessary -verify-diagnostics flags were removed from tests.
  • Alphabetical order was restored in mlir/include/mlir/Dialect/EmitC/IR/EmitC.td and mlir/lib/Dialect/EmitC/IR/EmitC.cpp.
  • Case 16 in bool mlir::emitc::isSupportedFloatType(Type type) in EmitC.cpp was refactored to:
case 16:
      if (!llvm::isa&lt;Float16Type, BFloat16Type&gt;(type))
        return false;
      LLVM_FALLTHROUGH;

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

10 Files Affected:

  • (modified) mlir/include/mlir/Dialect/EmitC/IR/EmitC.td (+743-743)
  • (modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+502-498)
  • (modified) mlir/test/Dialect/EmitC/attrs.mlir (+2-2)
  • (modified) mlir/test/Dialect/EmitC/ops.mlir (+12)
  • (modified) mlir/test/Dialect/EmitC/transforms.mlir (+152-95)
  • (modified) mlir/test/Dialect/EmitC/types.mlir (+3-3)
  • (removed) mlir/test/Dialect/EmitC/wrap_emitc_func_in_class.mlir (-40)
  • (removed) mlir/test/Dialect/EmitC/wrap_emitc_func_in_class_noAttr.mlir (-17)
  • (added) mlir/test/Target/Cpp/class.mlir (+78)
  • (removed) mlir/test/mlir-translate/emitc_classops.mlir (-78)
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 937b34a625628..8d5ae60c80452 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -67,52 +67,6 @@ def IntegerIndexOrOpaqueType : Type<CPred<"emitc::isIntegerIndexOrOpaqueType($_s
 "integer, index or opaque type supported by EmitC">;
 def FloatIntegerIndexOrOpaqueType : AnyTypeOf<[EmitCFloatType, IntegerIndexOrOpaqueType]>;
 
-def EmitC_FileOp
-    : EmitC_Op<"file", [IsolatedFromAbove, NoRegionArguments, SymbolTable,
-                        OpAsmOpInterface]#GraphRegionNoTerminator.traits> {
-  let summary = "A file container operation";
-  let description = [{
-    A `file` represents a single C/C++ file.
-
-    `mlir-translate` ignores the body of all `emitc.file` ops
-    unless the `-file-id=id` flag is used. With that flag, all `emitc.file` ops
-    with matching id are emitted.
-
-    Example:
-
-    ```mlir
-    emitc.file "main" {
-      emitc.func @func_one() {
-        emitc.return
-      }
-    }
-    ```
-  }];
-
-  let arguments = (ins Builtin_StringAttr:$id);
-  let regions = (region SizedRegion<1>:$bodyRegion);
-
-  let assemblyFormat = "$id attr-dict-with-keyword $bodyRegion";
-  let builders = [OpBuilder<(ins CArg<"StringRef">:$id)>];
-  let extraClassDeclaration = [{
-    /// Construct a file op from the given location with a name.
-    static FileOp create(Location loc, StringRef name);
-
-    //===------------------------------------------------------------------===//
-    // OpAsmOpInterface Methods
-    //===------------------------------------------------------------------===//
-
-    /// EmitC ops in the body can omit their 'emitc.' prefix in the assembly.
-    static ::llvm::StringRef getDefaultDialect() {
-      return "emitc";
-    }
-  }];
-
-  // We need to ensure that the body region has a block;
-  // the auto-generated builders do not guarantee that.
-  let skipDefaultBuilders = 1;
-}
-
 def EmitC_AddOp : EmitC_BinaryOp<"add", []> {
   let summary = "Addition operation";
   let description = [{
@@ -172,6 +126,35 @@ def EmitC_ApplyOp : EmitC_Op<"apply", [CExpressionInterface]> {
   let hasVerifier = 1;
 }
 
+def EmitC_AssignOp : EmitC_Op<"assign", []> {
+  let summary = "Assign operation";
+  let description = [{
+    The `emitc.assign` operation stores an SSA value to the location designated by an
+    EmitC variable. This operation doesn't return any value. The assigned value
+    must be of the same type as the variable being assigned. The operation is
+    emitted as a C/C++ '=' operator.
+
+    Example:
+
+    ```mlir
+    // Integer variable
+    %0 = "emitc.variable"(){value = 42 : i32} : () -> !emitc.lvalue<i32>
+    %1 = emitc.call_opaque "foo"() : () -> (i32)
+
+    // Assign emitted as `... = ...;`
+    "emitc.assign"(%0, %1) : (!emitc.lvalue<i32>, i32) -> ()
+    ```
+  }];
+
+  let arguments = (ins
+      Res<EmitC_LValueType, "", [MemWrite<DefaultResource, 1, FullEffect>]>:$var,
+      EmitCType:$value);
+  let results = (outs);
+
+  let hasVerifier = 1;
+  let assemblyFormat = "$value `:` type($value) `to` $var `:` type($var) attr-dict";
+}
+
 def EmitC_BitwiseAndOp : EmitC_BinaryOp<"bitwise_and", []> {
   let summary = "Bitwise and operation";
   let description = [{
@@ -280,6 +263,88 @@ def EmitC_BitwiseXorOp : EmitC_BinaryOp<"bitwise_xor", []> {
   }];
 }
 
+def EmitC_CallOp : EmitC_Op<"call",
+    [CallOpInterface, CExpressionInterface,
+     DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
+  let summary = "Call operation";
+  let description = [{
+    The `emitc.call` operation represents a direct call to an `emitc.func`
+    that is within the same symbol scope as the call. The operands and result type
+    of the call must match the specified function type. The callee is encoded as a
+    symbol reference attribute named "callee".
+
+    Example:
+
+    ```mlir
+    %2 = emitc.call @my_add(%0, %1) : (f32, f32) -> f32
+    ```
+  }];
+  let arguments = (ins
+    FlatSymbolRefAttr:$callee,
+    Variadic<EmitCType>:$operands,
+    OptionalAttr<DictArrayAttr>:$arg_attrs,
+    OptionalAttr<DictArrayAttr>:$res_attrs
+  );
+
+  let results = (outs Variadic<EmitCType>);
+
+  let builders = [
+    OpBuilder<(ins "FuncOp":$callee, CArg<"ValueRange", "{}">:$operands), [{
+      $_state.addOperands(operands);
+      $_state.addAttribute("callee", SymbolRefAttr::get(callee));
+      $_state.addTypes(callee.getFunctionType().getResults());
+    }]>,
+    OpBuilder<(ins "SymbolRefAttr":$callee, "TypeRange":$results,
+      CArg<"ValueRange", "{}">:$operands), [{
+      $_state.addOperands(operands);
+      $_state.addAttribute("callee", callee);
+      $_state.addTypes(results);
+    }]>,
+    OpBuilder<(ins "StringAttr":$callee, "TypeRange":$results,
+      CArg<"ValueRange", "{}">:$operands), [{
+      build($_builder, $_state, SymbolRefAttr::get(callee), results, operands);
+    }]>,
+    OpBuilder<(ins "StringRef":$callee, "TypeRange":$results,
+      CArg<"ValueRange", "{}">:$operands), [{
+      build($_builder, $_state, StringAttr::get($_builder.getContext(), callee),
+            results, operands);
+    }]>];
+
+  let extraClassDeclaration = [{
+    FunctionType getCalleeType();
+
+    /// Get the argument operands to the called function.
+    operand_range getArgOperands() {
+      return {arg_operand_begin(), arg_operand_end()};
+    }
+
+    MutableOperandRange getArgOperandsMutable() {
+      return getOperandsMutable();
+    }
+
+    operand_iterator arg_operand_begin() { return operand_begin(); }
+    operand_iterator arg_operand_end() { return operand_end(); }
+
+    /// Return the callee of this operation.
+    CallInterfaceCallable getCallableForCallee() {
+      return (*this)->getAttrOfType<SymbolRefAttr>("callee");
+    }
+
+    /// Set the callee for this operation.
+    void setCalleeFromCallable(CallInterfaceCallable callee) {
+      (*this)->setAttr("callee", cast<SymbolRefAttr>(callee));
+    }
+
+    bool hasSideEffects() {
+      return false;
+    }
+  }];
+
+  let assemblyFormat = [{
+    $callee `(` $operands `)` attr-dict `:` functional-type($operands, results)
+  }];
+}
+
 def EmitC_CallOpaqueOp : EmitC_Op<"call_opaque", [CExpressionInterface]> {
   let summary = "Opaque call operation";
   let description = [{
@@ -399,6 +464,42 @@ def EmitC_CmpOp : EmitC_BinaryOp<"cmp", []> {
   let assemblyFormat = "$predicate `,` operands attr-dict `:` functional-type(operands, results)";
 }
 
+def EmitC_ConditionalOp : EmitC_Op<"conditional",
+    [AllTypesMatch<["true_value", "false_value", "result"]>, CExpressionInterface]> {
+  let summary = "Conditional (ternary) operation";
+  let description = [{
+    With the `emitc.conditional` operation the ternary conditional operator can
+    be applied.
+
+    Example:
+
+    ```mlir
+    %0 = emitc.cmp gt, %arg0, %arg1 : (i32, i32) -> i1
+
+    %c0 = "emitc.constant"() {value = 10 : i32} : () -> i32
+    %c1 = "emitc.constant"() {value = 11 : i32} : () -> i32
+
+    %1 = emitc.conditional %0, %c0, %c1 : i32
+    ```
+    ```c++
+    // Code emitted for the operations above.
+    bool v3 = v1 > v2;
+    int32_t v4 = 10;
+    int32_t v5 = 11;
+    int32_t v6 = v3 ? v4 : v5;
+    ```
+  }];
+  let arguments = (ins I1:$condition, EmitCType:$true_value, EmitCType:$false_value);
+  let results = (outs EmitCType:$result);
+  let assemblyFormat = "operands attr-dict `:` type($result)";
+
+  let extraClassDeclaration = [{
+    bool hasSideEffects() {
+      return false;
+    }
+  }];
+}
+
 def EmitC_ConstantOp : EmitC_Op<"constant", [ConstantLike]> {
   let summary = "Constant operation";
   let description = [{
@@ -428,43 +529,138 @@ def EmitC_ConstantOp : EmitC_Op<"constant", [ConstantLike]> {
   let hasVerifier = 1;
 }
 
-def EmitC_DivOp : EmitC_BinaryOp<"div", []> {
-  let summary = "Division operation";
+def EmitC_ClassOp
+    : EmitC_Op<"class", [AutomaticAllocationScope, IsolatedFromAbove,
+                         OpAsmOpInterface, SymbolTable,
+                         Symbol]#GraphRegionNoTerminator.traits> {
+  let summary =
+      "Represents a C++ class definition, encapsulating fields and methods.";
+
   let description = [{
-    With the `emitc.div` operation the arithmetic operator / (division) can
-    be applied.
+    The `emitc.class` operation defines a C++ class, acting as a container
+    for its data fields (`emitc.field`) and methods (`emitc.func`).
+    It creates a distinct scope, isolating its contents from the surrounding
+    MLIR region, similar to how C++ classes encapsulate their internals.
 
     Example:
 
     ```mlir
-    // Custom form of the division operation.
-    %0 = emitc.div %arg0, %arg1 : (i32, i32) -> i32
-    %1 = emitc.div %arg2, %arg3 : (f32, f32) -> f32
-    ```
-    ```c++
-    // Code emitted for the operations above.
-    int32_t v5 = v1 / v2;
-    float v6 = v3 / v4;
+    emitc.class @modelClass {
+      emitc.field @fieldName0 : !emitc.array<1xf32> = {emitc.opaque = "input_tensor"}
+      emitc.func @execute() {
+        %0 = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
+        %1 = get_field @fieldName0 : !emitc.array<1xf32>
+        %2 = subscript %1[%0] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
+        return
+      }
+    }
+    // Class with a final speciferAdd commentMore actions
+    emitc.class final @modelClass {
+      emitc.field @fieldName0 : !emitc.array<1xf32> = {emitc.opaque = "input_tensor"}
+      emitc.func @execute() {
+        %0 = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
+        %1 = get_field @fieldName0 : !emitc.array<1xf32>
+        %2 = subscript %1[%0] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
+        return
+      }
+    }
     ```
   }];
 
-  let arguments = (ins FloatIntegerIndexOrOpaqueType, FloatIntegerIndexOrOpaqueType);
-  let results = (outs FloatIntegerIndexOrOpaqueType);
-}
+  let arguments = (ins SymbolNameAttr:$sym_name, UnitAttr:$final_specifier);
 
-def EmitC_ExpressionOp : EmitC_Op<"expression",
-      [HasOnlyGraphRegion, OpAsmOpInterface,
-       SingleBlockImplicitTerminator<"emitc::YieldOp">, NoRegionArguments]> {
-  let summary = "Expression operation";
-  let description = [{
-    The `emitc.expression` operation returns a single SSA value which is yielded by
-    its single-basic-block region. The operation doesn't take any arguments.
+  let regions = (region AnyRegion:$body);
 
-    As the operation is to be emitted as a C expression, the operations within
-    its body must form a single Def-Use tree of emitc ops whose result is
-    yielded by a terminating `emitc.yield`.
+  let extraClassDeclaration = [{
+    // Returns the body block containing class members and methods.
+    Block &getBlock();
+  }];
 
-    Example:
+  let hasCustomAssemblyFormat = 1;
+
+  let assemblyFormat =
+      [{ (`final` $final_specifier^)? $sym_name attr-dict-with-keyword $body }];
+}
+
+def EmitC_DeclareFuncOp : EmitC_Op<"declare_func", [
+  DeclareOpInterfaceMethods<SymbolUserOpInterface>
+]> {
+  let summary = "An operation to declare a function";
+  let description = [{
+    The `emitc.declare_func` operation allows to insert a function declaration for an
+    `emitc.func` at a specific position. The operation only requires the "callee"
+    of the `emitc.func` to be specified as an attribute.
+
+    Example:
+
+    ```mlir
+    emitc.declare_func @bar
+    emitc.func @foo(%arg0: i32) -> i32 {
+      %0 = emitc.call @bar(%arg0) : (i32) -> (i32)
+      emitc.return %0 : i32
+    }
+
+    emitc.func @bar(%arg0: i32) -> i32 {
+      emitc.return %arg0 : i32
+    }
+    ```
+
+    ```c++
+    // Code emitted for the operations above.
+    int32_t bar(int32_t v1);
+    int32_t foo(int32_t v1) {
+      int32_t v2 = bar(v1);
+      return v2;
+    }
+
+    int32_t bar(int32_t v1) {
+      return v1;
+    }
+    ```
+  }];
+  let arguments = (ins FlatSymbolRefAttr:$sym_name);
+  let assemblyFormat = [{
+    $sym_name attr-dict
+  }];
+}
+
+def EmitC_DivOp : EmitC_BinaryOp<"div", []> {
+  let summary = "Division operation";
+  let description = [{
+    With the `emitc.div` operation the arithmetic operator / (division) can
+    be applied.
+
+    Example:
+
+    ```mlir
+    // Custom form of the division operation.
+    %0 = emitc.div %arg0, %arg1 : (i32, i32) -> i32
+    %1 = emitc.div %arg2, %arg3 : (f32, f32) -> f32
+    ```
+    ```c++
+    // Code emitted for the operations above.
+    int32_t v5 = v1 / v2;
+    float v6 = v3 / v4;
+    ```
+  }];
+
+  let arguments = (ins FloatIntegerIndexOrOpaqueType, FloatIntegerIndexOrOpaqueType);
+  let results = (outs FloatIntegerIndexOrOpaqueType);
+}
+
+def EmitC_ExpressionOp : EmitC_Op<"expression",
+      [HasOnlyGraphRegion, OpAsmOpInterface,
+       SingleBlockImplicitTerminator<"emitc::YieldOp">, NoRegionArguments]> {
+  let summary = "Expression operation";
+  let description = [{
+    The `emitc.expression` operation returns a single SSA value which is yielded by
+    its single-basic-block region. The operation doesn't take any arguments.
+
+    As the operation is to be emitted as a C expression, the operations within
+    its body must form a single Def-Use tree of emitc ops whose result is
+    yielded by a terminating `emitc.yield`.
+
+    Example:
 
     ```mlir
     %r = emitc.expression : i32 {
@@ -519,6 +715,85 @@ def EmitC_ExpressionOp : EmitC_Op<"expression",
   }];
 }
 
+def EmitC_FieldOp : EmitC_Op<"field", [Symbol]> {
+  let summary = "A field within a class";
+  let description = [{
+    The `emitc.field` operation declares a named field within an `emitc.class`
+    operation. The field's type must be an EmitC type.
+
+    Example:
+
+    ```mlir
+    // Example with an attribute:
+    emitc.field @fieldName0 : !emitc.array<1xf32>  {emitc.opaque = "another_feature"}
+    // Example with no attribute:
+    emitc.field @fieldName0 : !emitc.array<1xf32>
+    // Example with an initial value:
+    emitc.field @fieldName0 : !emitc.array<1xf32> = dense<0.0>
+    // Example with an initial value and attributes:
+    emitc.field @fieldName0 : !emitc.array<1xf32> = dense<0.0> {
+      emitc.opaque = "input_tensor"}
+    ```
+  }];
+
+  let arguments = (ins SymbolNameAttr:$sym_name, TypeAttr:$type,
+      OptionalAttr<EmitC_OpaqueOrTypedAttr>:$initial_value);
+
+  let assemblyFormat = [{
+       $sym_name
+       `:` custom<EmitCFieldOpTypeAndInitialValue>($type, $initial_value)
+       attr-dict
+  }];
+
+  let hasVerifier = 1;
+}
+
+def EmitC_FileOp
+    : EmitC_Op<"file", [IsolatedFromAbove, NoRegionArguments, SymbolTable,
+                        OpAsmOpInterface]#GraphRegionNoTerminator.traits> {
+  let summary = "A file container operation";
+  let description = [{
+    A `file` represents a single C/C++ file.
+
+    `mlir-translate` ignores the body of all `emitc.file` ops
+    unless the `-file-id=id` flag is used. With that flag, all `emitc.file` ops
+    with matching id are emitted.
+
+    Example:
+
+    ```mlir
+    emitc.file "main" {
+      emitc.func @func_one() {
+        emitc.return
+      }
+    }
+    ```
+  }];
+
+  let arguments = (ins Builtin_StringAttr:$id);
+  let regions = (region SizedRegion<1>:$bodyRegion);
+
+  let assemblyFormat = "$id attr-dict-with-keyword $bodyRegion";
+  let builders = [OpBuilder<(ins CArg<"StringRef">:$id)>];
+  let extraClassDeclaration = [{
+    /// Construct a file op from the given location with a name.
+    static FileOp create(Location loc, StringRef name);
+
+    //===------------------------------------------------------------------===//
+    // OpAsmOpInterface Methods
+    //===------------------------------------------------------------------===//
+
+    /// EmitC ops in the body can omit their 'emitc.' prefix in the assembly.
+    static ::llvm::StringRef getDefaultDialect() {
+      return "emitc";
+    }
+  }];
+
+  // We need to ensure that the body region has a block;
+  // the auto-generated builders do not guarantee that.
+  let skipDefaultBuilders = 1;
+}
+
 def EmitC_ForOp : EmitC_Op<"for",
       [AllTypesMatch<["lowerBound", "upperBound", "step"]>,
        OpAsmOpInterface, SingleBlockImplicitTerminator<"emitc::YieldOp">,
@@ -589,172 +864,48 @@ def EmitC_ForOp : EmitC_Op<"for",
   let hasRegionVerifier = 1;
 }
 
-def EmitC_CallOp : EmitC_Op<"call",
-    [CallOpInterface, CExpressionInterface,
-     DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
-  let summary = "Call operation";
+def EmitC_FuncOp : EmitC_Op<"func", [
+  AutomaticAllocationScope,
+  FunctionOpInterface, IsolatedFromAbove, OpAsmOpInterface
+]> {
+  let summary = "An operation with a name containing a single `SSACFG` region";
   let description = [{
-    The `emitc.call` operation represents a direct call to an `emitc.func`
-    that is within the same symbol scope as the call. The operands and result type
-    of the call must match the specified function type. The callee is encoded as a
-    symbol reference attribute named "callee".
+    Operations within the function cannot implicitly capture values defined
+    outside of the function, i.e. Functions are `IsolatedFromAbove`. All
+    external references must use function arguments or attributes that establish
+    a symbolic connection (e.g. symbols referenced by name via a string
+    attribute like SymbolRefAttr). While the MLIR textual form provides a nice
+    inline syntax for function arguments, they are internally represented as
+    “block arguments” to the first block in the region.
+
+    Only dialect attribute names may be specified in the attribute dictionaries
+    for function arguments, results, or the function itself.
 
     Example:
 
     ```mlir
-    %2 = emitc.call @my_add(%0, %1) : (f32, f32) -> f32
-    ```
-  }];
-  let arguments = (ins 
-    FlatSymbolRefAttr:$callee,
-    Variadic<EmitCType>:$operands,
-    OptionalAttr<DictArrayAttr>:$arg_attrs,
-    OptionalAttr<DictArrayAttr>:$res_attrs
-  );
-
-  let results = (outs Variadic<EmitCType>);
-
-  let builders = [
-    OpBuilder<(ins "FuncOp":$callee, CArg<"ValueRange", "{}">:$operands), [{
-      $_state.addOperands(operands);
-      $_state.addAttribute("callee", SymbolRefAttr::get(callee));
-      $_state.addTypes(callee.getFunctionType().getResults());
-    }]>,
-    OpBuilder<(ins "SymbolRefAttr":$callee, "TypeRange":$results,
-      CArg<"ValueRange", "{}">:$operands), [{
-      $_state.addOperands(operands);
-      $_state.addAttribute("callee", callee);
-      $_state.addTypes(results);
-    }]>,
-    OpBuilder<(ins "StringAttr":$callee, "TypeRange":$results,
-      CArg<"ValueRange", "{}">:$operands), [{
-      build($_builder, $_state, SymbolRefAttr::get(callee), results, operands);
-    }]>,
-    OpBuilder<(ins "StringRef":$callee, "TypeRange":$results,
-      CArg<"ValueRange", "{}">:$operands), [{
-      build($_builder, $_state, StringAttr::get($_builder.getContext(), callee),
-            results, operands);
-    }]>];
-
-  let extraClassDeclaration = [{
-    FunctionType getCalleeType();
-
-    /// Get the argument operands to the called function.
-    operand_range getArgOperands() {
-      return {arg_operand_begin(), arg_operand_end()};
-    }
-
-    MutableOperandRange getArgOperandsMutable() {
-      return getOperandsMutable();
+    // A function with no results:
+    emitc.func @foo(%arg0 : i32) {
+      emitc.call_opaque "bar" (%arg0) : (i32) -> ()
+      emitc.return
     }
 
-    operand_iterator arg_operand_begin() { return operand_begin(); }
-    operand_iterator arg_operand_end() { return operand_end(); }
-
-    /// Return the callee of this operation.
-    CallInterfaceCallable getCallableForCallee() {
-      return (*this)->getAttrOfType<SymbolRefAttr>("callee");
+    // A function with its argument as single result:
+    emitc.func @foo(%arg0 : i32) -> i32 {
+      emitc.return %arg0 : i32
     }
 
-    /// Set the callee for this operation.
-    void setCalleeFromCallable(CallInterfaceCallable callee) {
-      (*this)->setAttr("callee", cast<SymbolRefAttr>(callee));
+    // A function with specifiers attribute:
+    emitc.func @example_specifiers_fn_attr() -> i32
+                attributes {specifiers = ["static","inline"]} {
+      %0 = emitc.call_opaque "foo" (): () -> i32
+      emitc.return %0 : i32
     }
 
-    bool hasSideEffects() {
-      return false;
-    }
-  }];
-
-  let assemblyFormat...
[truncated]

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.

Thanks for taking a shot at cleaning up.

I don't think this a NFC with +1,492 −1,476 lines changed as it isn't easy to review and furthermore makes it harder to find the original commits and their commit message which often provide additional details.

The following is

  • mlir/test/mlir-translate/emitc_classops.mlir was moved to mlir/test/Target/Cpp/class.mlir.

  • Test for emitc.class, emitc.field and emitc.get_field was added to mlir/test/Dialect/EmitC/ops.mlir.

  • Unnecessary -verify-diagnostics flags were removed from tests.

is not controversial from my point of view and can be included in a single commit.

To follow https://llvm.org/docs/Contributing.html#how-to-submit-a-patch more closely

  • not contain any unrelated changes
  • be an isolated change. Independent changes should be submitted as separate patches as this makes reviewing easier.

the change

  • Case 16 in bool mlir::emitc::isSupportedFloatType(Type type) in EmitC.cpp was refactored to:

could be submitted on its own even though a one line change.

I don't have a strong opinion on:

  • mlir/test/Dialect/EmitC/wrap_emitc_func_in_class.mlir and mlir/test/Dialect/EmitC/wrap_emitc_func_in_class_noAttr.mlir were moved to mlir/test/Dialect/EmitC/transforms.mlir.

but I think

  • Alphabetical order was restored in mlir/include/mlir/Dialect/EmitC/IR/EmitC.td and mlir/lib/Dialect/EmitC/IR/EmitC.cpp.

should be dropped as there is not much benefit compared to the disadvantage.

@EtoAndruwa
Copy link
Contributor Author

EtoAndruwa commented Aug 7, 2025

Thank you for the review!

I don't have a strong opinion on:
"mlir/test/Dialect/EmitC/wrap_emitc_func_in_class.mlir and mlir/test/Dialect/EmitC/wrap_emitc_func_in_class_noAttr.mlir were moved to mlir/test/Dialect/EmitC/transforms.mlir."

  • Perhaps we should rename transfrom.mlir to form-expressions.mlir, since it is not related to the transform dialect and does not use the --transform-interpreter option. This naming better reflects the purpose of the test, in my opinion.

  • Both wrap_emitc_func_in_class.mlir and wrap_emitc_func_in_class_noAttr.mlir can be combined into wrap-func-in-class.mlir. This would avoid creating extra files while still achieving the same result.

@EtoAndruwa
Copy link
Contributor Author

the change
Case 16 in bool mlir::emitc::isSupportedFloatType(Type type) in EmitC.cpp was refactored to:

OK, I will prepare separate patch for this change.

@EtoAndruwa
Copy link
Contributor Author

the change
Case 16 in bool mlir::emitc::isSupportedFloatType(Type type) in EmitC.cpp was refactored to:

OK, I will prepare separate patch for this change.

The appropriate patch #152464.

Changes:
    - Unnecessary `-verify-diagnostics` flags were removed from tests.
    - `mlir/test/mlir-translate/emitc_classops.mlir` was moved to
      `mlir/test/Target/Cpp/class.mlir`.
    - The `transfrom.mlir` test was renamed to `form-expressions.mlir`
      for clarity.
    - Test for `emitc.class`, `emitc.field` and `emitc.get_field` was
      added to `mlir/test/Dialect/EmitC/ops.mlir`.
    - `wrap_emitc_func_in_class.mlir` and `wrap_emitc_func_in_class_noAttr.mlir`
      were combined into `wrap-func-in-class.mlir`.
@EtoAndruwa EtoAndruwa changed the title [mlir][emitc][NFC] Clean up EmitC [mlir][emitc] Clean up EmitC tests Aug 7, 2025
@EtoAndruwa EtoAndruwa requested a review from marbre August 7, 2025 09:58
Copy link
Contributor

@jacquesguan jacquesguan left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Thanks for the clean up!

@marbre marbre merged commit d1e6923 into llvm:main Aug 11, 2025
9 checks passed
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