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

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();

lib/Firtool/Firtool.cpp

@@ -297,7 +297,6 @@ LogicalResult firtool::populateLowFIRRTLToHW(mlir::PassManager &pm,
   pm.nest<firrtl::CircuitOp>().addPass(firrtl::createLowerDPI());
   pm.nest<firrtl::CircuitOp>().addPass(firrtl::createLowerDomains());
   pm.nest<firrtl::CircuitOp>().addPass(firrtl::createLowerClasses());
-  pm.nest<firrtl::CircuitOp>().addPass(om::createVerifyObjectFields());
 
   // Check for static asserts.
   pm.nest<firrtl::CircuitOp>().addPass(circt::firrtl::createLint(
@@ -317,9 +316,6 @@ LogicalResult firtool::populateLowFIRRTLToHW(mlir::PassManager &pm,
   // Check inner symbols and inner refs.
   pm.addPass(hw::createVerifyInnerRefNamespace());
 
-  // Check OM object fields.
-  pm.addPass(om::createVerifyObjectFields());
-
   // Run the verif op verification pass
   pm.addNestedPass<hw::HWModuleOp>(verif::createVerifyClockedAssertLikePass());
 
@@ -369,9 +365,6 @@ LogicalResult firtool::populateHWToSV(mlir::PassManager &pm,
   // Check inner symbols and inner refs.
   pm.addPass(hw::createVerifyInnerRefNamespace());
 
-  // Check OM object fields.
-  pm.addPass(om::createVerifyObjectFields());
-
   return success();
 }
 
@@ -408,9 +401,6 @@ populatePrepareForExportVerilog(mlir::PassManager &pm,
   // Check inner symbols and inner refs.
   pm.addPass(hw::createVerifyInnerRefNamespace());
 
-  // Check OM object fields.
-  pm.addPass(om::createVerifyObjectFields());
-
   return success();
 }
 } // namespace detail