From 0a0d7c6aac4326f2127d6d95efa5a4e10c81946c Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Mon, 12 Oct 2020 21:02:27 -0700 Subject: Make it possible to GC Data instances No longer create a pointer from parent to every HasId, only do it by default for BaseModules and MemBases. Add pointer from parent to Data upon binding the Data. * Add MemTypeBinding for port types of Mems This binding is similar to the SampleElementBinding for Vecs in that these Data are not truly hardware, but are represented in the FIRRTL IR and thus need some representation. * Call _onModuleClose on unbound Records This maintains some corner-case behavior that is nevertheless relied upon. It ensures that refs are set for the elements of Records, even if they are not bound to any real hardware. --- core/src/main/scala/chisel3/Module.scala | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index d34211f1..9f8087bf 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -209,6 +209,8 @@ package experimental { */ // TODO: seal this? abstract class BaseModule extends HasId { + _parent.foreach(_.addId(this)) + // // Builder Internals - this tracks which Module RTL construction belongs to. // @@ -382,6 +384,17 @@ package experimental { names } + /** Invokes _onModuleClose on HasIds found via reflection but not bound to hardware + * (thus not part of _ids) + * This maintains old naming behavior for non-hardware Data + */ + private[chisel3] def closeUnboundIds(names: HashMap[HasId, String]): Unit = { + val idLookup = _ids.toSet + for ((id, _) <- names if !idLookup(id)) { + id._onModuleClose + } + } + /** Compatibility function. Allows Chisel2 code which had ports without the IO wrapper to * compile under Bindings checks. Does nothing in non-compatibility mode. * -- cgit v1.2.3 From 923ccbde1353e37f0948d3c5d94b49965dc6d950 Mon Sep 17 00:00:00 2001 From: Jiuyang Liu Date: Sat, 27 Feb 2021 05:01:10 +0800 Subject: Expose AnnotationSeq to Module. (#1731) --- core/src/main/scala/chisel3/Module.scala | 1 + 1 file changed, 1 insertion(+) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 9f8087bf..ede9ccc6 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -14,6 +14,7 @@ import chisel3.internal.firrtl._ import chisel3.internal.sourceinfo.{InstTransform, SourceInfo, UnlocatableSourceInfo} import chisel3.experimental.BaseModule import _root_.firrtl.annotations.{IsModule, ModuleName, ModuleTarget} +import _root_.firrtl.AnnotationSeq object Module extends SourceInfoDoc { /** A wrapper method that all Module instantiations must be wrapped in -- cgit v1.2.3 From c5861176887bfa529277e686df09a42aeceb6cd7 Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Thu, 29 Apr 2021 16:18:06 -0700 Subject: Scala 2.13 support (#1751) --- core/src/main/scala/chisel3/Module.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index ede9ccc6..b204be8d 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -4,9 +4,7 @@ package chisel3 import scala.collection.immutable.ListMap import scala.collection.mutable.{ArrayBuffer, HashMap} -import scala.collection.JavaConversions._ import scala.language.experimental.macros -import java.util.IdentityHashMap import chisel3.internal._ import chisel3.internal.Builder._ @@ -139,6 +137,8 @@ abstract class Module(implicit moduleCompileOptions: CompileOptions) extends Raw package experimental { + import chisel3.internal.requireIsChiselType // Fix ambiguous import + object IO { /** Constructs a port for the current Module * -- cgit v1.2.3 From d3e13ce24956871d2f0fd01ca3a7d89317e3db68 Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Wed, 23 Jun 2021 17:11:22 -0700 Subject: Fix CloneModuleAsRecord support for .toTarget --- core/src/main/scala/chisel3/Module.scala | 91 ++++++++++++++++++++++++++------ 1 file changed, 76 insertions(+), 15 deletions(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index b204be8d..77735583 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -64,13 +64,18 @@ object Module extends SourceInfoDoc { Builder.currentClock = saveClock // Back to clock and reset scope Builder.currentReset = saveReset - val component = module.generateComponent() - Builder.components += component + // Only add the component if the module generates one + val componentOpt = module.generateComponent() + for (component <- componentOpt) { + Builder.components += component + } Builder.setPrefix(savePrefix) // Handle connections at enclosing scope - if(!Builder.currentModule.isEmpty) { + // We use _component because Modules that don't generate them may still have one + if (Builder.currentModule.isDefined && module._component.isDefined) { + val component = module._component.get pushCommand(DefInstance(sourceInfo, module, component.ports)) module.initializeInParent(compileOptions) } @@ -178,20 +183,73 @@ package internal { import chisel3.experimental.BaseModule object BaseModule { - private[chisel3] class ClonePorts (elts: Data*)(implicit compileOptions: CompileOptions) extends Record { + // Private internal class to serve as a _parent for Data in cloned ports + private[chisel3] class ModuleClone(proto: BaseModule) extends BaseModule { + // Don't generate a component, but point to the one for the cloned Module + private[chisel3] def generateComponent(): Option[Component] = { + _component = proto._component + None + } + // This module doesn't acutally exist in the FIRRTL so no initialization to do + private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = () + + override def desiredName: String = proto.name + } + + /** Record type returned by CloneModuleAsRecord + * + * @note These are not true Data (the Record doesn't correspond to anything in the emitted + * FIRRTL yet its elements *do*) so have some very specialized behavior. + * @param proto Optional pointer to the Module we are a clone of. Set for first instance, unset + * for clones + */ + private[chisel3] class ClonePorts (proto: Option[BaseModule], elts: Data*)(implicit compileOptions: CompileOptions) extends Record { val elements = ListMap(elts.map(d => d.instanceName -> d.cloneTypeFull): _*) def apply(field: String) = elements(field) - override def cloneType = (new ClonePorts(elts: _*)).asInstanceOf[this.type] + override def cloneType = (new ClonePorts(None, elts: _*)).asInstanceOf[this.type] + + // Because ClonePorts instances are *not* created inside of their parent module, but rather, + // their parent's parent, we have to intercept the standard setRef and replace it with our own + // special Ref type. + // This only applies to ClonePorts created in cloneIORecord, any clones of these Records have + // normal behavior. + // Also, the name of ClonePorts Records needs to be propagated to their parent ModuleClone + // since we have no other way of setting the instance name for those. + private[chisel3] override def setRef(imm: Arg, force: Boolean): Unit = { + val immx = (proto, imm) match { + case (Some(mod), Ref(name)) => + // Our _parent is a ModuleClone that needs its ref to match ours for .toAbsoluteTarget + _parent.foreach(_.setRef(Ref(name), force=true)) + // Return a specialize-ref that will do the right thing + ModuleCloneIO(mod, name) + case _ => imm + } + super.setRef(immx, force) + } + } + + // Recursively set the parent of the start Data and any children (eg. in an Aggregate) + private def setAllParents(start: Data, parent: Option[BaseModule]): Unit = { + def rec(data: Data): Unit = { + data._parent = parent + data match { + case _: Element => + case agg: Aggregate => + agg.getElements.foreach(rec) + } + } + rec(start) } private[chisel3] def cloneIORecord(proto: BaseModule)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): ClonePorts = { require(proto.isClosed, "Can't clone a module before module close") - val clonePorts = new ClonePorts(proto.getModulePorts: _*) - clonePorts.bind(WireBinding(Builder.forcedUserModule, Builder.currentWhen())) - val cloneInstance = new DefInstance(sourceInfo, proto, proto._component.get.ports) { - override def name = clonePorts.getRef.name - } - pushCommand(cloneInstance) + // We don't create this inside the ModuleClone because we need the ref to be set by the + // currentModule (and not clonePorts) + val clonePorts = new ClonePorts(Some(proto), proto.getModulePorts: _*) + val cloneParent = Module(new ModuleClone(proto)) + clonePorts.bind(PortBinding(cloneParent)) + setAllParents(clonePorts, Some(cloneParent)) + // Normally handled during Module construction but ClonePorts really lives in its parent's parent if (!compileOptions.explicitInvalidate) { pushCommand(DefInvalid(sourceInfo, clonePorts.ref)) } @@ -270,7 +328,7 @@ package experimental { /** Generates the FIRRTL Component (Module or Blackbox) of this Module. * Also closes the module so no more construction can happen inside. */ - private[chisel3] def generateComponent(): Component + private[chisel3] def generateComponent(): Option[Component] /** Sets up this module in the parent context */ @@ -308,9 +366,12 @@ package experimental { /** Legalized name of this module. */ final lazy val name = try { - // If this is a module aspect, it should share the same name as the original module - // Thus, the desired name should be returned without uniquification - if(this.isInstanceOf[ModuleAspect]) desiredName else Builder.globalNamespace.name(desiredName) + // ModuleAspects and ModuleClones are not "true modules" and thus should share + // their original modules names without uniquification + this match { + case (_: ModuleAspect | _: internal.BaseModule.ModuleClone) => desiredName + case _ => Builder.globalNamespace.name(desiredName) + } } catch { case e: NullPointerException => throwException( s"Error: desiredName of ${this.getClass.getName} is null. Did you evaluate 'name' before all values needed by desiredName were available?", e) -- cgit v1.2.3 From b87107ad41e948de9da9c349505de414b1a9db7f Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Mon, 28 Jun 2021 14:07:03 -0700 Subject: Set refs for ModuleClone and ClonePorts in less hacky way --- core/src/main/scala/chisel3/Module.scala | 56 ++++++++++++++++---------------- 1 file changed, 28 insertions(+), 28 deletions(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 77735583..8a914fbc 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -184,48 +184,44 @@ package internal { object BaseModule { // Private internal class to serve as a _parent for Data in cloned ports - private[chisel3] class ModuleClone(proto: BaseModule) extends BaseModule { + private[chisel3] class ModuleClone(_proto: BaseModule) extends BaseModule { + // ClonePorts that hold the bound ports for this module + // Used for setting the refs of both this module and the Record + private[BaseModule] var _portsRecord: Record = _ // Don't generate a component, but point to the one for the cloned Module private[chisel3] def generateComponent(): Option[Component] = { - _component = proto._component + _component = _proto._component None } // This module doesn't acutally exist in the FIRRTL so no initialization to do private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = () - override def desiredName: String = proto.name + override def desiredName: String = _proto.name + + private[chisel3] def setRefAndPortsRef(namespace: Namespace): Unit = { + val record = _portsRecord + // Use .forceName to re-use default name resolving behavior + record.forceName(None, default=this.desiredName, namespace) + // Now take the Ref that forceName set and convert it to the correct Arg + val instName = record.getRef match { + case Ref(name) => name + case bad => throwException(s"Internal Error! Cloned-module Record $record has unexpected ref $bad") + } + // Set both the record and the module to have the same instance name + record.setRef(ModuleCloneIO(_proto, instName), force=true) // force because we did .forceName first + this.setRef(Ref(instName)) + } } /** Record type returned by CloneModuleAsRecord * * @note These are not true Data (the Record doesn't correspond to anything in the emitted * FIRRTL yet its elements *do*) so have some very specialized behavior. - * @param proto Optional pointer to the Module we are a clone of. Set for first instance, unset - * for clones */ - private[chisel3] class ClonePorts (proto: Option[BaseModule], elts: Data*)(implicit compileOptions: CompileOptions) extends Record { + private[chisel3] class ClonePorts (elts: Data*)(implicit compileOptions: CompileOptions) extends Record { val elements = ListMap(elts.map(d => d.instanceName -> d.cloneTypeFull): _*) def apply(field: String) = elements(field) - override def cloneType = (new ClonePorts(None, elts: _*)).asInstanceOf[this.type] - - // Because ClonePorts instances are *not* created inside of their parent module, but rather, - // their parent's parent, we have to intercept the standard setRef and replace it with our own - // special Ref type. - // This only applies to ClonePorts created in cloneIORecord, any clones of these Records have - // normal behavior. - // Also, the name of ClonePorts Records needs to be propagated to their parent ModuleClone - // since we have no other way of setting the instance name for those. - private[chisel3] override def setRef(imm: Arg, force: Boolean): Unit = { - val immx = (proto, imm) match { - case (Some(mod), Ref(name)) => - // Our _parent is a ModuleClone that needs its ref to match ours for .toAbsoluteTarget - _parent.foreach(_.setRef(Ref(name), force=true)) - // Return a specialize-ref that will do the right thing - ModuleCloneIO(mod, name) - case _ => imm - } - super.setRef(immx, force) - } + override def cloneType = (new ClonePorts(elts: _*)).asInstanceOf[this.type] } // Recursively set the parent of the start Data and any children (eg. in an Aggregate) @@ -243,12 +239,16 @@ package internal { private[chisel3] def cloneIORecord(proto: BaseModule)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): ClonePorts = { require(proto.isClosed, "Can't clone a module before module close") + // Fake Module to serve as the _parent of the cloned ports + // We make this before clonePorts because we want it to come up first in naming in + // currentModule + val cloneParent = Module(new ModuleClone(proto)) // We don't create this inside the ModuleClone because we need the ref to be set by the // currentModule (and not clonePorts) - val clonePorts = new ClonePorts(Some(proto), proto.getModulePorts: _*) - val cloneParent = Module(new ModuleClone(proto)) + val clonePorts = new ClonePorts(proto.getModulePorts: _*) clonePorts.bind(PortBinding(cloneParent)) setAllParents(clonePorts, Some(cloneParent)) + cloneParent._portsRecord = clonePorts // Normally handled during Module construction but ClonePorts really lives in its parent's parent if (!compileOptions.explicitInvalidate) { pushCommand(DefInvalid(sourceInfo, clonePorts.ref)) -- cgit v1.2.3 From 0531cb53d3cedaff33c2a280e34418f6af5bc6a1 Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Tue, 29 Jun 2021 15:34:18 -0700 Subject: Restore aop.Select behavior for CloneModuleAsRecord --- core/src/main/scala/chisel3/Module.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 8a914fbc..f82b6a7b 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -184,12 +184,14 @@ package internal { object BaseModule { // Private internal class to serve as a _parent for Data in cloned ports - private[chisel3] class ModuleClone(_proto: BaseModule) extends BaseModule { + private[chisel3] class ModuleClone(private[chisel3] val _proto: BaseModule) extends BaseModule { // ClonePorts that hold the bound ports for this module // Used for setting the refs of both this module and the Record private[BaseModule] var _portsRecord: Record = _ // Don't generate a component, but point to the one for the cloned Module private[chisel3] def generateComponent(): Option[Component] = { + require(!_closed, "Can't generate module more than once") + _closed = true _component = _proto._component None } -- cgit v1.2.3 From 25a84b5667614ea3f437b656f1939caba57e6f66 Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Tue, 29 Jun 2021 16:39:45 -0700 Subject: Change behavior of aop.Select to not include CloneModuleAsRecord Previously, CloneModuleAsRecord clones would result in the same BaseModule object coming up multiple times when using APIs like .instances, .collectDeep, and .getDeep. This was not the intended behavior and can lead to very subtle bugs. --- core/src/main/scala/chisel3/Module.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index f82b6a7b..41fe4554 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -184,7 +184,7 @@ package internal { object BaseModule { // Private internal class to serve as a _parent for Data in cloned ports - private[chisel3] class ModuleClone(private[chisel3] val _proto: BaseModule) extends BaseModule { + private[chisel3] class ModuleClone(_proto: BaseModule) extends BaseModule { // ClonePorts that hold the bound ports for this module // Used for setting the refs of both this module and the Record private[BaseModule] var _portsRecord: Record = _ -- cgit v1.2.3 From 1ceb974c55c6785c21ab3934fa750ade0702e276 Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Thu, 12 Aug 2021 17:04:11 -0700 Subject: Add DataView (#1955) DataView is a mechanism for "viewing" Scala objects as a subtype of `Data`. Often, this is useful for viewing one subtype of `Data`, as another. One can think about a DataView as a cross between a customizable cast and an untagged union. A DataView has a Target type `T`, and a View type `V`. DataView requires that an implementation of `DataProduct` is available for Target types. DataProduct is a type class that provides a way to iterate on `Data` children of objects of implementing types. If a DataView is provided for a type T to a type V, then the function .viewAs[V] (of type T => V) is available. The object (of type T) returned by .viewAs is called a "View" and can be used as both an rvalue and an lvalue. Unlike when using an .asTypeOf cast, connecting to a "View" will connect to the associated field or fields of the underlying Target. DataView also enables .viewAsSupertype which is available for viewing Bundles as a parent Bundle type. It is similar to .viewAs but requires a prototype object of the Target type which will be cloned in order to create the returned View. .viewAsSupertype maps between the corresponding fields of the parent and child Bundle types.--- core/src/main/scala/chisel3/Module.scala | 42 +++++++++++++++++--------------- 1 file changed, 22 insertions(+), 20 deletions(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 41fe4554..64065ec9 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -184,7 +184,7 @@ package internal { object BaseModule { // Private internal class to serve as a _parent for Data in cloned ports - private[chisel3] class ModuleClone(_proto: BaseModule) extends BaseModule { + private[chisel3] class ModuleClone(_proto: BaseModule) extends PseudoModule { // ClonePorts that hold the bound ports for this module // Used for setting the refs of both this module and the Record private[BaseModule] var _portsRecord: Record = _ @@ -195,7 +195,7 @@ package internal { _component = _proto._component None } - // This module doesn't acutally exist in the FIRRTL so no initialization to do + // This module doesn't actually exist in the FIRRTL so no initialization to do private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = () override def desiredName: String = _proto.name @@ -226,19 +226,6 @@ package internal { override def cloneType = (new ClonePorts(elts: _*)).asInstanceOf[this.type] } - // Recursively set the parent of the start Data and any children (eg. in an Aggregate) - private def setAllParents(start: Data, parent: Option[BaseModule]): Unit = { - def rec(data: Data): Unit = { - data._parent = parent - data match { - case _: Element => - case agg: Aggregate => - agg.getElements.foreach(rec) - } - } - rec(start) - } - private[chisel3] def cloneIORecord(proto: BaseModule)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): ClonePorts = { require(proto.isClosed, "Can't clone a module before module close") // Fake Module to serve as the _parent of the cloned ports @@ -249,7 +236,7 @@ package internal { // currentModule (and not clonePorts) val clonePorts = new ClonePorts(proto.getModulePorts: _*) clonePorts.bind(PortBinding(cloneParent)) - setAllParents(clonePorts, Some(cloneParent)) + clonePorts.setAllParents(Some(cloneParent)) cloneParent._portsRecord = clonePorts // Normally handled during Module construction but ClonePorts really lives in its parent's parent if (!compileOptions.explicitInvalidate) { @@ -303,7 +290,15 @@ package experimental { } } - protected def getIds = { + // Returns the last id contained within a Module + private[chisel3] def _lastId: Long = _ids.last match { + case mod: BaseModule => mod._lastId + case _ => + // Ideally we could just take last._id, but Records store and thus bind their Data in reverse order + _ids.maxBy(_._id)._id + } + + private[chisel3] def getIds = { require(_closed, "Can't get ids before module close") _ids.toSeq } @@ -368,10 +363,10 @@ package experimental { /** Legalized name of this module. */ final lazy val name = try { - // ModuleAspects and ModuleClones are not "true modules" and thus should share + // PseudoModules are not "true modules" and thus should share // their original modules names without uniquification this match { - case (_: ModuleAspect | _: internal.BaseModule.ModuleClone) => desiredName + case _: PseudoModule => desiredName case _ => Builder.globalNamespace.name(desiredName) } } catch { @@ -399,7 +394,14 @@ package experimental { final def toAbsoluteTarget: IsModule = { _parent match { case Some(parent) => parent.toAbsoluteTarget.instOf(this.instanceName, toTarget.module) - case None => toTarget + case None => + // FIXME Special handling for Views - evidence of "weirdness" of .toAbsoluteTarget + // In theory, .toAbsoluteTarget should not be necessary, .toTarget combined with the + // target disambiguation in FIRRTL's deduplication transform should ensure that .toTarget + // is always unambigous. However, legacy workarounds for Chisel's lack of an instance API + // have lead some to use .toAbsoluteTarget as a workaround. A proper instance API will make + // it possible to deprecate and remove .toAbsoluteTarget + if (this == ViewParent) ViewParent.absoluteTarget else toTarget } } -- cgit v1.2.3 From 9fa8da227569455a77596355aeb114f9c164510a Mon Sep 17 00:00:00 2001 From: Adam Izraelevitz Date: Sun, 5 Sep 2021 12:11:32 -0700 Subject: Add Definition and Instance API (#2045) This introduces a new experimental API for module instantiation that disentagles elaborating the definition (or implementation) from instantiation of a given module. This solves Chisel's longstanding reliance on "Deduplication" for generating Verilog with multiple instances of the same module. The new API resides in package chisel3.experimental.hierarchy. Please see the hierarchy ScalaDoc, documentation, and tests for examples of use. Co-authored-by: Jack Koenig Co-authored-by: Megan Wachs Co-authored-by: Schuyler Eldridge --- core/src/main/scala/chisel3/Module.scala | 107 +++++++++++++++++++++++++++++-- 1 file changed, 101 insertions(+), 6 deletions(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 64065ec9..d0171693 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -121,7 +121,8 @@ abstract class Module(implicit moduleCompileOptions: CompileOptions) extends Raw private[chisel3] def mkReset: Reset = { // Top module and compatibility mode use Bool for reset - val inferReset = _parent.isDefined && moduleCompileOptions.inferModuleReset + // Note that a Definition elaboration will lack a parent, but still not be a Top module + val inferReset = (_parent.isDefined || Builder.inDefinition) && moduleCompileOptions.inferModuleReset if (inferReset) Reset() else Bool() } @@ -181,13 +182,31 @@ package experimental { package internal { import chisel3.experimental.BaseModule + import chisel3.experimental.hierarchy.IsInstantiable object BaseModule { + /** Represents a clone of an underlying object. This is used to support CloneModuleAsRecord and Instance/Definition. + * + * @note We don't actually "clone" anything in the traditional sense but is a placeholder so we lazily clone internal state + */ + private [chisel3] trait IsClone[+T] { + // Underlying object of which this is a clone of + val _proto: T + def getProto: T = _proto + def isACloneOf(a: Any): Boolean = this == a || _proto == a + } + // Private internal class to serve as a _parent for Data in cloned ports - private[chisel3] class ModuleClone(_proto: BaseModule) extends PseudoModule { + private[chisel3] class ModuleClone[T <: BaseModule] (val _proto: T) extends PseudoModule with IsClone[T] { + override def toString = s"ModuleClone(${_proto})" + def getPorts = _portsRecord // ClonePorts that hold the bound ports for this module // Used for setting the refs of both this module and the Record private[BaseModule] var _portsRecord: Record = _ + // This is necessary for correctly supporting .toTarget on a Module Clone. If it is made from the + // Instance/Definition API, it should return an instanceTarget. If made from CMAR, it should return a + // ModuleTarget. + private[chisel3] var _madeFromDefinition: Boolean = false // Don't generate a component, but point to the one for the cloned Module private[chisel3] def generateComponent(): Option[Component] = { require(!_closed, "Can't generate module more than once") @@ -195,9 +214,15 @@ package internal { _component = _proto._component None } + // Maps proto ports to module clone's ports + private[chisel3] lazy val ioMap: Map[Data, Data] = { + val name2Port = getPorts.elements + _proto.getChiselPorts.map { case (name, data) => data -> name2Port(name) }.toMap + } // This module doesn't actually exist in the FIRRTL so no initialization to do private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = () + // Name of this instance's module is the same as the proto's name override def desiredName: String = _proto.name private[chisel3] def setRefAndPortsRef(namespace: Namespace): Unit = { @@ -215,6 +240,53 @@ package internal { } } + /** Represents a module viewed from a different instance context. + * + * @note Why do we need both ModuleClone and InstanceClone? If we are annotating a reference in a module-clone, + * all submodules must be also be 'cloned' so the toTarget can be computed properly. However, we don't need separate + * connectable ports for this instance; all that's different from the proto is the parent. + * + * @note In addition, the instance name of an InstanceClone is going to be the SAME as the proto, but this is not true + * for ModuleClone. + */ + private[chisel3] final class InstanceClone[T <: BaseModule] (val _proto: T, val instName: () => String) extends PseudoModule with IsClone[T] { + override def toString = s"InstanceClone(${_proto})" + // No addition components are generated + private[chisel3] def generateComponent(): Option[Component] = None + // Necessary for toTarget to work + private[chisel3] def setAsInstanceRef(): Unit = { this.setRef(Ref(instName())) } + // This module doesn't acutally exist in the FIRRTL so no initialization to do + private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = () + // Instance name is the same as proto's instance name + override def instanceName = instName() + // Module name is the same as proto's module name + override def desiredName: String = _proto.name + } + + /** Represents a Definition root module, when accessing something from a definition + * + * @note This is necessary to distinguish between the toTarget behavior for a Module returned from a Definition, + * versus a normal Module. A normal Module.toTarget will always return a local target. If calling toTarget + * on a Module returned from a Definition (and thus wrapped in an Instance), we need to return the non-local + * target whose root is the Definition. This DefinitionClone is used to represent the root parent of the + * InstanceClone (which represents the returned module). + */ + private[chisel3] class DefinitionClone[T <: BaseModule] (val _proto: T) extends PseudoModule with IsClone[T] { + override def toString = s"DefinitionClone(${_proto})" + // No addition components are generated + private[chisel3] def generateComponent(): Option[Component] = None + // Necessary for toTarget to work + private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = () + // Module name is the same as proto's module name + override def desiredName: String = _proto.name + } + + /** @note If we are cloning a non-module, we need another object which has the proper _parent set! + */ + private[chisel3] final class InstantiableClone[T <: IsInstantiable] (val _proto: T) extends IsClone[T] { + private[chisel3] var _parent: Option[BaseModule] = internal.Builder.currentModule + } + /** Record type returned by CloneModuleAsRecord * * @note These are not true Data (the Record doesn't correspond to anything in the emitted @@ -232,6 +304,9 @@ package internal { // We make this before clonePorts because we want it to come up first in naming in // currentModule val cloneParent = Module(new ModuleClone(proto)) + require(proto.isClosed, "Can't clone a module before module close") + require(cloneParent.getOptionRef.isEmpty, "Can't have ref set already!") + // Fake Module to serve as the _parent of the cloned ports // We don't create this inside the ModuleClone because we need the ref to be set by the // currentModule (and not clonePorts) val clonePorts = new ClonePorts(proto.getModulePorts: _*) @@ -253,10 +328,19 @@ package internal { package experimental { + import chisel3.experimental.hierarchy.IsInstantiable + + object BaseModule { + implicit class BaseModuleExtensions[T <: BaseModule](b: T) { + import chisel3.experimental.hierarchy.{Instance, Definition} + def toInstance: Instance[T] = new Instance(Left(b)) + def toDefinition: Definition[T] = new Definition(Left(b)) + } + } /** Abstract base class for Modules, an instantiable organizational unit for RTL. */ // TODO: seal this? - abstract class BaseModule extends HasId { + abstract class BaseModule extends HasId with IsInstantiable { _parent.foreach(_.addId(this)) // @@ -379,13 +463,24 @@ package experimental { * * @note Should not be called until circuit elaboration is complete */ - final def toNamed: ModuleName = toTarget.toNamed + final def toNamed: ModuleName = ModuleTarget(this.circuitName, this.name).toNamed /** Returns a FIRRTL ModuleTarget that references this object * * @note Should not be called until circuit elaboration is complete */ - final def toTarget: ModuleTarget = ModuleTarget(this.circuitName, this.name) + final def toTarget: IsModule = { + this match { + case m: internal.BaseModule.InstanceClone[_] if m._parent.nonEmpty => m._parent.get.toTarget.instOf(instanceName, name) + case m: internal.BaseModule.InstanceClone[_] => ModuleTarget(this.circuitName, this.name) + case m: internal.BaseModule.ModuleClone[_] if m._madeFromDefinition => m._parent.get.toTarget.instOf(instanceName, name) + case m: internal.BaseModule.ModuleClone[_] => ModuleTarget(this.circuitName, this.name) + // Without this, we get the wrong CircuitName for the Definition + case m: internal.BaseModule.DefinitionClone[_] if m._circuit.nonEmpty => ModuleTarget(this._circuit.get.circuitName, this.name) + case m: internal.BaseModule.DefinitionClone[_] => ModuleTarget(this.circuitName, this.name) + case m => ModuleTarget(this.circuitName, this.name) + } + } /** Returns a FIRRTL ModuleTarget that references this object * @@ -393,7 +488,7 @@ package experimental { */ final def toAbsoluteTarget: IsModule = { _parent match { - case Some(parent) => parent.toAbsoluteTarget.instOf(this.instanceName, toTarget.module) + case Some(parent) => parent.toAbsoluteTarget.instOf(this.instanceName, name) case None => // FIXME Special handling for Views - evidence of "weirdness" of .toAbsoluteTarget // In theory, .toAbsoluteTarget should not be necessary, .toTarget combined with the -- cgit v1.2.3 From 2b51053fe7744d6860416c7de8bcb99d4aa9e532 Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Wed, 8 Sep 2021 14:31:10 -0700 Subject: Restore return type of BaseModule.toTarget to ModuleTarget (#2117) Definition/Instance introduced the need for representing the targets of instances as InstanceTargets. This original implementation changed the return type of BaseModule.toTarget to express this need. This is a backwards incompatible change that is actually unnecessary because it is impossible for users to get references to the internal InstanceClone objects, instead only accessing such modules via Instance[_] wrappers and cloned Data. We restored the old API by adding a new internal method "getTarget" which will give the correct targets for InstanceClones while maintaining the API of BaseModule.toTarget.--- core/src/main/scala/chisel3/Module.scala | 36 +++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 12 deletions(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index d0171693..3ae48821 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -469,17 +469,29 @@ package experimental { * * @note Should not be called until circuit elaboration is complete */ - final def toTarget: IsModule = { - this match { - case m: internal.BaseModule.InstanceClone[_] if m._parent.nonEmpty => m._parent.get.toTarget.instOf(instanceName, name) - case m: internal.BaseModule.InstanceClone[_] => ModuleTarget(this.circuitName, this.name) - case m: internal.BaseModule.ModuleClone[_] if m._madeFromDefinition => m._parent.get.toTarget.instOf(instanceName, name) - case m: internal.BaseModule.ModuleClone[_] => ModuleTarget(this.circuitName, this.name) - // Without this, we get the wrong CircuitName for the Definition - case m: internal.BaseModule.DefinitionClone[_] if m._circuit.nonEmpty => ModuleTarget(this._circuit.get.circuitName, this.name) - case m: internal.BaseModule.DefinitionClone[_] => ModuleTarget(this.circuitName, this.name) - case m => ModuleTarget(this.circuitName, this.name) - } + final def toTarget: ModuleTarget = this match { + case m: internal.BaseModule.InstanceClone[_] => throwException(s"Internal Error! It's not legal to call .toTarget on an InstanceClone. $m") + case m: internal.BaseModule.DefinitionClone[_] => throwException(s"Internal Error! It's not legal to call .toTarget on an DefinitionClone. $m") + case _ => ModuleTarget(this.circuitName, this.name) + } + + /** Returns the real target of a Module which may be an [[InstanceTarget]] + * + * BaseModule.toTarget returns a ModuleTarget because the classic Module(new MyModule) API elaborates + * Modules in a way that there is a 1:1 relationship between instances and elaborated definitions + * + * Instance/Definition introduced special internal modules [[InstanceClone]] and [[ModuleClone]] that + * do not have this 1:1 relationship so need the ability to return [[InstanceTarget]]s. + * Because users can never actually get references to these underlying objects, we can maintain + * BaseModule.toTarget's API returning [[ModuleTarget]] while providing an internal API for getting + * the correct [[InstanceTarget]]s whenever using the Definition/Instance API. + */ + private[chisel3] def getTarget: IsModule = this match { + case m: internal.BaseModule.InstanceClone[_] if m._parent.nonEmpty => m._parent.get.getTarget.instOf(instanceName, name) + case m: internal.BaseModule.ModuleClone[_] if m._madeFromDefinition => m._parent.get.getTarget.instOf(instanceName, name) + // Without this, we get the wrong CircuitName for the Definition + case m: internal.BaseModule.DefinitionClone[_] if m._circuit.nonEmpty => ModuleTarget(this._circuit.get.circuitName, this.name) + case _ => this.toTarget } /** Returns a FIRRTL ModuleTarget that references this object @@ -496,7 +508,7 @@ package experimental { // is always unambigous. However, legacy workarounds for Chisel's lack of an instance API // have lead some to use .toAbsoluteTarget as a workaround. A proper instance API will make // it possible to deprecate and remove .toAbsoluteTarget - if (this == ViewParent) ViewParent.absoluteTarget else toTarget + if (this == ViewParent) ViewParent.absoluteTarget else getTarget } } -- cgit v1.2.3