Skip to content

Commit

Permalink
Fix auto generated DedupGroups to be sensitive to module prefix
Browse files Browse the repository at this point in the history
Also fix logic for naming PseudoModules which should ignore the prefix.
  • Loading branch information
jackkoenig committed Dec 3, 2024
1 parent 1154121 commit 69cc121
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 deletions.
16 changes: 13 additions & 3 deletions core/src/main/scala/chisel3/ModuleImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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) = {
Expand All @@ -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 =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
22 changes: 22 additions & 0 deletions src/test/scala/chiselTests/ModulePrefixSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"))
}

}

0 comments on commit 69cc121

Please sign in to comment.