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/Aggregate.scala | 6 ++++-- core/src/main/scala/chisel3/BlackBox.scala | 2 ++ core/src/main/scala/chisel3/Data.scala | 5 +++-- core/src/main/scala/chisel3/Element.scala | 3 ++- core/src/main/scala/chisel3/Mem.scala | 4 ++++ core/src/main/scala/chisel3/Module.scala | 13 +++++++++++++ core/src/main/scala/chisel3/RawModule.scala | 2 ++ core/src/main/scala/chisel3/experimental/Analog.scala | 3 ++- core/src/main/scala/chisel3/internal/Binding.scala | 4 ++++ core/src/main/scala/chisel3/internal/Builder.scala | 1 - 10 files changed, 36 insertions(+), 7 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 403fcdba..f07b2543 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -20,7 +20,8 @@ class AliasedAggregateFieldException(message: String) extends ChiselException(me * of) other Data objects. */ sealed abstract class Aggregate extends Data { - private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection) { + private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = { + _parent.foreach(_.addId(this)) binding = target val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection) @@ -166,7 +167,8 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) case _ => false } - private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection) { + private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = { + _parent.foreach(_.addId(this)) binding = target val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection) diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala index 03543790..8ba4b612 100644 --- a/core/src/main/scala/chisel3/BlackBox.scala +++ b/core/src/main/scala/chisel3/BlackBox.scala @@ -81,6 +81,8 @@ package experimental { id._onModuleClose } + closeUnboundIds(names) + val firrtlPorts = getModulePorts map {port => Port(port, port.specifiedDirection)} val component = DefBlackBox(this, name, firrtlPorts, SpecifiedDirection.Unspecified, params) _component = Some(component) diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 332527bf..377a94e6 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -339,13 +339,14 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { private[chisel3] final def isSynthesizable: Boolean = _binding.map { case ChildBinding(parent) => parent.isSynthesizable case _: TopBinding => true - case _: SampleElementBinding[_] => false + case (_: SampleElementBinding[_] | _: MemTypeBinding[_]) => false }.getOrElse(false) private[chisel3] def topBindingOpt: Option[TopBinding] = _binding.flatMap { case ChildBinding(parent) => parent.topBindingOpt case bindingVal: TopBinding => Some(bindingVal) case SampleElementBinding(parent) => parent.topBindingOpt + case _: MemTypeBinding[_] => None } private[chisel3] def topBinding: TopBinding = topBindingOpt.get @@ -355,7 +356,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { * node is the top-level. * binding and direction are valid after this call completes. */ - private[chisel3] def bind(target: Binding, parentDirection: SpecifiedDirection = SpecifiedDirection.Unspecified) + private[chisel3] def bind(target: Binding, parentDirection: SpecifiedDirection = SpecifiedDirection.Unspecified): Unit // Both _direction and _resolvedUserDirection are saved versions of computed variables (for // efficiency, avoid expensive recomputation of frequent operations). diff --git a/core/src/main/scala/chisel3/Element.scala b/core/src/main/scala/chisel3/Element.scala index 55415f3d..0c99ff70 100644 --- a/core/src/main/scala/chisel3/Element.scala +++ b/core/src/main/scala/chisel3/Element.scala @@ -17,7 +17,8 @@ abstract class Element extends Data { def widthKnown: Boolean = width.known def name: String = getRef.name - private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection) { + private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = { + _parent.foreach(_.addId(this)) binding = target val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection) direction = ActualDirection.fromSpecified(resolvedDirection) diff --git a/core/src/main/scala/chisel3/Mem.scala b/core/src/main/scala/chisel3/Mem.scala index a60b31ac..90525bfa 100644 --- a/core/src/main/scala/chisel3/Mem.scala +++ b/core/src/main/scala/chisel3/Mem.scala @@ -35,6 +35,7 @@ object Mem { } val mt = t.cloneTypeFull val mem = new Mem(mt, size) + mt.bind(MemTypeBinding(mem)) pushCommand(DefMemory(sourceInfo, mem, mt, size)) mem } @@ -45,6 +46,8 @@ object Mem { } sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) extends HasId with NamedComponent with SourceInfoDoc { + _parent.foreach(_.addId(this)) + // REVIEW TODO: make accessors (static/dynamic, read/write) combinations consistent. /** Creates a read accessor into the memory with static addressing. See the @@ -174,6 +177,7 @@ object SyncReadMem { } val mt = t.cloneTypeFull val mem = new SyncReadMem(mt, size, ruw) + mt.bind(MemTypeBinding(mem)) pushCommand(DefSeqMemory(sourceInfo, mem, mt, size, ruw)) mem } 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. * diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala index 0adacedb..d2ba6e84 100644 --- a/core/src/main/scala/chisel3/RawModule.scala +++ b/core/src/main/scala/chisel3/RawModule.scala @@ -98,6 +98,8 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions) id._onModuleClose } + closeUnboundIds(names) + val firrtlPorts = getModulePorts map { port: Data => // Special case Vec to make FIRRTL emit the direction of its // element. diff --git a/core/src/main/scala/chisel3/experimental/Analog.scala b/core/src/main/scala/chisel3/experimental/Analog.scala index df76fd70..2ce2c86d 100644 --- a/core/src/main/scala/chisel3/experimental/Analog.scala +++ b/core/src/main/scala/chisel3/experimental/Analog.scala @@ -45,7 +45,8 @@ final class Analog private (private[chisel3] val width: Width) extends Element { // Define setter/getter pairing // Analog can only be bound to Ports and Wires (and Unbound) - private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection) { + private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = { + _parent.foreach(_.addId(this)) SpecifiedDirection.fromParent(parentDirection, specifiedDirection) match { case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip => case x => throwException(s"Analog may not have explicit direction, got '$x'") diff --git a/core/src/main/scala/chisel3/internal/Binding.scala b/core/src/main/scala/chisel3/internal/Binding.scala index 4442c62e..9e17aded 100644 --- a/core/src/main/scala/chisel3/internal/Binding.scala +++ b/core/src/main/scala/chisel3/internal/Binding.scala @@ -110,6 +110,10 @@ case class ChildBinding(parent: Data) extends Binding { case class SampleElementBinding[T <: Data](parent: Vec[T]) extends Binding { def location = parent.topBinding.location } +/** Special binding for Mem types */ +case class MemTypeBinding[T <: Data](parent: MemBase[T]) extends Binding { + def location: Option[BaseModule] = parent._parent +} // A DontCare element has a specific Binding, somewhat like a literal. // It is a source (RHS). It may only be connected/applied to sinks. case class DontCareBinding() extends UnconstrainedBinding diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index b7772aea..31d4666c 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -84,7 +84,6 @@ trait InstanceId { private[chisel3] trait HasId extends InstanceId { private[chisel3] def _onModuleClose: Unit = {} private[chisel3] val _parent: Option[BaseModule] = Builder.currentModule - _parent.foreach(_.addId(this)) private[chisel3] val _id: Long = Builder.idGen.next -- cgit v1.2.3