[FIRRTL] Initial support for classes and objects in Dedup.

lib/Dialect/FIRRTL/Transforms/Dedup.cpp

@@ -159,6 +159,15 @@ struct StructuralHasher {
 
   void update(OpResult result) {
     record(result.getAsOpaquePointer());
+
+    // Like instance ops, don't use object ops' result types since they might be
+    // replaced by dedup. Record the class names and lazily combine their hashes
+    // using the same mechanism as instances and modules.
+    if (auto objectOp = dyn_cast<ObjectOp>(result.getOwner())) {
+      referredModuleNames.push_back(objectOp.getType().getNameAttr().getAttr());
+      return;
+    }
+
     update(result.getType());
   }
 
@@ -247,6 +256,11 @@ struct StructuralHasher {
       // Hash the interned pointer.
       update(name.getAsOpaquePointer());
 
+      // TODO: properly handle DistinctAttr, including its use in paths.
+      // See https://github.com/llvm/circt/issues/6583.
+      if (isa<DistinctAttr>(value))
+        continue;
+
       // If this is an symbol reference, we need to perform name erasure.
       if (auto innerRef = dyn_cast<hw::InnerRefAttr>(value))
         update(innerRef);
@@ -588,6 +602,9 @@ struct Equivalence {
         if (failed(check(diag, a, cast<IntegerAttr>(aAttr), b,
                          cast<IntegerAttr>(bAttr))))
           return failure();
+      } else if (isa<DistinctAttr>(aAttr) && isa<DistinctAttr>(bAttr)) {
+        // TODO: properly handle DistinctAttr, including its use in paths.
+        // See https://github.com/llvm/circt/issues/6583
       } else if (aAttr != bAttr) {
         diag.attachNote(a->getLoc())
             << "first operation has attribute '" << attrName.getValue()
@@ -617,21 +634,22 @@ struct Equivalence {
   }
 
   // NOLINTNEXTLINE(misc-no-recursion)
-  LogicalResult check(InFlightDiagnostic &diag, InstanceOp a, InstanceOp b) {
-    auto aName = a.getModuleNameAttr().getAttr();
-    auto bName = b.getModuleNameAttr().getAttr();
+  LogicalResult check(InFlightDiagnostic &diag, FInstanceLike a,
+                      FInstanceLike b) {
+    auto aName = a.getReferencedModuleNameAttr();
+    auto bName = b.getReferencedModuleNameAttr();
     if (aName == bName)
       return success();
 
     // If the modules instantiate are different we will want to know why the
     // sub module did not dedupliate. This code recursively checks the child
     // module.
-    auto aModule = a.getReferencedModule(instanceGraph);
-    auto bModule = b.getReferencedModule(instanceGraph);
+    auto aModule = instanceGraph.lookup(aName)->getModule();
+    auto bModule = instanceGraph.lookup(bName)->getModule();
     // Create a new error for the submodule.
     diag.attachNote(std::nullopt)
-        << "in instance " << a.getNameAttr() << " of " << aName
-        << ", and instance " << b.getNameAttr() << " of " << bName;
+        << "in instance " << a.getInstanceNameAttr() << " of " << aName
+        << ", and instance " << b.getInstanceNameAttr() << " of " << bName;
     check(diag, aModule, bModule);
     return failure();
   }
@@ -648,8 +666,8 @@ struct Equivalence {
 
     // If its an instance operaiton, perform some checking and possibly
     // recurse.
-    if (auto aInst = dyn_cast<InstanceOp>(a)) {
-      auto bInst = cast<InstanceOp>(b);
+    if (auto aInst = dyn_cast<FInstanceLike>(a)) {
+      auto bInst = cast<FInstanceLike>(b);
       if (failed(check(diag, aInst, bInst)))
         return failure();
     }
@@ -940,9 +958,15 @@ struct Deduper {
     auto *toNode = instanceGraph[toModule];
     auto toModuleRef = FlatSymbolRefAttr::get(toModule.getModuleNameAttr());
     for (auto *oldInstRec : llvm::make_early_inc_range(fromNode->uses())) {
-      auto inst = ::cast<InstanceOp>(*oldInstRec->getInstance());
-      inst.setModuleNameAttr(toModuleRef);
-      inst.setPortNamesAttr(toModule.getPortNamesAttr());
+      auto inst = oldInstRec->getInstance();
+      if (auto instOp = dyn_cast<InstanceOp>(*inst)) {
+        instOp.setModuleNameAttr(toModuleRef);
+        instOp.setPortNamesAttr(toModule.getPortNamesAttr());
+      } else if (auto objectOp = dyn_cast<ObjectOp>(*inst)) {
+        auto classLike = cast<ClassLike>(*toNode->getModule());
+        ClassType classType = detail::getInstanceTypeForClassLike(classLike);
+        objectOp.getResult().setType(classType);
+      }
       oldInstRec->getParent()->addInstance(inst, toNode);
       oldInstRec->erase();
     }
@@ -1523,15 +1547,20 @@ class DedupPass : public DedupBase<DedupPass> {
           // If module has symbol (name) that must be preserved even if unused,
           // skip it. All symbol uses must be supported, which is not true if
           // non-private.
-          if (!module.isPrivate() || !module.canDiscardOnUseEmpty()) {
+          if (!module.isPrivate() ||
+              (!module.canDiscardOnUseEmpty() && !isa<ClassLike>(*module))) {
             return success();
           }
 
-          // Explicitly skip class-like modules.  This is presently unreachable
-          // due to above and current implementation but check anyway as dedup
-          // code does not handle these or object operations.
+          // Minimal ClassLike dedup--only ClassLikes within FModuleOps.
           if (isa<ClassLike>(*module)) {
-            return success();
+            InstanceGraphNode *node = instanceGraph.lookup(module);
+            bool onlyUsedInModules =
+                llvm::all_of(node->uses(), [](InstanceRecord *use) {
+                  return isa<FModuleOp>(*use->getParent()->getModule());
+                });
+            if (!onlyUsedInModules)
+              return success();
           }
 
           llvm::SmallSetVector<StringAttr, 1> groups;

lib/Dialect/FIRRTL/Transforms/LowerClasses.cpp

@@ -204,19 +204,16 @@ LogicalResult LowerClassesPass::processPaths(
 
       auto moduleName = target.getModule().getModuleNameAttr();
 
-      // Copy the middle part from the annotation's NLA.
+      // Verify a nonlocal annotation refers to a HierPathOp.
+      hw::HierPathOp hierPathOp;
       if (auto hierName = anno.getMember<FlatSymbolRefAttr>("circt.nonlocal")) {
-        auto hierPathOp =
+        hierPathOp =
             dyn_cast<hw::HierPathOp>(symbolTable.lookup(hierName.getAttr()));
         if (!hierPathOp) {
           op->emitError("annotation does not point at a HierPathOp");
           error = true;
           return false;
         }
-        // Copy the old path, dropping the module name.
-        auto oldPath = hierPathOp.getNamepath().getValue();
-        llvm::append_range(path, llvm::reverse(oldPath.drop_back()));
-        moduleName = cast<hw::InnerRefAttr>(oldPath.front()).getModule();
       }
 
       auto [it, inserted] = pathInfoTable.try_emplace(id);
@@ -236,6 +233,34 @@ LogicalResult LowerClassesPass::processPaths(
       if (!owningModule)
         return true;
 
+      // Copy the middle part from the annotation's NLA.
+      if (hierPathOp) {
+        // Copy the old path, dropping the module name.
+        auto oldPath = hierPathOp.getNamepath().getValue();
+        llvm::append_range(path, llvm::reverse(oldPath.drop_back()));
+
+        // Set the moduleName based on the hierarchical path. If the
+        // owningModule is in the hierarichal path, set the moduleName to the
+        // owning module. Otherwise use the top of the hierarchical path.
+        bool pathContainsOwningModule =
+            llvm::any_of(oldPath, [&](auto pathFragment) {
+              return llvm::TypeSwitch<Attribute, bool>(pathFragment)
+                  .Case([&](hw::InnerRefAttr innerRef) {
+                    return innerRef.getModule() ==
+                           owningModule.getModuleNameAttr();
+                  })
+                  .Case([&](FlatSymbolRefAttr symRef) {
+                    return symRef.getAttr() == owningModule.getModuleNameAttr();
+                  })
+                  .Default([](auto attr) { return false; });
+            });
+        if (pathContainsOwningModule) {
+          moduleName = owningModule.getModuleNameAttr();
+        } else {
+          moduleName = cast<hw::InnerRefAttr>(oldPath.front()).getModule();
+        }
+      }
+
       // Copy the leading part of the hierarchical path from the owning module
       // to the start of the annotation's NLA.
       auto *node = instanceGraph.lookup(moduleName);

test/firtool/classes-dedupe.fir

@@ -0,0 +1,114 @@
+; RUN: firtool %s -ir-verilog | FileCheck %s
+
+FIRRTL version 3.3.0
+
+circuit Test : %[[
+{
+  "class": "firrtl.transforms.MustDeduplicateAnnotation",
+  "modules": ["~Test|CPU_1", "~Test|CPU_2"]
+}
+]]
+  ; CHECK: hw.hierpath private [[NLA1:@.+]] [@Test::@sym, @CPU_1::[[SYM1:@.+]]]
+  ; CHECK: hw.hierpath private [[NLA2:@.+]] [@Test::@sym, @CPU_1::[[SYM2:@.+]], @Fetch_1::[[SYM3:@.+]]]
+  module Test :
+    input in : UInt<1>
+    output out_1 : UInt<1>
+    output out_2 : UInt<1>
+    output om_out_1 : AnyRef
+    output om_out_2 : AnyRef
+    inst cpu_1 of CPU_1
+    inst cpu_2 of CPU_2
+    connect cpu_1.in, in
+    connect cpu_2.in, in
+    connect out_1, cpu_1.out
+    connect out_2, cpu_2.out
+    propassign om_out_1, cpu_1.om_out
+    propassign om_out_2, cpu_2.om_out
+
+  ; CHECK-LABEL: hw.module private @CPU_1
+  ; CHECK-SAME: out out : i1 {hw.exportPort = #hw<innerSym[[SYM1]]>}
+  module CPU_1 :
+    input in : UInt<1>
+    output out : UInt<1>
+    output om_out : AnyRef
+
+    object om of OM_1
+    propassign om_out, om
+
+    ; CHECK: hw.instance "fetch_1" sym [[SYM2]]
+    inst fetch_1 of Fetch_1
+    inst fetch_2 of Fetch_1
+    connect fetch_1.in, in
+    connect fetch_2.in, in
+    connect out, fetch_1.out
+
+  ; CHECK-NOT: CPU_2
+  module CPU_2 :
+    input in : UInt<1>
+    output out : UInt<1>
+    output om_out : AnyRef
+
+    object om of OM_2
+    propassign om_out, om
+
+    inst fetch_1 of Fetch_2
+    inst fetch_2 of Fetch_2
+    connect fetch_1.in, in
+    connect fetch_2.in, in
+    connect out, fetch_1.out
+
+  module Fetch_1 :
+    input in : UInt<1>
+    output out : UInt<1>
+    ; CHECK: %foo = sv.wire sym [[SYM3]]
+    wire foo : UInt<1>
+    connect foo, in
+    connect out, foo
+
+  ; CHECK-NOT: Fetch_2
+  module Fetch_2 :
+    input in : UInt<1>
+    output out : UInt<1>
+    wire foo : UInt<1>
+    connect foo, in
+    connect out, foo
+
+  class Foo_1 :
+    skip
+
+  class Foo_2 :
+    skip
+
+  ; CHECK-LABEL: om.class @OM_1(%basepath: !om.basepath)
+  class OM_1 :
+    output out_1 : Path
+    output out_2 : Path
+    output out_foo_1 : Inst<Foo_1>
+    output out_foo_2 : Inst<Foo_2>
+
+    object foo_1 of Foo_1
+    propassign out_foo_1, foo_1
+
+    object foo_2 of Foo_2
+    propassign out_foo_2, foo_2
+
+    ; CHECK: om.path_create reference %basepath [[NLA1]]
+    propassign out_1, path("OMReferenceTarget:~Test|CPU_1>out")
+    ; CHECK: om.path_create reference %basepath [[NLA2]]
+    propassign out_2, path("OMReferenceTarget:~Test|CPU_1/fetch_1:Fetch_1>foo")
+
+  ; CHECK-NOT: OM_2
+  class OM_2 :
+    output out_1 : Path
+    output out_2 : Path
+    output out_foo_1 : Inst<Foo_1>
+    output out_foo_2 : Inst<Foo_2>
+
+    object foo_1 of Foo_1
+    propassign out_foo_1, foo_1
+
+    object foo_2 of Foo_2
+    propassign out_foo_2, foo_2
+
+    propassign out_1, path("OMReferenceTarget:~Test|CPU_2>out")
+    propassign out_2, path("OMReferenceTarget:~Test|CPU_2/fetch_1:Fetch_2>foo")