From 69cc121ef0bd684cc0c250db21f9f333b1a294fb Mon Sep 17 00:00:00 2001
From: Jack Koenig <koenig@sifive.com>
Date: Mon, 2 Dec 2024 15:54:29 -0800
Subject: [PATCH] Fix auto generated DedupGroups to be sensitive to module
 prefix

Also fix logic for naming PseudoModules which should ignore the prefix.
---
 core/src/main/scala/chisel3/ModuleImpl.scala  | 16 +++++++++++---
 .../phases/AddDedupGroupAnnotations.scala     |  2 +-
 .../scala/chiselTests/ModulePrefixSpec.scala  | 22 +++++++++++++++++++
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/core/src/main/scala/chisel3/ModuleImpl.scala b/core/src/main/scala/chisel3/ModuleImpl.scala
index 95d36393f3..30b220148a 100644
--- a/core/src/main/scala/chisel3/ModuleImpl.scala
+++ b/core/src/main/scala/chisel3/ModuleImpl.scala
@@ -682,6 +682,17 @@ package experimental {
       */
     def desiredName: String = simpleClassName(this.getClass)
 
+    /** The name that will be proposed for this module, subject to uniquification.
+      *
+      * Includes the module prefix for user-defined modules (but not for blackboxes).
+      */
+    private[chisel3] def _proposedName: String = this match {
+      // PseudoModules (e.g. Instances) and BlackBoxes have their names set by desiredName.
+      case _: PseudoModule => desiredName
+      case _: BaseBlackBox => desiredName
+      case _ => this.modulePrefix + desiredName
+    }
+
     /** Legalized name of this module. */
     final lazy val name = {
       def err(msg: String, cause: Throwable = null) = {
@@ -693,9 +704,8 @@ package experimental {
         // PseudoModules are not "true modules" and thus should share
         // their original modules names without uniquification
         this match {
-          case _: PseudoModule => Module.currentModulePrefix + desiredName
-          case _: BaseBlackBox => Builder.globalNamespace.name(desiredName)
-          case _ => Builder.globalNamespace.name(this.modulePrefix + desiredName)
+          case _: PseudoModule => _proposedName
+          case _ => Builder.globalNamespace.name(_proposedName)
         }
       } catch {
         case e: NullPointerException =>
diff --git a/src/main/scala/chisel3/stage/phases/AddDedupGroupAnnotations.scala b/src/main/scala/chisel3/stage/phases/AddDedupGroupAnnotations.scala
index d42774a9e8..6b920284f5 100644
--- a/src/main/scala/chisel3/stage/phases/AddDedupGroupAnnotations.scala
+++ b/src/main/scala/chisel3/stage/phases/AddDedupGroupAnnotations.scala
@@ -34,7 +34,7 @@ class AddDedupGroupAnnotations extends Phase {
       case DefClass(_, _, _, _)              => false
       case x                                 => true
     }.collect {
-      case x if !(skipAnnos.contains(x.id.toTarget)) => DedupGroupAnnotation(x.id.toTarget, x.id.desiredName)
+      case x if !(skipAnnos.contains(x.id.toTarget)) => DedupGroupAnnotation(x.id.toTarget, x.id._proposedName)
     }
     annos ++ annotations
   }
diff --git a/src/test/scala/chiselTests/ModulePrefixSpec.scala b/src/test/scala/chiselTests/ModulePrefixSpec.scala
index 1c025570ad..e4a61374bf 100644
--- a/src/test/scala/chiselTests/ModulePrefixSpec.scala
+++ b/src/test/scala/chiselTests/ModulePrefixSpec.scala
@@ -8,6 +8,7 @@ import chisel3.experimental.hierarchy.{instantiable, public, Definition, Instanc
 import circt.stage.ChiselStage.emitCHIRRTL
 import circt.stage.ChiselStage
 import chisel3.util.SRAM
+import firrtl.transforms.DedupGroupAnnotation
 
 object ModulePrefixSpec {
   // This has to be defined at the top-level because @instantiable doesn't work when nested.
@@ -425,4 +426,25 @@ class ModulePrefixSpec extends ChiselFlatSpec with ChiselRunners with Utils with
 
     matchesAndOmits(chirrtl)(lines: _*)()
   }
+
+  behavior.of("Module prefixes")
+
+  it should "affect the dedup group" in {
+    class Foo extends RawModule
+    class Bar extends RawModule {
+      override def localModulePrefix = Some("Outer")
+      val foo = withModulePrefix("Inner") {
+        Module(new Foo)
+      }
+    }
+    class Top extends RawModule {
+      val bar = Module(new Bar)
+    }
+    val (_, annos) = getFirrtlAndAnnos(new Top)
+    val dedupGroups = annos.collect {
+      case DedupGroupAnnotation(target, group) => target.module -> group
+    }
+    dedupGroups should be(Seq("Outer_Inner_Foo" -> "Outer_Inner_Foo", "Outer_Bar" -> "Outer_Bar", "Top" -> "Top"))
+  }
+
 }