From 14332084daf4bf63c8dabac24861dc80c204f2d0 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Tue, 30 Apr 2024 12:07:37 -0700 Subject: [PATCH 1/4] Add private Module API and internal DataMirror API for moduleIOs. --- core/src/main/scala/chisel3/Module.scala | 15 +++++++++++++++ .../main/scala/chisel3/reflect/DataMirror.scala | 10 ++++++++++ .../chiselTests/reflect/DataMirrorSpec.scala | 17 +++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 9f94a0911d4..75231c6e935 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -574,6 +574,21 @@ package experimental { _ports.toSeq } + /** Get IOs that are currently bound to this module. + */ + private[chisel3] def getIOs: Seq[Data] = { + _ids.flatMap { id => + id match { + case (data: Data) if data.isSynthesizable => + data.topBinding match { + case PortBinding(_) => Some(data) + case _ => None + } + case _ => None + } + }.toSeq + } + // These methods allow checking some properties of ports before the module is closed, // mainly for compatibility purposes. protected def portsContains(elem: Data): Boolean = { diff --git a/core/src/main/scala/chisel3/reflect/DataMirror.scala b/core/src/main/scala/chisel3/reflect/DataMirror.scala index bfec9030e96..acb39b15740 100644 --- a/core/src/main/scala/chisel3/reflect/DataMirror.scala +++ b/core/src/main/scala/chisel3/reflect/DataMirror.scala @@ -212,6 +212,16 @@ object DataMirror { def chiselTypeClone[T <: Data](target: T): T = { target.cloneTypeFull } + + /** Returns the IOs of a module. + * + * This method does not necessarily return the final ports of the target module. It consults Chisel's internal data + * structures to extract the module's IOs. For this reason, it is generally safer to prefer [[modulePorts]], but + * this method may be used for certain use cases that want the current list of ports before the module is closed. + * + * @param target BaseModule to get IOs from + */ + def moduleIOs(target: BaseModule): Seq[Data] = target.getIOs } // Old definition of collectLeafMembers diff --git a/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala b/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala index 4a8932f5a03..f1b7a09568f 100644 --- a/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala +++ b/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala @@ -245,4 +245,21 @@ class DataMirrorSpec extends ChiselFlatSpec { DataMirror.getLayerColor(foo.c) should be(Some(A)) } + "moduleIOs" should "return an in-progress module's IOs" in { + class Foo extends RawModule { + val in = IO(Input(Bool())) + val out = IO(Output(Bool())) + val wire = Wire(Bool()) + val child = Module(new RawModule {}) + + val ports = DataMirror.internal.moduleIOs(this) + } + + ChiselStage.emitCHIRRTL(new RawModule { + val foo = Module(new Foo) + foo.ports.size should be(2) + foo.ports(0).toNamed.name should be("in") + foo.ports(1).toNamed.name should be("out") + }) + } } From 614be9c87d6e403431bb9d353e8b72eb2784517f Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Tue, 30 Apr 2024 18:05:37 -0700 Subject: [PATCH 2/4] Some updates after a round of reviews. --- .../scala/chisel3/reflect/DataMirror.scala | 9 +++++---- .../chiselTests/reflect/DataMirrorSpec.scala | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/core/src/main/scala/chisel3/reflect/DataMirror.scala b/core/src/main/scala/chisel3/reflect/DataMirror.scala index acb39b15740..4bf2af9e6b0 100644 --- a/core/src/main/scala/chisel3/reflect/DataMirror.scala +++ b/core/src/main/scala/chisel3/reflect/DataMirror.scala @@ -213,15 +213,16 @@ object DataMirror { target.cloneTypeFull } - /** Returns the IOs of a module. + /** Returns the current ports of an in-progress module. * * This method does not necessarily return the final ports of the target module. It consults Chisel's internal data - * structures to extract the module's IOs. For this reason, it is generally safer to prefer [[modulePorts]], but - * this method may be used for certain use cases that want the current list of ports before the module is closed. + * structures to extract the module's IOs. For this reason, it is generally not safe, and users should prefer + * [[DataMirror.modulePorts]], but this method may be used for certain use cases that want the current list of + * ports before the module is closed. * * @param target BaseModule to get IOs from */ - def moduleIOs(target: BaseModule): Seq[Data] = target.getIOs + def currentModulePorts(target: BaseModule): Seq[Data] = target.getIOs } // Old definition of collectLeafMembers diff --git a/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala b/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala index f1b7a09568f..a223737a504 100644 --- a/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala +++ b/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala @@ -252,14 +252,24 @@ class DataMirrorSpec extends ChiselFlatSpec { val wire = Wire(Bool()) val child = Module(new RawModule {}) - val ports = DataMirror.internal.moduleIOs(this) + val ports0 = DataMirror.internal.currentModulePorts(this) + + val other = IO(Input(Bool())) + + val ports1 = DataMirror.internal.currentModulePorts(this) } ChiselStage.emitCHIRRTL(new RawModule { val foo = Module(new Foo) - foo.ports.size should be(2) - foo.ports(0).toNamed.name should be("in") - foo.ports(1).toNamed.name should be("out") + + foo.ports0.size should be(2) + foo.ports0(0).toNamed.name should be("in") + foo.ports0(1).toNamed.name should be("out") + + foo.ports1.size should be(3) + foo.ports1(0).toNamed.name should be("in") + foo.ports1(1).toNamed.name should be("out") + foo.ports1(2).toNamed.name should be("other") }) } } From 5429dab239a1155379ed5f6cd87ef1d2f4a49d63 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Wed, 1 May 2024 10:00:58 -0700 Subject: [PATCH 3/4] Just expose _ports. --- core/src/main/scala/chisel3/Module.scala | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 75231c6e935..3e11c0a1f5b 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -577,16 +577,7 @@ package experimental { /** Get IOs that are currently bound to this module. */ private[chisel3] def getIOs: Seq[Data] = { - _ids.flatMap { id => - id match { - case (data: Data) if data.isSynthesizable => - data.topBinding match { - case PortBinding(_) => Some(data) - case _ => None - } - case _ => None - } - }.toSeq + _ports.map(_._1).toSeq } // These methods allow checking some properties of ports before the module is closed, From 8e42bdd654eada13f75b4ef6f73627fcab38a155 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Wed, 1 May 2024 11:05:38 -0700 Subject: [PATCH 4/4] Simplify the test. --- .../scala/chiselTests/reflect/DataMirrorSpec.scala | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala b/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala index a223737a504..eb7bbc8e693 100644 --- a/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala +++ b/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala @@ -261,15 +261,8 @@ class DataMirrorSpec extends ChiselFlatSpec { ChiselStage.emitCHIRRTL(new RawModule { val foo = Module(new Foo) - - foo.ports0.size should be(2) - foo.ports0(0).toNamed.name should be("in") - foo.ports0(1).toNamed.name should be("out") - - foo.ports1.size should be(3) - foo.ports1(0).toNamed.name should be("in") - foo.ports1(1).toNamed.name should be("out") - foo.ports1(2).toNamed.name should be("other") + foo.ports0 should be(Seq(foo.in, foo.out)) + foo.ports1 should be(Seq(foo.in, foo.out, foo.other)) }) } }