From ce1f6d6bf3826b39a6aa1d2162ac919a83203d20 Mon Sep 17 00:00:00 2001 From: Deborah Soung Date: Fri, 29 Apr 2022 16:00:44 -0700 Subject: [PATCH 1/9] first draft --- .../hierarchy/core/Instance.scala | 29 ++++- .../main/scala/chisel3/internal/Builder.scala | 18 ++- .../hierarchy/SeparateElaborationSpec.scala | 113 ++++++++++++++++++ 3 files changed, 156 insertions(+), 4 deletions(-) create mode 100644 src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala b/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala index 90eba378c88..6bf3d78aff3 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala @@ -2,12 +2,13 @@ package chisel3.experimental.hierarchy.core -import scala.collection.mutable.{ArrayBuffer, HashMap} import scala.language.experimental.macros import chisel3._ import chisel3.experimental.hierarchy.{InstantiableClone, ModuleClone} +import chisel3.internal.Builder import chisel3.internal.sourceinfo.{InstanceTransform, SourceInfo} -import chisel3.experimental.BaseModule +import chisel3.experimental.{BaseModule, ExtModule} +import chisel3.internal.firrtl.{Component, DefBlackBox, DefModule, Port} import firrtl.annotations.IsModule /** User-facing Instance type. @@ -107,9 +108,33 @@ object Instance extends SourceInfoDoc { implicit sourceInfo: SourceInfo, compileOptions: CompileOptions ): Instance[T] = { + // Check to see if the module is already defined + val existingMod = Builder.components.map { + case c: DefModule if c.id == definition.proto => Some(c) + case c: DefBlackBox if c.name == definition.proto.name => Some(c) + case _ => None + }.flatten + + if (existingMod.isEmpty) { + // Add a module that will get extracted later on so that FIRRTL does not + // complain about a missing element + class EmptyExtModule extends ExtModule { + override def generateComponent(): Option[Component] = { + require(!_closed, "Can't generate module more than once") + _closed = true + val firrtlPorts = definition.proto.getModulePorts.map { port => Port(port, port.specifiedDirection) } + val component = DefBlackBox(this, definition.proto.name, firrtlPorts, SpecifiedDirection.Unspecified, params) + Some(component) + } + override def desiredName: String = definition.proto.name + } + Definition(new EmptyExtModule() {}) + } + val ports = experimental.CloneModuleAsRecord(definition.proto) val clone = ports._parent.get.asInstanceOf[ModuleClone[T]] clone._madeFromDefinition = true + new Instance(Clone(clone)) } diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 4e775778db5..46a2d480955 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -6,13 +6,14 @@ import scala.util.DynamicVariable import scala.collection.mutable.ArrayBuffer import chisel3._ import chisel3.experimental._ -import chisel3.experimental.hierarchy.core.{Clone, Instance} +import chisel3.experimental.hierarchy.core.{Clone, Instance, IsInstantiable} import chisel3.internal.firrtl._ import chisel3.internal.naming._ -import _root_.firrtl.annotations.{CircuitName, ComponentName, IsMember, ModuleName, Named, ReferenceTarget} +import _root_.firrtl.annotations.{CircuitName, ComponentName, IsMember, ModuleName, Named, NoTargetAnnotation, ReferenceTarget} import _root_.firrtl.annotations.AnnotationUtils.validComponentName import _root_.firrtl.{AnnotationSeq, RenameMap} import chisel3.experimental.dataview.{reify, reifySingleData} +import chisel3.experimental.hierarchy.Definition import chisel3.internal.Builder.Prefix import logger.LazyLogging @@ -363,8 +364,21 @@ private[chisel3] class ChiselContext() { val viewNamespace = Namespace.empty } +/** Stores a [[Definition]] that is imported so that its Instances can be + * compiled separately. + */ +case class ImportedDefinitionAnnotation[T <: BaseModule with IsInstantiable](importedDefinition: Definition[T]) extends NoTargetAnnotation + private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) { + val importedDefinitionsAnno = annotationSeq.collect{case a: ImportedDefinitionAnnotation[_] => a} val globalNamespace = Namespace.empty + + // Ensure imported Definitions get the same name as their modules so that + // they can be properly linked to Instances that are separately compiled + importedDefinitionsAnno.foreach { importedDef => + globalNamespace.name(importedDef.importedDefinition.proto.name) + } + val components = ArrayBuffer[Component]() val annotations = ArrayBuffer[ChiselAnnotation]() var currentModule: Option[BaseModule] = None diff --git a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala new file mode 100644 index 00000000000..4bc19808999 --- /dev/null +++ b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chiselTests.experimental.hierarchy + +import chiselTests.ChiselFunSpec +import chisel3._ +import chisel3.stage.{ChiselGeneratorAnnotation, ChiselStage, DesignAnnotation} +import chisel3.internal.ImportedDefinitionAnnotation +import chisel3.experimental.hierarchy.{Definition, Instance} +import firrtl.options.TargetDirAnnotation + +import java.nio.file.Paths +import scala.io.Source + +class SeparateElaborationSpec extends ChiselFunSpec with Utils { + import Examples._ + + describe("(0): Elaborating an Instance separately from its Definition") { + it("should result in an instantiation in FIRRTL without a module declaration.") { + val testDir = createTestDirectory(this.getClass.getSimpleName).toString + val dutAnnos = (new ChiselStage).run(Seq( + ChiselGeneratorAnnotation(() => new AddOne), + TargetDirAnnotation(testDir), + )) + + // Grab DUT definition to pass into testbench + val designAnnos = dutAnnos.flatMap { a => + a match { + case a: DesignAnnotation[_] => + Some(a.design.asInstanceOf[AddOne].toDefinition) + case _ => None + } + } + require(designAnnos.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designAnnos.") + val dutDef = designAnnos.head + + class Testbench(defn: Definition[AddOne]) extends Module { + // Make sure names do not conflict + val mod = Module(new AddOne) + val inst = Instance(defn) + + // Tie inputs to a value so ChiselStage does not complain + mod.in := 0.U + inst.in := 0.U + } + + (new ChiselStage).run(Seq( + ChiselGeneratorAnnotation(() => new Testbench(dutDef)), + TargetDirAnnotation(testDir), + ImportedDefinitionAnnotation(dutDef) + )) + + // Check that the output RTL has only a module instantiation and no + // module declaration. + val tb_rtl = Source.fromFile(s"$testDir/Testbench.v").getLines.mkString + tb_rtl should include("AddOne inst (") + tb_rtl should not include("module AddOne") + } + } + + describe("(1): Elaborating an Instance and Definition together") { + it("should not result in a repeat definition of the module.") { + val testDir = createTestDirectory(this.getClass.getSimpleName).toString + + class Top extends Module { + val inst = Instance(Definition(new AddOne)) + inst.in := 0.U + } + + // If there is a repeat module definition, FIRRTL emission will fail + (new ChiselStage).emitFirrtl( + gen = new Top, + args = Array("-td", testDir), + ) + } + } + + describe("(2): Elaborating multiple Instances separately from its Definition") { + it ("should not result in a repeat definition of the module.") { + val testDir = createTestDirectory(this.getClass.getSimpleName).toString + val dutAnnos = (new ChiselStage).run(Seq( + ChiselGeneratorAnnotation(() => new AddOne), + TargetDirAnnotation(testDir), + )) + + // Grab DUT definition to pass into testbench + val designAnnos = dutAnnos.flatMap { a => + a match { + case a: DesignAnnotation[_] => + Some(a.design.asInstanceOf[AddOne].toDefinition) + case _ => None + } + } + require(designAnnos.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designAnnos.") + val dutDef = designAnnos.head + + class Testbench(defn: Definition[AddOne]) extends Module { + val inst0 = Instance(defn) + val inst1 = Instance(defn) + + inst0.in := 0.U + inst1.in := 0.U + } + + // If there is a repeat module definition, FIRRTL emission will fail + val firrtl = (new ChiselStage).emitFirrtl( + gen = new Testbench(dutDef), + args = Array("-td", testDir, "--full-stacktrace"), + annotations = Seq(ImportedDefinitionAnnotation(dutDef)) + ) + } + } +} \ No newline at end of file From d9503e9f38751ccd6efc1b43e6373ed8b993315e Mon Sep 17 00:00:00 2001 From: Deborah Soung Date: Fri, 29 Apr 2022 16:03:40 -0700 Subject: [PATCH 2/9] whitespace issue --- .../experimental/hierarchy/SeparateElaborationSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala index 4bc19808999..605a21e1924 100644 --- a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala +++ b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala @@ -110,4 +110,4 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { ) } } -} \ No newline at end of file +} From e8d5979b5483bfb3a40396ba8311d4665fb4c5a6 Mon Sep 17 00:00:00 2001 From: Deborah Soung Date: Mon, 2 May 2022 08:13:49 -0700 Subject: [PATCH 3/9] lint fixes; pr feedback --- .../hierarchy/core/Instance.scala | 8 ++-- .../main/scala/chisel3/internal/Builder.scala | 22 +++++++--- .../hierarchy/SeparateElaborationSpec.scala | 44 +++++++++++-------- 3 files changed, 45 insertions(+), 29 deletions(-) diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala b/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala index 6bf3d78aff3..8dab25d20d9 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala @@ -108,16 +108,16 @@ object Instance extends SourceInfoDoc { implicit sourceInfo: SourceInfo, compileOptions: CompileOptions ): Instance[T] = { - // Check to see if the module is already defined + // Check to see if the module is already defined internally or externally val existingMod = Builder.components.map { - case c: DefModule if c.id == definition.proto => Some(c) + case c: DefModule if c.id == definition.proto => Some(c) case c: DefBlackBox if c.name == definition.proto.name => Some(c) case _ => None }.flatten if (existingMod.isEmpty) { - // Add a module that will get extracted later on so that FIRRTL does not - // complain about a missing element + // Add a Definition that will get emitted as an ExtModule so that FIRRTL + // does not complain about a missing element class EmptyExtModule extends ExtModule { override def generateComponent(): Option[Component] = { require(!_closed, "Can't generate module more than once") diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 46a2d480955..9f00eeb3d89 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -9,7 +9,15 @@ import chisel3.experimental._ import chisel3.experimental.hierarchy.core.{Clone, Instance, IsInstantiable} import chisel3.internal.firrtl._ import chisel3.internal.naming._ -import _root_.firrtl.annotations.{CircuitName, ComponentName, IsMember, ModuleName, Named, NoTargetAnnotation, ReferenceTarget} +import _root_.firrtl.annotations.{ + CircuitName, + ComponentName, + IsMember, + ModuleName, + Named, + NoTargetAnnotation, + ReferenceTarget +} import _root_.firrtl.annotations.AnnotationUtils.validComponentName import _root_.firrtl.{AnnotationSeq, RenameMap} import chisel3.experimental.dataview.{reify, reifySingleData} @@ -367,15 +375,17 @@ private[chisel3] class ChiselContext() { /** Stores a [[Definition]] that is imported so that its Instances can be * compiled separately. */ -case class ImportedDefinitionAnnotation[T <: BaseModule with IsInstantiable](importedDefinition: Definition[T]) extends NoTargetAnnotation +case class ImportedDefinitionAnnotation[T <: BaseModule with IsInstantiable](importedDefinition: Definition[T]) + extends NoTargetAnnotation private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) { - val importedDefinitionsAnno = annotationSeq.collect{case a: ImportedDefinitionAnnotation[_] => a} + val importedDefinitionAnnos = annotationSeq.collect { case a: ImportedDefinitionAnnotation[_] => a } val globalNamespace = Namespace.empty - // Ensure imported Definitions get the same name as their modules so that - // they can be properly linked to Instances that are separately compiled - importedDefinitionsAnno.foreach { importedDef => + // Ensure imported Definitions emit as ExtModules with the correct name so + // that instantiations will also use the correct name and prevent any name + // conflicts with Modules/Definitions in this elaboration + importedDefinitionAnnos.foreach { importedDef => globalNamespace.name(importedDef.importedDefinition.proto.name) } diff --git a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala index 605a21e1924..01a8da6a380 100644 --- a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala +++ b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala @@ -18,21 +18,23 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { describe("(0): Elaborating an Instance separately from its Definition") { it("should result in an instantiation in FIRRTL without a module declaration.") { val testDir = createTestDirectory(this.getClass.getSimpleName).toString - val dutAnnos = (new ChiselStage).run(Seq( - ChiselGeneratorAnnotation(() => new AddOne), - TargetDirAnnotation(testDir), - )) + val dutAnnos = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddOne), + TargetDirAnnotation(testDir) + ) + ) // Grab DUT definition to pass into testbench - val designAnnos = dutAnnos.flatMap { a => + val designDefs = dutAnnos.flatMap { a => a match { case a: DesignAnnotation[_] => Some(a.design.asInstanceOf[AddOne].toDefinition) case _ => None } } - require(designAnnos.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designAnnos.") - val dutDef = designAnnos.head + require(designDefs.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs.") + val dutDef = designDefs.head class Testbench(defn: Definition[AddOne]) extends Module { // Make sure names do not conflict @@ -44,17 +46,19 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { inst.in := 0.U } - (new ChiselStage).run(Seq( - ChiselGeneratorAnnotation(() => new Testbench(dutDef)), - TargetDirAnnotation(testDir), - ImportedDefinitionAnnotation(dutDef) - )) + (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new Testbench(dutDef)), + TargetDirAnnotation(testDir), + ImportedDefinitionAnnotation(dutDef) + ) + ) // Check that the output RTL has only a module instantiation and no // module declaration. val tb_rtl = Source.fromFile(s"$testDir/Testbench.v").getLines.mkString tb_rtl should include("AddOne inst (") - tb_rtl should not include("module AddOne") + (tb_rtl should not).include("module AddOne") } } @@ -70,18 +74,20 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { // If there is a repeat module definition, FIRRTL emission will fail (new ChiselStage).emitFirrtl( gen = new Top, - args = Array("-td", testDir), + args = Array("-td", testDir) ) } } describe("(2): Elaborating multiple Instances separately from its Definition") { - it ("should not result in a repeat definition of the module.") { + it("should not result in a repeat definition of the module.") { val testDir = createTestDirectory(this.getClass.getSimpleName).toString - val dutAnnos = (new ChiselStage).run(Seq( - ChiselGeneratorAnnotation(() => new AddOne), - TargetDirAnnotation(testDir), - )) + val dutAnnos = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddOne), + TargetDirAnnotation(testDir) + ) + ) // Grab DUT definition to pass into testbench val designAnnos = dutAnnos.flatMap { a => From c0c0f8e4c19ef064cc8556aa5b2a2dbed210ff70 Mon Sep 17 00:00:00 2001 From: Deborah Soung Date: Mon, 2 May 2022 11:05:04 -0700 Subject: [PATCH 4/9] update tests, move anno --- .../hierarchy/core/Definition.scala | 9 +- .../main/scala/chisel3/internal/Builder.scala | 19 +-- .../hierarchy/SeparateElaborationSpec.scala | 110 +++++++++--------- 3 files changed, 68 insertions(+), 70 deletions(-) diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala b/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala index 4ee9d46384c..231e1d21d59 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala @@ -9,7 +9,8 @@ import scala.collection.mutable.HashMap import chisel3.internal.{Builder, DynamicContext} import chisel3.internal.sourceinfo.{DefinitionTransform, DefinitionWrapTransform, SourceInfo} import chisel3.experimental.BaseModule -import firrtl.annotations.{IsModule, ModuleTarget} +import chisel3.experimental.hierarchy.Definition +import firrtl.annotations.{IsModule, ModuleTarget, NoTargetAnnotation} /** User-facing Definition type. * Represents a definition of an object of type [[A]] which are marked as @instantiable @@ -111,3 +112,9 @@ object Definition extends SourceInfoDoc { } } + +/** Stores a [[Definition]] that is imported so that its Instances can be + * compiled separately. + */ +case class ImportedDefinitionAnnotation[T <: BaseModule with IsInstantiable](importedDefinition: Definition[T]) + extends NoTargetAnnotation diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 9f00eeb3d89..cac5fbe6031 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -6,22 +6,13 @@ import scala.util.DynamicVariable import scala.collection.mutable.ArrayBuffer import chisel3._ import chisel3.experimental._ -import chisel3.experimental.hierarchy.core.{Clone, Instance, IsInstantiable} +import chisel3.experimental.hierarchy.core.{Clone, ImportedDefinitionAnnotation, Instance} import chisel3.internal.firrtl._ import chisel3.internal.naming._ -import _root_.firrtl.annotations.{ - CircuitName, - ComponentName, - IsMember, - ModuleName, - Named, - NoTargetAnnotation, - ReferenceTarget -} +import _root_.firrtl.annotations.{CircuitName, ComponentName, IsMember, ModuleName, Named, ReferenceTarget} import _root_.firrtl.annotations.AnnotationUtils.validComponentName import _root_.firrtl.{AnnotationSeq, RenameMap} import chisel3.experimental.dataview.{reify, reifySingleData} -import chisel3.experimental.hierarchy.Definition import chisel3.internal.Builder.Prefix import logger.LazyLogging @@ -372,12 +363,6 @@ private[chisel3] class ChiselContext() { val viewNamespace = Namespace.empty } -/** Stores a [[Definition]] that is imported so that its Instances can be - * compiled separately. - */ -case class ImportedDefinitionAnnotation[T <: BaseModule with IsInstantiable](importedDefinition: Definition[T]) - extends NoTargetAnnotation - private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) { val importedDefinitionAnnos = annotationSeq.collect { case a: ImportedDefinitionAnnotation[_] => a } val globalNamespace = Namespace.empty diff --git a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala index 01a8da6a380..0f5892be2fb 100644 --- a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala +++ b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala @@ -5,8 +5,8 @@ package chiselTests.experimental.hierarchy import chiselTests.ChiselFunSpec import chisel3._ import chisel3.stage.{ChiselGeneratorAnnotation, ChiselStage, DesignAnnotation} -import chisel3.internal.ImportedDefinitionAnnotation import chisel3.experimental.hierarchy.{Definition, Instance} +import chisel3.experimental.hierarchy.core.ImportedDefinitionAnnotation import firrtl.options.TargetDirAnnotation import java.nio.file.Paths @@ -15,35 +15,41 @@ import scala.io.Source class SeparateElaborationSpec extends ChiselFunSpec with Utils { import Examples._ - describe("(0): Elaborating an Instance separately from its Definition") { - it("should result in an instantiation in FIRRTL without a module declaration.") { - val testDir = createTestDirectory(this.getClass.getSimpleName).toString - val dutAnnos = (new ChiselStage).run( - Seq( - ChiselGeneratorAnnotation(() => new AddOne), - TargetDirAnnotation(testDir) - ) + /** Elaborates [[AddOne]] and returns its [[Definition]]. */ + def getAddOneDefinition(testDir: String): Definition[AddOne] = { + val dutAnnos = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddOne), + TargetDirAnnotation(testDir) ) - - // Grab DUT definition to pass into testbench - val designDefs = dutAnnos.flatMap { a => - a match { - case a: DesignAnnotation[_] => - Some(a.design.asInstanceOf[AddOne].toDefinition) - case _ => None - } + ) + + // Grab DUT definition to pass into testbench + val designDefs = dutAnnos.flatMap { a => + a match { + case a: DesignAnnotation[_] => + Some(a.design.asInstanceOf[AddOne].toDefinition) + case _ => None } - require(designDefs.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs.") - val dutDef = designDefs.head + } + require(designDefs.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs.") + designDefs.head + } + + describe("(0): Name conflicts") { + it("(0.a): should not occur between a Module and an Instance of a previously elaborated Definition.") { + val testDir = createTestDirectory(this.getClass.getSimpleName).toString + + val dutDef = getAddOneDefinition(testDir) class Testbench(defn: Definition[AddOne]) extends Module { - // Make sure names do not conflict val mod = Module(new AddOne) val inst = Instance(defn) // Tie inputs to a value so ChiselStage does not complain mod.in := 0.U inst.in := 0.U + dontTouch(mod.out) } (new ChiselStage).run( @@ -54,51 +60,51 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { ) ) - // Check that the output RTL has only a module instantiation and no - // module declaration. val tb_rtl = Source.fromFile(s"$testDir/Testbench.v").getLines.mkString + tb_rtl should include("module AddOne_1(") + tb_rtl should include("AddOne_1 mod (") + (tb_rtl should not).include("module AddOne(") tb_rtl should include("AddOne inst (") - (tb_rtl should not).include("module AddOne") } - } - describe("(1): Elaborating an Instance and Definition together") { - it("should not result in a repeat definition of the module.") { + it( + "(0.b): should not occur between an Instance of a Definition and an Instance of a previously elaborated Definition." + ) { val testDir = createTestDirectory(this.getClass.getSimpleName).toString - class Top extends Module { - val inst = Instance(Definition(new AddOne)) - inst.in := 0.U + val dutDef = getAddOneDefinition(testDir) + + class Testbench(defn: Definition[AddOne]) extends Module { + val inst0 = Instance(Definition(new AddOne)) + val inst1 = Instance(defn) + + // Tie inputs to a value so ChiselStage does not complain + inst0.in := 0.U + inst1.in := 0.U + dontTouch(inst0.out) } - // If there is a repeat module definition, FIRRTL emission will fail - (new ChiselStage).emitFirrtl( - gen = new Top, - args = Array("-td", testDir) + (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new Testbench(dutDef)), + TargetDirAnnotation(testDir), + ImportedDefinitionAnnotation(dutDef) + ) ) + + val tb_rtl = Source.fromFile(s"$testDir/Testbench.v").getLines.mkString + tb_rtl should include("module AddOne_1(") + tb_rtl should include("AddOne_1 inst0 (") + (tb_rtl should not).include("module AddOne(") + tb_rtl should include("AddOne inst1 (") } } - describe("(2): Elaborating multiple Instances separately from its Definition") { - it("should not result in a repeat definition of the module.") { + describe("(1): Repeat Module definitions") { + it("(1.a): should not occur when elaborating multiple Instances separately from its Definition.") { val testDir = createTestDirectory(this.getClass.getSimpleName).toString - val dutAnnos = (new ChiselStage).run( - Seq( - ChiselGeneratorAnnotation(() => new AddOne), - TargetDirAnnotation(testDir) - ) - ) - // Grab DUT definition to pass into testbench - val designAnnos = dutAnnos.flatMap { a => - a match { - case a: DesignAnnotation[_] => - Some(a.design.asInstanceOf[AddOne].toDefinition) - case _ => None - } - } - require(designAnnos.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designAnnos.") - val dutDef = designAnnos.head + val dutDef = getAddOneDefinition(testDir) class Testbench(defn: Definition[AddOne]) extends Module { val inst0 = Instance(defn) @@ -109,7 +115,7 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { } // If there is a repeat module definition, FIRRTL emission will fail - val firrtl = (new ChiselStage).emitFirrtl( + (new ChiselStage).emitFirrtl( gen = new Testbench(dutDef), args = Array("-td", testDir, "--full-stacktrace"), annotations = Seq(ImportedDefinitionAnnotation(dutDef)) From 0cf1d9bfcec2c62136dd3ca07a6dac3f98022865 Mon Sep 17 00:00:00 2001 From: Deborah Soung Date: Tue, 3 May 2022 09:36:35 -0700 Subject: [PATCH 5/9] checkpoint for tests; builder needs to throw error --- .../hierarchy/SeparateElaborationSpec.scala | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala index 0f5892be2fb..18d7124ddd1 100644 --- a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala +++ b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala @@ -122,4 +122,148 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { ) } } + + describe("(2): Multiple imported Definitions") { + it( + "(2.a): should work if a list of imported Definitions is passed between Stages." + ) { + val testDir = createTestDirectory(this.getClass.getSimpleName).toString + + val dutAnnos0 = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddOneParameterized(4)), + TargetDirAnnotation(s"$testDir/dutDef0") + ) + ) + + // Grab DUT definition to pass into testbench + val designDefs0 = dutAnnos0.flatMap { a => + a match { + case a: DesignAnnotation[_] => + Some(a.design.asInstanceOf[AddOneParameterized].toDefinition) + case _ => None + } + } + require(designDefs0.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs0.") + val dutDef0 = designDefs0.head + + val dutAnnos1 = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddOneParameterized(8)), + TargetDirAnnotation(s"$testDir/dutDef1"), + // pass in previously elaborated Definitions + ImportedDefinitionAnnotation(dutDef0) + ) + ) + + // Grab DUT definition to pass into testbench + val designDefs1 = dutAnnos1.flatMap { a => + a match { + case a: DesignAnnotation[_] => + Some(a.design.asInstanceOf[AddOneParameterized].toDefinition) + case _ => None + } + } + require(designDefs1.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs1.") + val dutDef1 = designDefs1.head + + class Testbench(defn0: Definition[AddOneParameterized], defn1: Definition[AddOneParameterized]) extends Module { + val inst0 = Instance(defn0) + val inst1 = Instance(defn1) + + // Tie inputs to a value so ChiselStage does not complain + inst0.in := 0.U + inst1.in := 0.U + } + + (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new Testbench(dutDef0, dutDef1)), + TargetDirAnnotation(testDir), + ImportedDefinitionAnnotation(dutDef0), + ImportedDefinitionAnnotation(dutDef1) + ) + ) + + val dutDef0_rtl = Source.fromFile(s"$testDir/dutDef0/AddOneParameterized.v").getLines.mkString + dutDef0_rtl should include("module AddOneParameterized(") + val dutDef1_rtl = Source.fromFile(s"$testDir/dutDef1/AddOneParameterized_1.v").getLines.mkString + dutDef1_rtl should include("module AddOneParameterized_1(") + + val tb_rtl = Source.fromFile(s"$testDir/Testbench.v").getLines.mkString + tb_rtl should include("AddOneParameterized inst0 (") + tb_rtl should include("AddOneParameterized_1 inst1 (") + (tb_rtl should not).include("module AddOneParameterized(") + (tb_rtl should not).include("module AddOneParameterized_1(") + } + + it( + "(2.b): should throw an exception if information is not passed between Stages." + ) { + val testDir = createTestDirectory(this.getClass.getSimpleName).toString + + val dutAnnos0 = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddOneParameterized(4)), + TargetDirAnnotation(s"$testDir/dutDef0") + ) + ) + + // Grab DUT definition to pass into testbench + val designDefs0 = dutAnnos0.flatMap { a => + a match { + case a: DesignAnnotation[_] => + Some(a.design.asInstanceOf[AddOneParameterized].toDefinition) + case _ => None + } + } + require(designDefs0.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs0.") + val dutDef0 = designDefs0.head + + val dutAnnos1 = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddOneParameterized(8)), + TargetDirAnnotation(s"$testDir/dutDef1"), + ) + ) + + // Grab DUT definition to pass into testbench + val designDefs1 = dutAnnos1.flatMap { a => + a match { + case a: DesignAnnotation[_] => + Some(a.design.asInstanceOf[AddOneParameterized].toDefinition) + case _ => None + } + } + require(designDefs1.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs1.") + val dutDef1 = designDefs1.head + + class Testbench(defn0: Definition[AddOneParameterized], defn1: Definition[AddOneParameterized]) extends Module { + val inst0 = Instance(defn0) + val inst1 = Instance(defn1) + + // Tie inputs to a value so ChiselStage does not complain + inst0.in := 0.U + inst1.in := 0.U + } + + // TODO assertThrows() + (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new Testbench(dutDef0, dutDef1)), + TargetDirAnnotation(testDir), + ImportedDefinitionAnnotation(dutDef0), + ImportedDefinitionAnnotation(dutDef1) + ) + ) + + // Because these elaborations have no knowledge of each other, they create + // modules of the same name + val dutDef0_rtl = Source.fromFile(s"$testDir/dutDef0/AddOneParameterized.v").getLines.mkString + dutDef0_rtl should include("module AddOneParameterized(") + val dutDef1_rtl = Source.fromFile(s"$testDir/dutDef1/AddOneParameterized_1.v").getLines.mkString + dutDef1_rtl should include("module AddOneParameterized(") + } + } + } From 71c80c0eda8d0d9ad8ef290056eb5d4d07201a00 Mon Sep 17 00:00:00 2001 From: Deborah Soung Date: Tue, 3 May 2022 10:32:03 -0700 Subject: [PATCH 6/9] throw exception in Builder if imported defs share names --- .../hierarchy/core/Instance.scala | 1 + .../main/scala/chisel3/internal/Builder.scala | 11 ++++++-- .../hierarchy/SeparateElaborationSpec.scala | 26 ++++++++++--------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala b/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala index 8dab25d20d9..8df79f9a65a 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala @@ -108,6 +108,7 @@ object Instance extends SourceInfoDoc { implicit sourceInfo: SourceInfo, compileOptions: CompileOptions ): Instance[T] = { + // TODO do we want to use a hash map instead of linear lookup? // Check to see if the module is already defined internally or externally val existingMod = Builder.components.map { case c: DefModule if c.id == definition.proto => Some(c) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index cac5fbe6031..30c949ea90b 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -365,13 +365,20 @@ private[chisel3] class ChiselContext() { private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) { val importedDefinitionAnnos = annotationSeq.collect { case a: ImportedDefinitionAnnotation[_] => a } + + // Ensure there are no repeated names for imported Definitions + val importedDefinitionNames = importedDefinitionAnnos.map { a => a.importedDefinition.proto.name } + if (importedDefinitionNames.distinct.length < importedDefinitionNames.length) { + throwException("Imported Definitions must have distinct names.") + } + val globalNamespace = Namespace.empty // Ensure imported Definitions emit as ExtModules with the correct name so // that instantiations will also use the correct name and prevent any name // conflicts with Modules/Definitions in this elaboration - importedDefinitionAnnos.foreach { importedDef => - globalNamespace.name(importedDef.importedDefinition.proto.name) + importedDefinitionNames.foreach { importedDefName => + globalNamespace.name(importedDefName) } val components = ArrayBuffer[Component]() diff --git a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala index 18d7124ddd1..7c60aefd14c 100644 --- a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala +++ b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala @@ -223,7 +223,7 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { val dutAnnos1 = (new ChiselStage).run( Seq( ChiselGeneratorAnnotation(() => new AddOneParameterized(8)), - TargetDirAnnotation(s"$testDir/dutDef1"), + TargetDirAnnotation(s"$testDir/dutDef1") ) ) @@ -247,22 +247,24 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { inst1.in := 0.U } - // TODO assertThrows() - (new ChiselStage).run( - Seq( - ChiselGeneratorAnnotation(() => new Testbench(dutDef0, dutDef1)), - TargetDirAnnotation(testDir), - ImportedDefinitionAnnotation(dutDef0), - ImportedDefinitionAnnotation(dutDef1) - ) - ) - // Because these elaborations have no knowledge of each other, they create // modules of the same name val dutDef0_rtl = Source.fromFile(s"$testDir/dutDef0/AddOneParameterized.v").getLines.mkString dutDef0_rtl should include("module AddOneParameterized(") - val dutDef1_rtl = Source.fromFile(s"$testDir/dutDef1/AddOneParameterized_1.v").getLines.mkString + val dutDef1_rtl = Source.fromFile(s"$testDir/dutDef1/AddOneParameterized.v").getLines.mkString dutDef1_rtl should include("module AddOneParameterized(") + + val errMsg = intercept[ChiselException] { + (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new Testbench(dutDef0, dutDef1)), + TargetDirAnnotation(testDir), + ImportedDefinitionAnnotation(dutDef0), + ImportedDefinitionAnnotation(dutDef1) + ) + ) + } + errMsg.getMessage should include("Imported Definitions must have distinct names.") } } From 5357ce32f6aefd20ec4b1f09989d330e3d36a7d3 Mon Sep 17 00:00:00 2001 From: Deborah Soung Date: Tue, 3 May 2022 10:36:59 -0700 Subject: [PATCH 7/9] cleanup --- .../scala/chisel3/experimental/hierarchy/core/Instance.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala b/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala index 8df79f9a65a..8dab25d20d9 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala @@ -108,7 +108,6 @@ object Instance extends SourceInfoDoc { implicit sourceInfo: SourceInfo, compileOptions: CompileOptions ): Instance[T] = { - // TODO do we want to use a hash map instead of linear lookup? // Check to see if the module is already defined internally or externally val existingMod = Builder.components.map { case c: DefModule if c.id == definition.proto => Some(c) From bf41ac916f2673652a555344866fd360f791fd30 Mon Sep 17 00:00:00 2001 From: Deborah Soung Date: Tue, 3 May 2022 13:07:05 -0700 Subject: [PATCH 8/9] better error messages --- .../scala/chisel3/experimental/hierarchy/core/Instance.scala | 4 ++-- core/src/main/scala/chisel3/internal/Builder.scala | 3 ++- .../experimental/hierarchy/SeparateElaborationSpec.scala | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala b/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala index 8dab25d20d9..ef7ad22e3d6 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/core/Instance.scala @@ -119,14 +119,14 @@ object Instance extends SourceInfoDoc { // Add a Definition that will get emitted as an ExtModule so that FIRRTL // does not complain about a missing element class EmptyExtModule extends ExtModule { + override def desiredName: String = definition.proto.name override def generateComponent(): Option[Component] = { - require(!_closed, "Can't generate module more than once") + require(!_closed, s"Can't generate $desiredName module more than once") _closed = true val firrtlPorts = definition.proto.getModulePorts.map { port => Port(port, port.specifiedDirection) } val component = DefBlackBox(this, definition.proto.name, firrtlPorts, SpecifiedDirection.Unspecified, params) Some(component) } - override def desiredName: String = definition.proto.name } Definition(new EmptyExtModule() {}) } diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 30c949ea90b..dd4fde9c6d9 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -369,7 +369,8 @@ private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val thro // Ensure there are no repeated names for imported Definitions val importedDefinitionNames = importedDefinitionAnnos.map { a => a.importedDefinition.proto.name } if (importedDefinitionNames.distinct.length < importedDefinitionNames.length) { - throwException("Imported Definitions must have distinct names.") + val duplicates = importedDefinitionNames.diff(importedDefinitionNames.distinct).mkString(", ") + throwException(s"Expected distinct imported Definition names but found duplicates for: $duplicates") } val globalNamespace = Namespace.empty diff --git a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala index 7c60aefd14c..2650ff27427 100644 --- a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala +++ b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala @@ -264,7 +264,9 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { ) ) } - errMsg.getMessage should include("Imported Definitions must have distinct names.") + errMsg.getMessage should include( + "Expected distinct imported Definition names but found duplicates for: AddOneParameterized" + ) } } From 49ef5ec080f6b837cc9cecf8de6ef6dd64dc2724 Mon Sep 17 00:00:00 2001 From: Deborah Soung Date: Thu, 5 May 2022 16:07:52 -0700 Subject: [PATCH 9/9] adding submodule name conflict test cases --- .../hierarchy/core/Definition.scala | 2 +- .../main/scala/chisel3/internal/Builder.scala | 14 +- .../hierarchy/SeparateElaborationSpec.scala | 209 ++++++++++++------ 3 files changed, 155 insertions(+), 70 deletions(-) diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala b/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala index 231e1d21d59..e846374511f 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala @@ -116,5 +116,5 @@ object Definition extends SourceInfoDoc { /** Stores a [[Definition]] that is imported so that its Instances can be * compiled separately. */ -case class ImportedDefinitionAnnotation[T <: BaseModule with IsInstantiable](importedDefinition: Definition[T]) +case class ImportDefinitionAnnotation[T <: BaseModule with IsInstantiable](definition: Definition[T]) extends NoTargetAnnotation diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index dd4fde9c6d9..e31e1c6942a 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -6,7 +6,7 @@ import scala.util.DynamicVariable import scala.collection.mutable.ArrayBuffer import chisel3._ import chisel3.experimental._ -import chisel3.experimental.hierarchy.core.{Clone, ImportedDefinitionAnnotation, Instance} +import chisel3.experimental.hierarchy.core.{Clone, ImportDefinitionAnnotation, Instance} import chisel3.internal.firrtl._ import chisel3.internal.naming._ import _root_.firrtl.annotations.{CircuitName, ComponentName, IsMember, ModuleName, Named, ReferenceTarget} @@ -364,12 +364,12 @@ private[chisel3] class ChiselContext() { } private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) { - val importedDefinitionAnnos = annotationSeq.collect { case a: ImportedDefinitionAnnotation[_] => a } + val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a } // Ensure there are no repeated names for imported Definitions - val importedDefinitionNames = importedDefinitionAnnos.map { a => a.importedDefinition.proto.name } - if (importedDefinitionNames.distinct.length < importedDefinitionNames.length) { - val duplicates = importedDefinitionNames.diff(importedDefinitionNames.distinct).mkString(", ") + val importDefinitionNames = importDefinitionAnnos.map { a => a.definition.proto.name } + if (importDefinitionNames.distinct.length < importDefinitionNames.length) { + val duplicates = importDefinitionNames.diff(importDefinitionNames.distinct).mkString(", ") throwException(s"Expected distinct imported Definition names but found duplicates for: $duplicates") } @@ -378,8 +378,8 @@ private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val thro // Ensure imported Definitions emit as ExtModules with the correct name so // that instantiations will also use the correct name and prevent any name // conflicts with Modules/Definitions in this elaboration - importedDefinitionNames.foreach { importedDefName => - globalNamespace.name(importedDefName) + importDefinitionNames.foreach { importDefName => + globalNamespace.name(importDefName) } val components = ArrayBuffer[Component]() diff --git a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala index 2650ff27427..e67ed0b31c4 100644 --- a/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala +++ b/src/test/scala/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala @@ -4,9 +4,11 @@ package chiselTests.experimental.hierarchy import chiselTests.ChiselFunSpec import chisel3._ -import chisel3.stage.{ChiselGeneratorAnnotation, ChiselStage, DesignAnnotation} +import chisel3.experimental.BaseModule +import chisel3.stage.{ChiselCircuitAnnotation, ChiselGeneratorAnnotation, ChiselStage, DesignAnnotation} import chisel3.experimental.hierarchy.{Definition, Instance} -import chisel3.experimental.hierarchy.core.ImportedDefinitionAnnotation +import chisel3.experimental.hierarchy.core.ImportDefinitionAnnotation +import firrtl.AnnotationSeq import firrtl.options.TargetDirAnnotation import java.nio.file.Paths @@ -15,8 +17,20 @@ import scala.io.Source class SeparateElaborationSpec extends ChiselFunSpec with Utils { import Examples._ + /** Return a [[DesignAnnotation]] from a list of annotations. */ + private def getDesignAnnotation[T <: RawModule](annos: AnnotationSeq): DesignAnnotation[T] = { + val designAnnos = annos.flatMap { a => + a match { + case a: DesignAnnotation[T] => Some(a) + case _ => None + } + } + require(designAnnos.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designAnnos.") + designAnnos.head + } + /** Elaborates [[AddOne]] and returns its [[Definition]]. */ - def getAddOneDefinition(testDir: String): Definition[AddOne] = { + private def getAddOneDefinition(testDir: String): Definition[AddOne] = { val dutAnnos = (new ChiselStage).run( Seq( ChiselGeneratorAnnotation(() => new AddOne), @@ -25,15 +39,18 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { ) // Grab DUT definition to pass into testbench - val designDefs = dutAnnos.flatMap { a => + getDesignAnnotation(dutAnnos).design.asInstanceOf[AddOne].toDefinition + } + + /** Return [[Definition]]s of all modules in a circuit. */ + private def allModulesToImportedDefs(annos: AnnotationSeq): Seq[ImportDefinitionAnnotation[_]] = { + annos.flatMap { a => a match { - case a: DesignAnnotation[_] => - Some(a.design.asInstanceOf[AddOne].toDefinition) - case _ => None + case a: ChiselCircuitAnnotation => + a.circuit.components.map { c => ImportDefinitionAnnotation(c.id.toDefinition) } + case _ => Seq.empty } } - require(designDefs.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs.") - designDefs.head } describe("(0): Name conflicts") { @@ -56,7 +73,7 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { Seq( ChiselGeneratorAnnotation(() => new Testbench(dutDef)), TargetDirAnnotation(testDir), - ImportedDefinitionAnnotation(dutDef) + ImportDefinitionAnnotation(dutDef) ) ) @@ -88,7 +105,7 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { Seq( ChiselGeneratorAnnotation(() => new Testbench(dutDef)), TargetDirAnnotation(testDir), - ImportedDefinitionAnnotation(dutDef) + ImportDefinitionAnnotation(dutDef) ) ) @@ -118,12 +135,12 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { (new ChiselStage).emitFirrtl( gen = new Testbench(dutDef), args = Array("-td", testDir, "--full-stacktrace"), - annotations = Seq(ImportedDefinitionAnnotation(dutDef)) + annotations = Seq(ImportDefinitionAnnotation(dutDef)) ) } } - describe("(2): Multiple imported Definitions") { + describe("(2): Multiple imported Definitions of modules without submodules") { it( "(2.a): should work if a list of imported Definitions is passed between Stages." ) { @@ -135,37 +152,17 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { TargetDirAnnotation(s"$testDir/dutDef0") ) ) - - // Grab DUT definition to pass into testbench - val designDefs0 = dutAnnos0.flatMap { a => - a match { - case a: DesignAnnotation[_] => - Some(a.design.asInstanceOf[AddOneParameterized].toDefinition) - case _ => None - } - } - require(designDefs0.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs0.") - val dutDef0 = designDefs0.head + val dutDef0 = getDesignAnnotation(dutAnnos0).design.asInstanceOf[AddOneParameterized].toDefinition val dutAnnos1 = (new ChiselStage).run( Seq( ChiselGeneratorAnnotation(() => new AddOneParameterized(8)), TargetDirAnnotation(s"$testDir/dutDef1"), // pass in previously elaborated Definitions - ImportedDefinitionAnnotation(dutDef0) + ImportDefinitionAnnotation(dutDef0) ) ) - - // Grab DUT definition to pass into testbench - val designDefs1 = dutAnnos1.flatMap { a => - a match { - case a: DesignAnnotation[_] => - Some(a.design.asInstanceOf[AddOneParameterized].toDefinition) - case _ => None - } - } - require(designDefs1.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs1.") - val dutDef1 = designDefs1.head + val dutDef1 = getDesignAnnotation(dutAnnos1).design.asInstanceOf[AddOneParameterized].toDefinition class Testbench(defn0: Definition[AddOneParameterized], defn1: Definition[AddOneParameterized]) extends Module { val inst0 = Instance(defn0) @@ -180,8 +177,8 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { Seq( ChiselGeneratorAnnotation(() => new Testbench(dutDef0, dutDef1)), TargetDirAnnotation(testDir), - ImportedDefinitionAnnotation(dutDef0), - ImportedDefinitionAnnotation(dutDef1) + ImportDefinitionAnnotation(dutDef0), + ImportDefinitionAnnotation(dutDef1) ) ) @@ -208,17 +205,7 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { TargetDirAnnotation(s"$testDir/dutDef0") ) ) - - // Grab DUT definition to pass into testbench - val designDefs0 = dutAnnos0.flatMap { a => - a match { - case a: DesignAnnotation[_] => - Some(a.design.asInstanceOf[AddOneParameterized].toDefinition) - case _ => None - } - } - require(designDefs0.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs0.") - val dutDef0 = designDefs0.head + val dutDef0 = getDesignAnnotation(dutAnnos0).design.asInstanceOf[AddOneParameterized].toDefinition val dutAnnos1 = (new ChiselStage).run( Seq( @@ -226,17 +213,7 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { TargetDirAnnotation(s"$testDir/dutDef1") ) ) - - // Grab DUT definition to pass into testbench - val designDefs1 = dutAnnos1.flatMap { a => - a match { - case a: DesignAnnotation[_] => - Some(a.design.asInstanceOf[AddOneParameterized].toDefinition) - case _ => None - } - } - require(designDefs1.length == 1, s"Exactly one DesignAnnotation should exist, but found: $designDefs1.") - val dutDef1 = designDefs1.head + val dutDef1 = getDesignAnnotation(dutAnnos1).design.asInstanceOf[AddOneParameterized].toDefinition class Testbench(defn0: Definition[AddOneParameterized], defn1: Definition[AddOneParameterized]) extends Module { val inst0 = Instance(defn0) @@ -259,8 +236,8 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { Seq( ChiselGeneratorAnnotation(() => new Testbench(dutDef0, dutDef1)), TargetDirAnnotation(testDir), - ImportedDefinitionAnnotation(dutDef0), - ImportedDefinitionAnnotation(dutDef1) + ImportDefinitionAnnotation(dutDef0), + ImportDefinitionAnnotation(dutDef1) ) ) } @@ -270,4 +247,112 @@ class SeparateElaborationSpec extends ChiselFunSpec with Utils { } } + describe("(3): Multiple imported Definitions of modules with submodules") { + it( + "(3.a): should work if a list of imported Definitions for all modules is passed between Stages." + ) { + val testDir = createTestDirectory(this.getClass.getSimpleName).toString + + val dutAnnos0 = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddTwoMixedModules), + TargetDirAnnotation(s"$testDir/dutDef0") + ) + ) + val dutDef0 = getDesignAnnotation(dutAnnos0).design.asInstanceOf[AddTwoMixedModules].toDefinition + val importDefinitionAnnos0 = allModulesToImportedDefs(dutAnnos0) + + val dutAnnos1 = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddTwoMixedModules), + TargetDirAnnotation(s"$testDir/dutDef1") + ) ++ importDefinitionAnnos0 + ) + val dutDef1 = getDesignAnnotation(dutAnnos1).design.asInstanceOf[AddTwoMixedModules].toDefinition + val importDefinitionAnnos1 = allModulesToImportedDefs(dutAnnos1) + + class Testbench(defn0: Definition[AddTwoMixedModules], defn1: Definition[AddTwoMixedModules]) extends Module { + val inst0 = Instance(defn0) + val inst1 = Instance(defn1) + + // Tie inputs to a value so ChiselStage does not complain + inst0.in := 0.U + inst1.in := 0.U + } + + val dutDef0_rtl = Source.fromFile(s"$testDir/dutDef0/AddTwoMixedModules.v").getLines.mkString + dutDef0_rtl should include("module AddOne(") + dutDef0_rtl should include("module AddTwoMixedModules(") + val dutDef1_rtl = Source.fromFile(s"$testDir/dutDef1/AddTwoMixedModules_1.v").getLines.mkString + dutDef1_rtl should include("module AddOne_2(") + dutDef1_rtl should include("module AddTwoMixedModules_1(") + + (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new Testbench(dutDef0, dutDef1)), + TargetDirAnnotation(testDir) + ) ++ importDefinitionAnnos0 ++ importDefinitionAnnos1 + ) + + val tb_rtl = Source.fromFile(s"$testDir/Testbench.v").getLines.mkString + tb_rtl should include("AddTwoMixedModules inst0 (") + tb_rtl should include("AddTwoMixedModules_1 inst1 (") + (tb_rtl should not).include("module AddTwoMixedModules(") + (tb_rtl should not).include("module AddTwoMixedModules_1(") + } + } + + it( + "(3.b): should throw an exception if submodules are not passed between Definition elaborations." + ) { + val testDir = createTestDirectory(this.getClass.getSimpleName).toString + + val dutAnnos0 = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddTwoMixedModules), + TargetDirAnnotation(s"$testDir/dutDef0") + ) + ) + val dutDef0 = getDesignAnnotation(dutAnnos0).design.asInstanceOf[AddTwoMixedModules].toDefinition + val importDefinitionAnnos0 = allModulesToImportedDefs(dutAnnos0) + + val dutAnnos1 = (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new AddTwoMixedModules), + ImportDefinitionAnnotation(dutDef0), + TargetDirAnnotation(s"$testDir/dutDef1") + ) + ) + val dutDef1 = getDesignAnnotation(dutAnnos1).design.asInstanceOf[AddTwoMixedModules].toDefinition + val importDefinitionAnnos1 = allModulesToImportedDefs(dutAnnos1) + + class Testbench(defn0: Definition[AddTwoMixedModules], defn1: Definition[AddTwoMixedModules]) extends Module { + val inst0 = Instance(defn0) + val inst1 = Instance(defn1) + + // Tie inputs to a value so ChiselStage does not complain + inst0.in := 0.U + inst1.in := 0.U + } + + val dutDef0_rtl = Source.fromFile(s"$testDir/dutDef0/AddTwoMixedModules.v").getLines.mkString + dutDef0_rtl should include("module AddOne(") + dutDef0_rtl should include("module AddTwoMixedModules(") + val dutDef1_rtl = Source.fromFile(s"$testDir/dutDef1/AddTwoMixedModules_1.v").getLines.mkString + dutDef1_rtl should include("module AddOne(") + dutDef1_rtl should include("module AddTwoMixedModules_1(") + + val errMsg = intercept[ChiselException] { + (new ChiselStage).run( + Seq( + ChiselGeneratorAnnotation(() => new Testbench(dutDef0, dutDef1)), + TargetDirAnnotation(testDir) + ) ++ importDefinitionAnnos0 ++ importDefinitionAnnos1 + ) + } + errMsg.getMessage should include( + "Expected distinct imported Definition names but found duplicates for: AddOne" + ) + } + }