[OM] Clean up ObjectOfieldOp: Move away from symbols, remove a field path and VerifyObjectFields pass (#10303)

Replace FlatSymbolRefArrayAttr with StringAttr for field names, changing
syntax from 'object, [@field]' to 'object["field"]'. Nested field access
now requires chaining ObjectFieldOp instead of using an array path.

This moves verification from the VerifyObjectFields pass into the
ObjectFieldOp verifier itself by adding SymbolUserOpInterface, eliminating
the need for a separate pass. The Evaluator is updated to handle StringAttr
and dereference ReferenceValue.

Author: uenoku

include/circt/Dialect/OM/OMOps.td

@@ -238,18 +238,21 @@ def ElaboratedObjectOp : OMOp<"elaborated_object", [
   }];
 }
 
-def ObjectFieldOp : OMOp<"object.field", [Pure]> {
+def ObjectFieldOp : OMOp<"object.field", [
+    Pure,
+    DeclareOpInterfaceMethods<SymbolUserOpInterface>
+  ]> {
   let arguments = (ins
     ClassType:$object,
-    FlatSymbolRefArrayAttr:$fieldPath
+    StrAttr:$field
   );
 
   let results = (outs
     AnyType:$result
   );
 
   let assemblyFormat = [{
-    $object `,` $fieldPath attr-dict `:` functional-type($object, $result)
+    $object `[` $field `]` attr-dict `:` functional-type($object, $result)
   }];
 }
 

include/circt/Dialect/OM/OMPasses.td

@@ -25,13 +25,6 @@ def LinkModules: Pass<"om-link-modules", "mlir::ModuleOp"> {
   }];
 }
 
-def VerifyObjectFields: Pass<"om-verify-object-fields"> {
-  let summary = "Verify fields of ObjectOp are valid";
-  let description = [{
-    Verify object fields are valid.
-  }];
-}
-
 def StripOMPass : Pass<"strip-om", "mlir::ModuleOp"> {
   let summary = "Remove OM information";
   let description = [{

integration_test/Bindings/Python/dialects/om.py

@@ -88,10 +88,10 @@
 
     om.class @IntegerBinaryArithmeticObjectsDelayed() -> (result: !om.integer) {
       %0 = om.object @Class1(%5) : (!om.integer) -> !om.class.type<@Class1>
-      %1 = om.object.field %0, [@value] : (!om.class.type<@Class1>) -> !om.integer
+      %1 = om.object.field %0["value"] : (!om.class.type<@Class1>) -> !om.integer
 
       %2 = om.object @Class2() : () -> !om.class.type<@Class2>
-      %3 = om.object.field %2, [@value] : (!om.class.type<@Class2>) -> !om.integer
+      %3 = om.object.field %2["value"] : (!om.class.type<@Class2>) -> !om.integer
 
       %5 = om.integer.add %1, %3 : !om.integer
       om.class.fields %5 : !om.integer
@@ -152,13 +152,13 @@
 print(obj.reference)
 
 for (name, field) in obj:
-  # location from om.class.field @child, %0 : !om.class.type<@Child>
+  # location from om.class.field "child"
   # CHECK: name: child, field: <circt.dialects.om.Object object
   # CHECK-SAME: loc: loc("-":{{.*}}:{{.*}})
-  # location from om.class.field @field, %param : !om.integer
+  # location from om.class.field "field"
   # CHECK: name: field, field: 42
   # CHECK-SAME: loc: loc("-":{{.*}}:{{.*}})
-  # location from om.class.field @reference, %sym : !om.ref
+  # location from om.class.field "reference"
   # CHECK: name: reference, field: ('Root', 'x')
   # CHECK-SAME: loc: loc("-":{{.*}}:{{.*}})
   loc = obj.get_field_loc(name)

lib/Dialect/FIRRTL/Transforms/LowerClasses.cpp

@@ -1463,13 +1463,10 @@ updateInstanceInClass(InstanceOp firrtlInstance, hw::HierPathOp hierPath,
     if (!isa<PropertyType>(result.getType()))
       continue;
 
-    // The path to the field is just this output's name.
-    auto objectFieldPath = builder.getArrayAttr({FlatSymbolRefAttr::get(
-        firrtlInstance.getPortNameAttr(result.getResultNumber()))});
-
     // Create the field access.
     auto objectField = om::ObjectFieldOp::create(
-        builder, object.getLoc(), result.getType(), object, objectFieldPath);
+        builder, object.getLoc(), result.getType(), object,
+        firrtlInstance.getPortNameAttr(result.getResultNumber()));
 
     result.replaceAllUsesWith(objectField);
   }
@@ -1967,11 +1964,9 @@ struct ObjectSubfieldOpConversion
     if (element.direction == Direction::In)
       return failure();
 
-    auto field = FlatSymbolRefAttr::get(element.name);
-    auto path = rewriter.getArrayAttr({field});
     auto type = typeConverter->convertType(element.type);
     rewriter.replaceOpWithNewOp<om::ObjectFieldOp>(op, type, adaptor.getInput(),
-                                                   path);
+                                                   element.name);
     return success();
   }
 
@@ -2067,7 +2062,7 @@ struct ObjectFieldOpConversion : public OpConversionPattern<om::ObjectFieldOp> {
       return failure();
 
     rewriter.replaceOpWithNewOp<om::ObjectFieldOp>(
-        op, type, adaptor.getObject(), adaptor.getFieldPathAttr());
+        op, type, adaptor.getObject(), adaptor.getFieldAttr());
 
     return success();
   }

