Skip to content

Commit aebb874

Browse files
authored
Fix auto generated DedupGroups to be sensitive to module prefix (#4545)
Also fix logic for naming PseudoModules which should ignore the prefix.
1 parent 1154121 commit aebb874

File tree

3 files changed

+36
-4
lines changed

3 files changed

+36
-4
lines changed

core/src/main/scala/chisel3/ModuleImpl.scala

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,17 @@ package experimental {
682682
*/
683683
def desiredName: String = simpleClassName(this.getClass)
684684

685+
/** The name that will be proposed for this module, subject to uniquification.
686+
*
687+
* Includes the module prefix for user-defined modules (but not for blackboxes).
688+
*/
689+
private[chisel3] def _proposedName: String = this match {
690+
// PseudoModules (e.g. Instances) and BlackBoxes have their names set by desiredName.
691+
case _: PseudoModule => desiredName
692+
case _: BaseBlackBox => desiredName
693+
case _ => this.modulePrefix + desiredName
694+
}
695+
685696
/** Legalized name of this module. */
686697
final lazy val name = {
687698
def err(msg: String, cause: Throwable = null) = {
@@ -693,9 +704,8 @@ package experimental {
693704
// PseudoModules are not "true modules" and thus should share
694705
// their original modules names without uniquification
695706
this match {
696-
case _: PseudoModule => Module.currentModulePrefix + desiredName
697-
case _: BaseBlackBox => Builder.globalNamespace.name(desiredName)
698-
case _ => Builder.globalNamespace.name(this.modulePrefix + desiredName)
707+
case _: PseudoModule => _proposedName
708+
case _ => Builder.globalNamespace.name(_proposedName)
699709
}
700710
} catch {
701711
case e: NullPointerException =>

src/main/scala/chisel3/stage/phases/AddDedupGroupAnnotations.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class AddDedupGroupAnnotations extends Phase {
3434
case DefClass(_, _, _, _) => false
3535
case x => true
3636
}.collect {
37-
case x if !(skipAnnos.contains(x.id.toTarget)) => DedupGroupAnnotation(x.id.toTarget, x.id.desiredName)
37+
case x if !(skipAnnos.contains(x.id.toTarget)) => DedupGroupAnnotation(x.id.toTarget, x.id._proposedName)
3838
}
3939
annos ++ annotations
4040
}

src/test/scala/chiselTests/ModulePrefixSpec.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import chisel3.experimental.hierarchy.{instantiable, public, Definition, Instanc
88
import circt.stage.ChiselStage.emitCHIRRTL
99
import circt.stage.ChiselStage
1010
import chisel3.util.SRAM
11+
import firrtl.transforms.DedupGroupAnnotation
1112

1213
object ModulePrefixSpec {
1314
// 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
425426

426427
matchesAndOmits(chirrtl)(lines: _*)()
427428
}
429+
430+
behavior.of("Module prefixes")
431+
432+
it should "affect the dedup group" in {
433+
class Foo extends RawModule
434+
class Bar extends RawModule {
435+
override def localModulePrefix = Some("Outer")
436+
val foo = withModulePrefix("Inner") {
437+
Module(new Foo)
438+
}
439+
}
440+
class Top extends RawModule {
441+
val bar = Module(new Bar)
442+
}
443+
val (_, annos) = getFirrtlAndAnnos(new Top)
444+
val dedupGroups = annos.collect {
445+
case DedupGroupAnnotation(target, group) => target.module -> group
446+
}
447+
dedupGroups should be(Seq("Outer_Inner_Foo" -> "Outer_Inner_Foo", "Outer_Bar" -> "Outer_Bar", "Top" -> "Top"))
448+
}
449+
428450
}

0 commit comments

Comments
 (0)