Avoid unnecessary checks

Author: Jaddyen

mlir/include/mlir/Dialect/EmitC/IR/EmitC.td

@@ -1586,18 +1586,13 @@ def EmitC_ClassOp
     MLIR region, similar to how C++ classes encapsulate their internals.
 
     Example:
+
     ```mlir
-    emitc.func @model(%input_data : !emitc.array<1xf32> {emitc.opaque = ["input_tensor"]}) attributes { } {
-      %0 = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
-      %1 = subscript %input_data[%0] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
-      return
-    }
-    // becomes 
     emitc.class @modelClass {
-      emitc.field @input_tensor : !emitc.array<1xf32> = {emitc.opaque = ["input_tensor"]}
+      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 @input_tensor : !emitc.array<1xf32>
+        %1 = get_field @fieldName0 : !emitc.array<1xf32>
         %2 = subscript %1[%0] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
         return
       }
@@ -1609,8 +1604,6 @@ def EmitC_ClassOp
 
   let regions = (region AnyRegion:$body);
 
-  let builders = [];
-
   let extraClassDeclaration = [{
     // Returns the body block containing class members and methods.
     Block &getBlock();
@@ -1626,29 +1619,21 @@ def EmitC_FieldOp : EmitC_Op<"field", [Symbol]> {
   let description = [{
     The `emitc.field` operation declares a named field within an `emitc.class`
     operation. The field's type must be an EmitC type. 
-    If the corresponding function argument has attributes (accessed via `argAttrs`), 
-    these attributes are attached to the field operation. 
-    Otherwise, the field is created without additional attributes.
 
-    Example of func argument with attributes:
-    ```mlir
-    %arg0: !emitc.array<1xf32> {emitc.opaque = ["another_feature"]} 
-    // becomes
-    emitc.field @another_feature : !emitc.array<1xf32> = {emitc.opaque = ["another_feature"]}
-    ```
-    Example of func argument without attributes:
+    Example:
+
     ```mlir