lib/Dialect/OM/Evaluator/Evaluator.cpp

@@ -739,22 +739,24 @@ circt::om::Evaluator::evaluateObjectField(ObjectFieldOp op,
     return objectFieldValue;
   }
 
+  // If the result is a ReferenceValue, dereference it to get the actual object.
+  if (auto *ref = llvm::dyn_cast<evaluator::ReferenceValue>(result.get())) {
+    auto stripped = ref->getStrippedValue();
+    if (failed(stripped))
+      return failure();
+    result = stripped.value();
+  }
+
   auto *currentObject = llvm::cast<evaluator::ObjectValue>(result.get());
 
-  // Iteratively access nested fields through the path until we reach the final
-  // field in the path.
-  evaluator::EvaluatorValuePtr finalField;
-  for (auto field : op.getFieldPath().getAsRange<FlatSymbolRefAttr>()) {
-    // `currentObject` might no be fully evaluated.
-    if (!currentObject->getFields().contains(field.getAttr()))
-      return objectFieldValue;
-
-    auto currentField = currentObject->getField(field.getAttr());
-    finalField = currentField.value();
-    if (auto *nextObject =
-            llvm::dyn_cast<evaluator::ObjectValue>(finalField.get()))
-      currentObject = nextObject;
-  }
+  auto field = op.getFieldAttr();
+
+  // `currentObject` might not be fully evaluated.
+  if (!currentObject->getFields().contains(field))
+    return objectFieldValue;
+
+  auto currentField = currentObject->getField(field);
+  auto finalField = currentField.value();
 
   if (!finalField->isFullyEvaluated())
     return objectFieldValue;

lib/Dialect/OM/OMOps.cpp

@@ -579,6 +579,38 @@ circt::om::ObjectOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
   return success();
 }
 
+//===----------------------------------------------------------------------===//
+// ObjectFieldOp
+//===----------------------------------------------------------------------===//
+
+LogicalResult
+circt::om::ObjectFieldOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
+  auto classType = getObject().getType();
+  auto className = classType.getClassName().getAttr();
+
+  // Verify the referred-to class exists.
+  auto classDef = dyn_cast_or_null<ClassLike>(
+      symbolTable.lookupNearestSymbolFrom(*this, className));
+  if (!classDef)
+    return emitOpError("class ") << className << " was not found";
+
+  // Verify the field exists in the class.
+  auto fieldName = getFieldAttr();
+  std::optional<Type> fieldType = classDef.getFieldType(fieldName);
+  if (!fieldType) {
+    auto diag = emitOpError("referenced non-existent field ") << fieldName;
+    diag.attachNote(classDef.getLoc()) << "class defined here";
+    return diag;
+  }
+
+  // Verify the result type matches the field type.
+  if (getResult().getType() != fieldType.value())
+    return emitOpError("expected type ")
+           << getResult().getType() << ", but accessed field has type "
+           << fieldType.value();
+  return success();
+}
+
 //===----------------------------------------------------------------------===//
 // ElaboratedObjectOp
 //===----------------------------------------------------------------------===//

lib/Dialect/OM/OMReductions.cpp

@@ -156,12 +156,8 @@ struct OMClassFieldPruner : public OpReduction<ClassOp> {
         if (!fieldOp)
           continue;
 
-        auto fieldPath = fieldOp.getFieldPath();
-        if (fieldPath.empty())
-          continue;
-
         // Mark the accessed field as used.
-        usedFields.insert(cast<FlatSymbolRefAttr>(fieldPath[0]).getAttr());
+        usedFields.insert(fieldOp.getFieldAttr());
       }
     });
 

lib/Dialect/OM/Transforms/CMakeLists.txt

@@ -2,7 +2,6 @@ add_circt_dialect_library(CIRCTOMTransforms
   FreezePaths.cpp
   LinkModules.cpp
   StripOM.cpp
-  VerifyObjectFields.cpp
 
   DEPENDS
   CIRCTOMTransformsIncGen

lib/Dialect/OM/Transforms/FreezePaths.cpp

@@ -292,7 +292,7 @@ LogicalResult PathVisitor::process(ObjectFieldOp objectFieldOp) {
   OpBuilder builder(objectFieldOp);
   auto newObjectFieldOp = ObjectFieldOp::create(
       builder, objectFieldOp.getLoc(), newResultType, objectFieldOp.getObject(),
-      objectFieldOp.getFieldPath());
+      objectFieldOp.getFieldAttr());
   objectFieldOp.replaceAllUsesWith(newObjectFieldOp.getResult());
   objectFieldOp->erase();
   return success();