Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support separately elaborating definition and instance in ChiselStage #2512

Merged
merged 11 commits into from
May 12, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -111,3 +112,9 @@ object Definition extends SourceInfoDoc {
}

}

/** Stores a [[Definition]] that is imported so that its Instances can be
* compiled separately.
*/
case class ImportDefinitionAnnotation[T <: BaseModule with IsInstantiable](definition: Definition[T])
extends NoTargetAnnotation
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -107,9 +108,33 @@ object Instance extends SourceInfoDoc {
implicit sourceInfo: SourceInfo,
compileOptions: CompileOptions
): Instance[T] = {
// 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: DefBlackBox if c.name == definition.proto.name => Some(c)
case _ => None
}.flatten
Comment on lines +112 to +116
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jack previously pointed out that this is a linear search. Would it be worthwhile to change Builder.components from an ArrayBuffer to a HashMap for better lookup speed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think its worthwhile unless you can measure the speedup; your lookup function would be complicated because you'd have to find the DefBlackBox case anyways.


if (existingMod.isEmpty) {
// 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, 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)
}
}
Definition(new EmptyExtModule() {})
}

val ports = experimental.CloneModuleAsRecord(definition.proto)
val clone = ports._parent.get.asInstanceOf[ModuleClone[T]]
clone._madeFromDefinition = true

new Instance(Clone(clone))
}

Expand Down
19 changes: 18 additions & 1 deletion core/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ 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, ImportDefinitionAnnotation, Instance}
import chisel3.internal.firrtl._
import chisel3.internal.naming._
import _root_.firrtl.annotations.{CircuitName, ComponentName, IsMember, ModuleName, Named, ReferenceTarget}
Expand Down Expand Up @@ -364,7 +364,24 @@ private[chisel3] class ChiselContext() {
}

private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) {
val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a }

// Ensure there are no repeated names for imported Definitions
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")
}

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
importDefinitionNames.foreach { importDefName =>
globalNamespace.name(importDefName)
}

val components = ArrayBuffer[Component]()
val annotations = ArrayBuffer[ChiselAnnotation]()
var currentModule: Option[BaseModule] = None
Expand Down
Loading