-    %arg0 : !emitc.array<1xf32>
-    // becomes
+    // Example with an attribute:
+    emitc.field @fieldName0 : !emitc.array<1xf32>  {emitc.opaque = "another_feature"}
+    // Example with no attribute:
     emitc.field @fieldName0 : !emitc.array<1xf32>
     ```
   }];
 
   let arguments = (ins SymbolNameAttr:$sym_name, TypeAttr:$type,
-      OptionalAttr<AnyAttr>:$initial_value);
+      OptionalAttr<AnyAttr>:$attrs);
 
-  let assemblyFormat =
-      [{ $sym_name `:` $type (`=` $initial_value^)? attr-dict}];
+  let assemblyFormat = [{ $sym_name `:` $type ($attrs^)? attr-dict}];
 
   let hasVerifier = 1;
 }

mlir/include/mlir/Dialect/EmitC/Transforms/Passes.td

@@ -26,6 +26,29 @@ def WrapFuncInClassPass : Pass<"wrap-emitc-func-in-class"> {
     This pass transforms `emitc.func` operations into `emitc.class` operations.
     Function arguments become fields of the class, and the function body is moved
     to a new `execute` method within the class.
+    If the corresponding function argument has attributes (accessed via `argAttrs`), 
+    these attributes are attached to the field operation. 
+    Otherwise, the field is created without additional attributes.
+
+    Example:
+    
+    ```mlir
+    emitc.func @model(%input_data : !emitc.array<1xf32> {emitc.opaque = "input_tensor"}) attributes { } {
+      %0 = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
+      %1 = subscript %input_data[%0] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
+      return
+    }
+    // becomes 
+    emitc.class @modelClass {
+      emitc.field @input_tensor : !emitc.array<1xf32> {emitc.opaque = "input_tensor"}
+      emitc.func @execute() {
+        %0 = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
+        %1 = get_field @input_tensor : !emitc.array<1xf32>
+        %2 = subscript %1[%0] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
+        return
+      }
+    }
+    ```
   }];
   let dependentDialects = ["emitc::EmitCDialect"];
   let options = [Option<

mlir/include/mlir/Dialect/EmitC/Transforms/Transforms.h

@@ -30,7 +30,7 @@ void populateExpressionPatterns(RewritePatternSet &patterns);
 
 /// Populates 'patterns' with func-related patterns.
 void populateFuncPatterns(RewritePatternSet &patterns,
-                          const std::string &namedAttribute);
+                          StringRef namedAttribute);
 
 } // namespace emitc
 } // namespace mlir

mlir/lib/Dialect/EmitC/IR/EmitC.cpp

@@ -1404,39 +1404,19 @@ void FileOp::build(OpBuilder &builder, OperationState &state, StringRef id) {
 // FieldOp
 //===----------------------------------------------------------------------===//
 LogicalResult FieldOp::verify() {
-  if (!isSupportedEmitCType(getType())) {
+  if (!isSupportedEmitCType(getType()))
     return emitOpError("expected valid emitc type");
-  }
 
-  if (!getInitialValue()) {
-    return success();
-  }
+  Operation *parentOp = getOperation()->getParentOp();
+  if (!parentOp || !isa<emitc::ClassOp>(parentOp))
+    return emitOpError("field must be nested within an emitc.class operation");
 
-  Attribute initValue = *getInitialValue();
-  // Check that the type of the initial value is compatible with the type of
-  // the global variable.
-  if (ElementsAttr elementsAttr = llvm::dyn_cast<ElementsAttr>(initValue)) {
-    Type initialValueType = elementsAttr.getType();
-    if (!initialValueType) {
-      return emitOpError("initial value attribute must have a type");
-    }
-    Type fieldType = getType();
-    if (initialValueType != fieldType) {
-      if (LValueType lvalueType = dyn_cast<LValueType>(fieldType)) {
-        Type innerFieldType = lvalueType.getValueType();
-        if (innerFieldType != initialValueType) {
-          return emitOpError("initial value type ")
-                 << initialValueType << " is not compatible with field type '"
-                 << fieldType << "' its inner type '" << innerFieldType << "'";
-        }
-
-      } else {
-        return emitOpError("initial value type '")
-               << initialValueType << "' is not compatible with field type '"
-               << fieldType << "'";
-      }
-    }
-  }
+  StringAttr symName = getSymNameAttr();
+  if (!symName || symName.getValue().empty())
+    return emitOpError("field must have a non-empty symbol name");
+
+  if (!getAttrs())
+    return success();
 
   return success();
 }
@@ -1446,27 +1426,19 @@ LogicalResult FieldOp::verify() {
 //===----------------------------------------------------------------------===//
 LogicalResult GetFieldOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
   mlir::FlatSymbolRefAttr fieldNameAttr = getFieldNameAttr();
-  if (!fieldNameAttr) {
-    return emitError("field name attribute is mandatory");
-  }
-
-  StringRef fieldName = fieldNameAttr.getValue();
-
   FieldOp fieldOp =
-      symbolTable.lookupNearestSymbolFrom<FieldOp>(*this, getFieldNameAttr());
-
-  if (!fieldOp) {
-    return emitOpError("field '") << fieldName << "' not found in the class '";
-  }
+      symbolTable.lookupNearestSymbolFrom<FieldOp>(*this, fieldNameAttr);
+  if (!fieldOp)
+    return emitOpError("field '")
+           << fieldNameAttr << "' not found in the class";
 
   Type getFieldResultType = getResult().getType();
   Type fieldType = fieldOp.getType();
 
-  if (fieldType != getFieldResultType) {
+  if (fieldType != getFieldResultType)
     return emitOpError("result type ")
-           << getFieldResultType << " does not match field '" << fieldName
+           << getFieldResultType << " does not match field '" << fieldNameAttr
            << "' type " << fieldType;
-  }
 
   return success();
 }

mlir/lib/Dialect/EmitC/Transforms/WrapFuncInClass.cpp

@@ -1,79 +1,56 @@
-//===- ConvertFuncToClass.cpp - Convert functions to classes -------------===//
+//===- WrapFuncInClass.cpp - Wrap Emitc Funcs in classes -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "mlir-c/Rewrite.h"
 #include "mlir/Dialect/EmitC/IR/EmitC.h"
 #include "mlir/Dialect/EmitC/Transforms/Passes.h"
 #include "mlir/Dialect/EmitC/Transforms/Transforms.h"
 #include "mlir/IR/Attributes.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinAttributes.h"
 #include "mlir/IR/PatternMatch.h"
-#include "mlir/IR/TypeRange.h"
-#include "mlir/IR/Value.h"
-#include "mlir/Pass/Pass.h"
-#include "mlir/Transforms/DialectConversion.h"
-#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/GraphWriter.h"
-#include "llvm/Support/LogicalResult.h"
-#include <string>
+#include "mlir/Transforms/WalkPatternRewriteDriver.h"
+
+using namespace mlir;
+using namespace emitc;
 
 namespace mlir {
 namespace emitc {
-
 #define GEN_PASS_DEF_WRAPFUNCINCLASSPASS
 #include "mlir/Dialect/EmitC/Transforms/Passes.h.inc"
 
 namespace {
-
 struct WrapFuncInClassPass
     : public impl::WrapFuncInClassPassBase<WrapFuncInClassPass> {
   using WrapFuncInClassPassBase::WrapFuncInClassPassBase;
   void runOnOperation() override {
     Operation *rootOp = getOperation();
-    MLIRContext *context = rootOp->getContext();
 
-    RewritePatternSet patterns(context);
+    RewritePatternSet patterns(&getContext());
     populateFuncPatterns(patterns, namedAttribute);
 
-    if (failed(applyPatternsGreedily(rootOp, std::move(patterns))))
-      return signalPassFailure();
-  }
-  void getDependentDialects(DialectRegistry &registry) const override {
-    registry.insert<emitc::EmitCDialect>();
+    walkAndApplyPatterns(rootOp, std::move(patterns));
   }
 };
 
 } // namespace
-
 } // namespace emitc
 } // namespace mlir
 
-using namespace mlir;
-using namespace mlir::emitc;
-
 class WrapFuncInClass : public OpRewritePattern<emitc::FuncOp> {
-private:
-  std::string attributeName;
-
 public:
-  WrapFuncInClass(MLIRContext *context, const std::string &attrName)
+  WrapFuncInClass(MLIRContext *context, StringRef attrName)
       : OpRewritePattern<emitc::FuncOp>(context), attributeName(attrName) {}
 
   LogicalResult matchAndRewrite(emitc::FuncOp funcOp,
                                 PatternRewriter &rewriter) const override {
-    if (funcOp->getParentOfType<emitc::ClassOp>()) {
-      return failure();
-    }
+
     auto className = funcOp.getSymNameAttr().str() + "Class";
-    mlir::emitc::ClassOp newClassOp =
-        rewriter.create<emitc::ClassOp>(funcOp.getLoc(), className);
+    ClassOp newClassOp = rewriter.create<ClassOp>(funcOp.getLoc(), className);
 
     SmallVector<std::pair<StringAttr, TypeAttr>> fields;
     rewriter.createBlock(&newClassOp.getBody());
@@ -84,19 +61,11 @@ class WrapFuncInClass : public OpRewritePattern<emitc::FuncOp> {
       StringAttr fieldName;
       Attribute argAttr = nullptr;
 
+      fieldName = rewriter.getStringAttr("fieldName" + std::to_string(idx));
       if (argAttrs && idx < argAttrs->size()) {
         if (DictionaryAttr dictAttr =
-                dyn_cast<mlir::DictionaryAttr>((*argAttrs)[idx])) {
-          if (auto namedAttr = dictAttr.getNamed(attributeName)) {
-            Attribute nv = namedAttr->getValue();
-            fieldName = cast<mlir::StringAttr>(cast<mlir::ArrayAttr>(nv)[0]);
-            argAttr = (*argAttrs)[idx];
-          }
-        }
-      }
-
-      if (!fieldName) {
-        fieldName = rewriter.getStringAttr("fieldName" + std::to_string(idx));
+                dyn_cast<mlir::DictionaryAttr>((*argAttrs)[idx]))
+          argAttr = (*argAttrs)[idx];
       }
 
       TypeAttr typeAttr = TypeAttr::get(val.getType());
@@ -106,19 +75,17 @@ class WrapFuncInClass : public OpRewritePattern<emitc::FuncOp> {
     }
 
     rewriter.setInsertionPointToEnd(&newClassOp.getBody().front());
-    MLIRContext *funcContext = funcOp.getContext();
-    ArrayRef<Type> inputTypes = funcOp.getFunctionType().getInputs();
-    ArrayRef<Type> results = funcOp.getFunctionType().getResults();
-    FunctionType funcType = FunctionType::get(funcContext, inputTypes, results);
+    FunctionType funcType = funcOp.getFunctionType();
     Location loc = funcOp.getLoc();
-    FuncOp newFuncOp = rewriter.create<emitc::FuncOp>(
-        loc, rewriter.getStringAttr("execute"), funcType);
+    FuncOp newFuncOp =
+        rewriter.create<emitc::FuncOp>(loc, ("execute"), funcType);
 
     rewriter.createBlock(&newFuncOp.getBody());
     newFuncOp.getBody().takeBody(funcOp.getBody());
 
     rewriter.setInsertionPointToStart(&newFuncOp.getBody().front());
     std::vector<Value> newArguments;
+    newArguments.reserve(fields.size());
     for (auto &[fieldName, attr] : fields) {
       GetFieldOp arg =
           rewriter.create<emitc::GetFieldOp>(loc, attr.getValue(), fieldName);
@@ -132,15 +99,18 @@ class WrapFuncInClass : public OpRewritePattern<emitc::FuncOp> {
 
     llvm::BitVector argsToErase(newFuncOp.getNumArguments(), true);
     if (failed(newFuncOp.eraseArguments(argsToErase))) {
-      newFuncOp->emitOpError("Failed to erase all arguments using BitVector.");
+      newFuncOp->emitOpError("failed to erase all arguments using BitVector");
     }
 
     rewriter.replaceOp(funcOp, newClassOp);
     return success();
   }
+
+private:
+  StringRef attributeName;
 };
 
 void mlir::emitc::populateFuncPatterns(RewritePatternSet &patterns,
-                                       const std::string &namedAttribute) {
+                                       StringRef namedAttribute) {
   patterns.add<WrapFuncInClass>(patterns.getContext(), namedAttribute);
 }

mlir/test/Dialect/EmitC/wrap_emitc_func_in_class.mlir

@@ -1,7 +1,9 @@
-// RUN: mlir-opt --wrap-emitc-func-in-class='named-attribute=emitc.opaque' %s | FileCheck %s
+// RUN: mlir-opt --wrap-emitc-func-in-class='named-attribute=emitc.name_hint' %s | FileCheck %s
 
 module attributes { } {
-  emitc.func @model(%arg0: !emitc.array<1xf32> {emitc.opaque = ["another_feature"]}, %arg1: !emitc.array<1xf32> {emitc.opaque = ["some_feature"]}, %arg2: !emitc.array<1xf32> {emitc.opaque = ["output_0"]}) attributes { } {
+  emitc.func @model(%arg0: !emitc.array<1xf32> {emitc.name_hint = "another_feature"}, 
+   %arg1: !emitc.array<1xf32> {emitc.name_hint = "some_feature"},
+   %arg2: !emitc.array<1xf32> {emitc.name_hint = "output_0"}) attributes { } {
     %0 = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
     %1 = subscript %arg1[%0] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
     %2 = load %1 : <f32>
@@ -17,14 +19,14 @@ module attributes { } {
 
 // CHECK: module {
 // CHECK-NEXT:   emitc.class @modelClass {
-// CHECK-NEXT:     emitc.field @another_feature : !emitc.array<1xf32> = {emitc.opaque = ["another_feature"]}
-// CHECK-NEXT:     emitc.field @some_feature : !emitc.array<1xf32> = {emitc.opaque = ["some_feature"]}
-// CHECK-NEXT:     emitc.field @output_0 : !emitc.array<1xf32> = {emitc.opaque = ["output_0"]}
+// CHECK-NEXT:     emitc.field @fieldName0 : !emitc.array<1xf32> {emitc.name_hint = "another_feature"}
+// CHECK-NEXT:     emitc.field @fieldName1 : !emitc.array<1xf32>  {emitc.name_hint = "some_feature"}
+// CHECK-NEXT:     emitc.field @fieldName2 : !emitc.array<1xf32>  {emitc.name_hint = "output_0"}
 // CHECK-NEXT:     emitc.func @execute() {
+// CHECK-NEXT:       get_field @fieldName0 : !emitc.array<1xf32>
+// CHECK-NEXT:       get_field @fieldName1 : !emitc.array<1xf32>
+// CHECK-NEXT:       get_field @fieldName2 : !emitc.array<1xf32>
 // CHECK-NEXT:       "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
-// CHECK-NEXT:       get_field @another_feature : !emitc.array<1xf32>
-// CHECK-NEXT:       get_field @some_feature : !emitc.array<1xf32>
-// CHECK-NEXT:       get_field @output_0 : !emitc.array<1xf32>
 // CHECK-NEXT:       subscript {{.*}}[{{.*}}] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
 // CHECK-NEXT:       load {{.*}} : <f32>
 // CHECK-NEXT:       subscript {{.*}}[{{.*}}] : (!emitc.array<1xf32>, !emitc.size_t) -> !emitc.lvalue<f32>