From b1a1b258e099977f82f11dbabadd2d38fcd9fd26 Mon Sep 17 00:00:00 2001 From: ducky Date: Thu, 26 Jan 2017 17:41:25 -0800 Subject: [PATCH 1/6] Module hierarchy refactor Broke no longer broken Fix blackbox Moar fix Refactor mod_context -> UserModule Stricter checks on ImplicitModule MultiIO Module extmodule support MultiIOTester Expose RawModule --- .../src/main/scala/chisel3/core/Attach.scala | 4 +- .../main/scala/chisel3/core/BiConnect.scala | 10 +- .../src/main/scala/chisel3/core/Binder.scala | 10 +- .../src/main/scala/chisel3/core/Binding.scala | 96 ++-- .../src/main/scala/chisel3/core/Bits.scala | 2 +- .../main/scala/chisel3/core/BlackBox.scala | 133 +++++- .../scala/chisel3/core/ChiselAnnotation.scala | 2 +- .../scala/chisel3/core/CompileOptions.scala | 6 - .../src/main/scala/chisel3/core/Data.scala | 6 +- .../src/main/scala/chisel3/core/Mem.scala | 2 +- .../src/main/scala/chisel3/core/Module.scala | 413 ++++++++++++------ .../main/scala/chisel3/core/MonoConnect.scala | 8 +- .../src/main/scala/chisel3/core/Printf.scala | 1 + .../src/main/scala/chisel3/core/Reg.scala | 4 +- .../main/scala/chisel3/internal/Builder.scala | 26 +- .../main/scala/chisel3/internal/Error.scala | 2 +- .../scala/chisel3/internal/firrtl/IR.scala | 10 +- src/main/scala/chisel3/compatibility.scala | 31 +- .../chisel3/internal/firrtl/Emitter.scala | 4 +- src/main/scala/chisel3/package.scala | 7 +- .../scala/chisel3/util/BlackBoxUtils.scala | 4 +- .../chiselTests/AnalogIntegrationSpec.scala | 15 +- src/test/scala/chiselTests/AnalogSpec.scala | 6 +- .../scala/chiselTests/BetterNamingTests.scala | 5 +- .../scala/chiselTests/BundleEltsSpec.scala | 84 ++++ .../chiselTests/CompileOptionsTest.scala | 76 ---- src/test/scala/chiselTests/ExtModule.scala | 72 +++ .../scala/chiselTests/MultiIOModule.scala | 27 ++ .../scala/chiselTests/PrintableSpec.scala | 8 +- src/test/scala/cookbook/CookbookSpec.scala | 2 +- 30 files changed, 707 insertions(+), 369 deletions(-) create mode 100644 src/test/scala/chiselTests/BundleEltsSpec.scala create mode 100644 src/test/scala/chiselTests/ExtModule.scala create mode 100644 src/test/scala/chiselTests/MultiIOModule.scala diff --git a/chiselFrontend/src/main/scala/chisel3/core/Attach.scala b/chiselFrontend/src/main/scala/chisel3/core/Attach.scala index 5e767b84535..edc0a7c9a5d 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Attach.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Attach.scala @@ -14,7 +14,7 @@ object attach { // scalastyle:ignore object.name AttachException(": Conditional attach is not allowed!") // Actual implementation - private[core] def impl(elts: Seq[Analog], contextModule: Module)(implicit sourceInfo: SourceInfo): Unit = { + private[core] def impl(elts: Seq[Analog], contextModule: UserModule)(implicit sourceInfo: SourceInfo): Unit = { if (Builder.whenDepth != 0) throw ConditionalAttachException // TODO Check that references are valid and can be attached @@ -35,7 +35,7 @@ object attach { // scalastyle:ignore object.name */ def apply(elts: Analog*)(implicit sourceInfo: SourceInfo): Unit = { try { - impl(elts, Builder.forcedModule) + impl(elts, Builder.forcedUserModule) } catch { case AttachException(message) => throwException(elts.mkString("Attaching (", ", ", s") failed @$message")) diff --git a/chiselFrontend/src/main/scala/chisel3/core/BiConnect.scala b/chiselFrontend/src/main/scala/chisel3/core/BiConnect.scala index 825dbad7187..95ad95ef1ae 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/BiConnect.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/BiConnect.scala @@ -54,7 +54,7 @@ object BiConnect { * during the recursive decent and then rethrow them with extra information added. * This gives the user a 'path' to where in the connections things went wrong. */ - def connect(sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, left: Data, right: Data, context_mod: Module): Unit = + def connect(sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, left: Data, right: Data, context_mod: UserModule): Unit = (left, right) match { // Handle element case (root case) case (left_a: Analog, right_a: Analog) => @@ -121,12 +121,12 @@ object BiConnect { // This function checks if element-level connection operation allowed. // Then it either issues it or throws the appropriate exception. - def elemConnect(implicit sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, left: Element, right: Element, context_mod: Module): Unit = { + def elemConnect(implicit sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, left: Element, right: Element, context_mod: UserModule): Unit = { import Direction.{Input, Output} // Using extensively so import these // If left or right have no location, assume in context module // This can occur if one of them is a literal, unbound will error previously - val left_mod: Module = left.binding.location.getOrElse(context_mod) - val right_mod: Module = right.binding.location.getOrElse(context_mod) + val left_mod: BaseModule = left.binding.location.getOrElse(context_mod) + val right_mod: BaseModule = right.binding.location.getOrElse(context_mod) val left_direction: Option[Direction] = left.binding.direction val right_direction: Option[Direction] = right.binding.direction @@ -250,7 +250,7 @@ object BiConnect { // This function checks if analog element-level attaching is allowed // Then it either issues it or throws the appropriate exception. - def analogAttach(implicit sourceInfo: SourceInfo, left: Analog, right: Analog, contextModule: Module): Unit = { + def analogAttach(implicit sourceInfo: SourceInfo, left: Analog, right: Analog, contextModule: UserModule): Unit = { // Error if left or right is BICONNECTED in the current module already for (elt <- left :: right :: Nil) { elt.biConnectLocs.get(contextModule) match { diff --git a/chiselFrontend/src/main/scala/chisel3/core/Binder.scala b/chiselFrontend/src/main/scala/chisel3/core/Binder.scala index c7346dcec59..d872d7c62a9 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Binder.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Binder.scala @@ -41,24 +41,24 @@ case object LitBinder extends Binder[LitBinding] { def apply(in: UnboundBinding) = LitBinding() } -case class MemoryPortBinder(enclosure: Module) extends Binder[MemoryPortBinding] { +case class MemoryPortBinder(enclosure: UserModule) extends Binder[MemoryPortBinding] { def apply(in: UnboundBinding) = MemoryPortBinding(enclosure) } -case class OpBinder(enclosure: Module) extends Binder[OpBinding] { +case class OpBinder(enclosure: UserModule) extends Binder[OpBinding] { def apply(in: UnboundBinding) = OpBinding(enclosure) } // Notice how PortBinder uses the direction of the UnboundNode -case class PortBinder(enclosure: Module) extends Binder[PortBinding] { +case class PortBinder(enclosure: BaseModule) extends Binder[PortBinding] { def apply(in: UnboundBinding) = PortBinding(enclosure, in.direction) } -case class RegBinder(enclosure: Module) extends Binder[RegBinding] { +case class RegBinder(enclosure: UserModule) extends Binder[RegBinding] { def apply(in: UnboundBinding) = RegBinding(enclosure) } -case class WireBinder(enclosure: Module) extends Binder[WireBinding] { +case class WireBinder(enclosure: UserModule) extends Binder[WireBinding] { def apply(in: UnboundBinding) = WireBinding(enclosure) } diff --git a/chiselFrontend/src/main/scala/chisel3/core/Binding.scala b/chiselFrontend/src/main/scala/chisel3/core/Binding.scala index a857ae85ab6..87e706b788d 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Binding.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Binding.scala @@ -91,8 +91,6 @@ object Binding { case unbound @ UnboundBinding(_) => { element.binding = binder(unbound) } - // If autoIOWrap is enabled and we're rebinding a PortBinding, just ignore the rebinding. - case portBound @ PortBinding(_, _) if (!(forcedModule.compileOptions.requireIOWrap) && binder.isInstanceOf[PortBinder]) => case binding => throw AlreadyBoundException(binding.toString) } ) @@ -118,68 +116,32 @@ object Binding { // Excepts if any root element is unbound and thus not on the hardware graph def checkSynthesizable(target: Data, error_prelude: String): Unit = { - // This is called if we support autoIOWrap - def elementOfIO(element: Element): Boolean = { - element._parent match { - case None => false - case Some(x: Module) => { - // Have we defined the IO ports for this module? If not, do so now. - if (!x.ioDefined) { - x.computePorts - element.binding match { - case SynthesizableBinding() => true - case _ => false - } - } else { - // io.flatten eliminates Clock elements, so we need to use io.allElements - val ports = x.io.allElements - val isIOElement = ports.contains(element) || element == x.clock || element == x.reset - isIOElement - } - } - } - } - - // Diagnose a binding error caused by a missing IO() wrapper. - // element is the element triggering the binding error. - // Returns true if the element is a member of the module's io but ioDefined is false. - def isMissingIOWrapper(element: Element): Boolean = { - element._parent match { - case None => false - case Some(x: Module) => { - // If the IO() wrapper has been executed, it isn't missing. - if (x.ioDefined) { - false - } else { - // TODO: We should issue the message only once, and if we get here, - // we know the wrapper is missing, whether or not the element is a member of io. - // But if it's not an io element, we want to issue the complementary "unbound" error. - // Revisit this when we collect error messages instead of throwing exceptions. - // The null test below is due to the fact that we may be evaluating the arguments - // of the IO() wrapper itself. - (x.io != null) && x.io.flatten.contains(element) - } - } - } - } - try walkToBinding( target, - element => element.binding match { - case SynthesizableBinding() => {} // OK - case binding => - // The following kludge is an attempt to provide backward compatibility - // It should be done at at higher level. - if ((forcedModule.compileOptions.requireIOWrap || !elementOfIO(element))) { - // Generate a better error message if this is a result of a missing IO() wrapper. - if (isMissingIOWrapper(element)) { - throw MissingIOWrapperException - } else { - throw NotSynthesizableException + element => { + // Compatibility mode to automatically wrap ports in IO + // TODO: remove me, perhaps by removing Bindings checks from compatibility mode + element._parent match { + case Some(x: BaseModule) => x._autoWrapPorts + case _ => + } + // Actual binding check + element.binding match { + case SynthesizableBinding() => // OK + case binding => { + // Attempt to diagnose common bindings issues, like forgot to wrap IO(...) + element._parent match { + case Some(x: LegacyModule) => + // null check in case we try to access io before it is defined + if ((x.io != null) && (x.io.flatten contains element)) { + throw MissingIOWrapperException + } + case _ => } - } else { - Binding.bind(element, PortBinder(element._parent.get), "Error: IO") + // Fallback generic exception + throw NotSynthesizableException } + } } ) catch { @@ -190,7 +152,7 @@ object Binding { // Location refers to 'where' in the Module hierarchy this lives sealed trait Binding { - def location: Option[Module] + def location: Option[BaseModule] def direction: Option[Direction] } @@ -202,7 +164,7 @@ sealed trait UnconstrainedBinding extends Binding { // A constrained binding can only be read/written by specific modules // Location will track where this Module is sealed trait ConstrainedBinding extends Binding { - def enclosure: Module + def enclosure: BaseModule def location = Some(enclosure) } @@ -224,19 +186,19 @@ sealed trait SynthesizableBinding extends Binding case class LitBinding() // will eventually have literal info extends SynthesizableBinding with UnconstrainedBinding with UndirectionedBinding -case class MemoryPortBinding(enclosure: Module) +case class MemoryPortBinding(enclosure: UserModule) extends SynthesizableBinding with ConstrainedBinding with UndirectionedBinding // TODO(twigg): Ops between unenclosed nodes can also be unenclosed // However, Chisel currently binds all op results to a module -case class OpBinding(enclosure: Module) +case class OpBinding(enclosure: UserModule) extends SynthesizableBinding with ConstrainedBinding with UndirectionedBinding -case class PortBinding(enclosure: Module, direction: Option[Direction]) +case class PortBinding(enclosure: BaseModule, direction: Option[Direction]) extends SynthesizableBinding with ConstrainedBinding -case class RegBinding(enclosure: Module) +case class RegBinding(enclosure: UserModule) extends SynthesizableBinding with ConstrainedBinding with UndirectionedBinding -case class WireBinding(enclosure: Module) +case class WireBinding(enclosure: UserModule) extends SynthesizableBinding with ConstrainedBinding with UndirectionedBinding diff --git a/chiselFrontend/src/main/scala/chisel3/core/Bits.scala b/chiselFrontend/src/main/scala/chisel3/core/Bits.scala index dd3a2e8b92f..a8a96946ec1 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Bits.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Bits.scala @@ -1047,7 +1047,7 @@ final class Analog private (width: Width) extends Element(width) { // Used to enforce single bulk connect of Analog types, multi-attach is still okay // Note that this really means 1 bulk connect per Module because a port can // be connected in the parent module as well - private[core] val biConnectLocs = mutable.Map.empty[Module, SourceInfo] + private[core] val biConnectLocs = mutable.Map.empty[UserModule, SourceInfo] // Define setter/getter pairing // Analog can only be bound to Ports and Wires (and Unbound) diff --git a/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala b/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala index fa81a4a5bf7..621ddd7b285 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala @@ -5,7 +5,7 @@ package chisel3.core import chisel3.internal.Builder.pushCommand import chisel3.internal.firrtl._ import chisel3.internal.throwException -import chisel3.internal.sourceinfo.SourceInfo +import chisel3.internal.sourceinfo.{SourceInfo, UnlocatableSourceInfo} // TODO: remove this once we have CompileOptions threaded through the macro system. import chisel3.core.ExplicitCompileOptions.NotStrict @@ -17,6 +17,84 @@ case class StringParam(value: String) extends Param /** Unquoted String */ case class RawParam(value: String) extends Param +abstract class BaseBlackBox extends BaseModule + +/** Defines a black box, which is a module that can be referenced from within + * Chisel, but is not defined in the emitted Verilog. Useful for connecting + * to RTL modules defined outside Chisel. + * + * A variant of BlackBox, this has a more consistent naming scheme in allowing + * multiple top-level IO and does not drop the top prefix. + * + * @example + * Some design require a differential input clock to clock the all design. + * With the xilinx FPGA for example, a Verilog template named IBUFDS must be + * integrated to use differential input: + * {{{ + * IBUFDS #(.DIFF_TERM("TRUE"), + * .IOSTANDARD("DEFAULT")) ibufds ( + * .IB(ibufds_IB), + * .I(ibufds_I), + * .O(ibufds_O) + * ); + * }}} + * + * To instantiate it, a BlackBox can be used like following: + * {{{ + * import chisel3._ + * import chisel3.experimental._ + * + * // Example with Xilinx differential buffer IBUFDS + * class IBUFDS extends ExtModule(Map("DIFF_TERM" -> "TRUE", // Verilog parameters + * "IOSTANDARD" -> "DEFAULT" + * )) { + * val O = IO(Output(Clock())) + * val I = IO(Input(Clock())) + * val IB = IO(Input(Clock())) + * } + * }}} + * @note The parameters API is experimental and may change + */ +abstract class ExtModule(val params: Map[String, Param] = Map.empty[String, Param]) extends BaseBlackBox { + private[core] override def generateComponent(): Component = { + require(!_closed, "Can't generate module more than once") + _closed = true + + val names = nameIds(classOf[ExtModule]) + + // Name ports based on reflection + for (port <- _ports) { + require(names.contains(port), s"Unable to name port $port in $this") + port.setRef(ModuleIO(this, _namespace.name(names(port)))) + } + + // All suggestions are in, force names to every node. + for (id <- _ids) { + id.forceName(default="_T", _namespace) + id._onModuleClose + } + + val firrtlPorts = for (port <- _ports) yield { + // Port definitions need to know input or output at top-level. + // By FIRRTL semantics, 'flipped' becomes an Input + val direction = if(Data.isFirrtlFlipped(port)) Direction.Input else Direction.Output + Port(port, direction) + } + + val component = DefBlackBox(this, name, firrtlPorts, params) + _component = Some(component) + component + } + + private[core] def initializeInParent() { + implicit val sourceInfo = UnlocatableSourceInfo + + _ports.foreach { x: Data => + pushCommand(DefInvalid(sourceInfo, x.ref)) + } + } +} + /** Defines a black box, which is a module that can be referenced from within * Chisel, but is not defined in the emitted Verilog. Useful for connecting * to RTL modules defined outside Chisel. @@ -52,22 +130,29 @@ case class RawParam(value: String) extends Param * }}} * @note The parameters API is experimental and may change */ -abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param]) extends Module { +abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param]) extends BaseBlackBox { + def io: Record + + private[core] override def generateComponent(): Component = { + _autoWrapPorts() // pre-IO(...) compatibility hack + + // Restrict IO to just io, clock, and reset + require(io != null, "BlackBox must have io") + require(_ports contains io, "BlackBox must have io wrapped in IO(...)") + require(_ports.size == 1, "BlackBox must only have io as IO") - // The body of a BlackBox is empty, the real logic happens in firrtl/Emitter.scala - // Bypass standard clock, reset, io port declaration by flattening io - // TODO(twigg): ? Really, overrides are bad, should extend BaseModule.... - override private[core] def ports = io.elements.toSeq + require(!_closed, "Can't generate module more than once") + _closed = true - // Do not do reflective naming of internal signals, just name io - override private[core] def setRefs(): this.type = { + val namedPorts = io.elements.toSeq // setRef is not called on the actual io. // There is a risk of user improperly attempting to connect directly with io // Long term solution will be to define BlackBox IO differently as part of // it not descending from the (current) Module - for ((name, port) <- ports) { + for ((name, port) <- namedPorts) { port.setRef(ModuleIO(this, _namespace.name(name))) } + // We need to call forceName and onModuleClose on all of the sub-elements // of the io bundle, but NOT on the io bundle itself. // Doing so would cause the wrong names to be assigned, since their parent @@ -76,21 +161,25 @@ abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param id.forceName(default="_T", _namespace) id._onModuleClose } - this - } - // Don't setup clock, reset - // Cann't invalide io in one bunch, must invalidate each part separately - override private[core] def setupInParent(implicit sourceInfo: SourceInfo): this.type = _parent match { - case Some(p) => { - // Just init instance inputs - for((_,port) <- ports) pushCommand(DefInvalid(sourceInfo, port.ref)) - this + val firrtlPorts = for ((_, port) <- namedPorts) yield { + // Port definitions need to know input or output at top-level. + // By FIRRTL semantics, 'flipped' becomes an Input + val direction = if(Data.isFirrtlFlipped(port)) Direction.Input else Direction.Output + Port(port, direction) } - case None => this + + val component = DefBlackBox(this, name, firrtlPorts, params) + _component = Some(component) + component } - // Using null is horrible but these signals SHOULD NEVER be used: - override val clock = null - override val reset = null + private[core] def initializeInParent() { + implicit val sourceInfo = UnlocatableSourceInfo + + val namedPorts = io.elements.toSeq + for ((_, port) <- namedPorts) { + pushCommand(DefInvalid(sourceInfo, port.ref)) + } + } } diff --git a/chiselFrontend/src/main/scala/chisel3/core/ChiselAnnotation.scala b/chiselFrontend/src/main/scala/chisel3/core/ChiselAnnotation.scala index 73573bb104f..015629e5ab0 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/ChiselAnnotation.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/ChiselAnnotation.scala @@ -18,7 +18,7 @@ case class ChiselAnnotation(component: InstanceId, transformClass: Class[_ <: Tr def toFirrtl: Annotation = { val circuitName = CircuitName(component.pathName.split("""\.""").head) component match { - case m: Module => + case m: BaseModule => Annotation( ModuleName(m.name, circuitName), transformClass, value) case _ => diff --git a/chiselFrontend/src/main/scala/chisel3/core/CompileOptions.scala b/chiselFrontend/src/main/scala/chisel3/core/CompileOptions.scala index 482c1566541..803e6c0fcef 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/CompileOptions.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/CompileOptions.scala @@ -12,8 +12,6 @@ trait CompileOptions { val connectFieldsMustMatch: Boolean // When creating an object that takes a type argument, the argument must be unbound (a pure type). val declaredTypeMustBeUnbound: Boolean - // Module IOs should be wrapped in an IO() to define their bindings before the reset of the module is defined. - val requireIOWrap: Boolean // If a connection operator fails, don't try the connection with the operands (source and sink) reversed. val dontTryConnectionsSwapped: Boolean // If connection directionality is not explicit, do not use heuristics to attempt to determine it. @@ -44,8 +42,6 @@ object ExplicitCompileOptions { val connectFieldsMustMatch: Boolean, // When creating an object that takes a type argument, the argument must be unbound (a pure type). val declaredTypeMustBeUnbound: Boolean, - // Module IOs should be wrapped in an IO() to define their bindings before the reset of the module is defined. - val requireIOWrap: Boolean, // If a connection operator fails, don't try the connection with the operands (source and sink) reversed. val dontTryConnectionsSwapped: Boolean, // If connection directionality is not explicit, do not use heuristics to attempt to determine it. @@ -63,7 +59,6 @@ object ExplicitCompileOptions { implicit val NotStrict = new CompileOptionsClass ( connectFieldsMustMatch = false, declaredTypeMustBeUnbound = false, - requireIOWrap = false, dontTryConnectionsSwapped = false, dontAssumeDirectionality = false, deprecateOldDirectionMethods = false, @@ -75,7 +70,6 @@ object ExplicitCompileOptions { implicit val Strict = new CompileOptionsClass ( connectFieldsMustMatch = true, declaredTypeMustBeUnbound = true, - requireIOWrap = true, dontTryConnectionsSwapped = true, dontAssumeDirectionality = true, deprecateOldDirectionMethods = true, diff --git a/chiselFrontend/src/main/scala/chisel3/core/Data.scala b/chiselFrontend/src/main/scala/chisel3/core/Data.scala index 556f2aeb653..c17672ff052 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Data.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Data.scala @@ -200,7 +200,7 @@ abstract class Data extends HasId { Binding.checkSynthesizable(this, s"'this' ($this)") Binding.checkSynthesizable(that, s"'that' ($that)") try { - MonoConnect.connect(sourceInfo, connectCompileOptions, this, that, Builder.forcedModule) + MonoConnect.connect(sourceInfo, connectCompileOptions, this, that, Builder.forcedUserModule) } catch { case MonoConnect.MonoConnectException(message) => throwException( @@ -216,7 +216,7 @@ abstract class Data extends HasId { Binding.checkSynthesizable(this, s"'this' ($this)") Binding.checkSynthesizable(that, s"'that' ($that)") try { - BiConnect.connect(sourceInfo, connectCompileOptions, this, that, Builder.forcedModule) + BiConnect.connect(sourceInfo, connectCompileOptions, this, that, Builder.forcedUserModule) } catch { case BiConnect.BiConnectException(message) => throwException( @@ -374,7 +374,7 @@ object Wire { val x = t.chiselCloneType // Bind each element of x to being a Wire - Binding.bind(x, WireBinder(Builder.forcedModule), "Error: t") + Binding.bind(x, WireBinder(Builder.forcedUserModule), "Error: t") pushCommand(DefWire(sourceInfo, x)) pushCommand(DefInvalid(sourceInfo, x.ref)) diff --git a/chiselFrontend/src/main/scala/chisel3/core/Mem.scala b/chiselFrontend/src/main/scala/chisel3/core/Mem.scala index a48af15ac44..006670e72a1 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Mem.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Mem.scala @@ -95,7 +95,7 @@ sealed abstract class MemBase[T <: Data](t: T, val length: Int) extends HasId wi t.chiselCloneType, Node(this), dir, i.ref, Node(Builder.forcedClock)) ).id // Bind each element of port to being a MemoryPort - Binding.bind(port, MemoryPortBinder(Builder.forcedModule), "Error: Fresh t") + Binding.bind(port, MemoryPortBinder(Builder.forcedUserModule), "Error: Fresh t") port } } diff --git a/chiselFrontend/src/main/scala/chisel3/core/Module.scala b/chiselFrontend/src/main/scala/chisel3/core/Module.scala index b838eb0544a..d2c33cd59c4 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Module.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Module.scala @@ -2,8 +2,12 @@ package chisel3.core -import scala.collection.mutable.ArrayBuffer +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._ import chisel3.internal.firrtl._ @@ -18,19 +22,16 @@ object Module { * * @return the input module `m` with Chisel metadata properly set */ - def apply[T <: Module](bc: => T): T = macro InstTransform.apply[T] - - def do_apply[T <: Module](bc: => T)(implicit sourceInfo: SourceInfo): T = { - // Don't generate source info referencing parents inside a module, sincce this interferes with - // module de-duplication in FIRRTL emission. - val childSourceInfo = UnlocatableSourceInfo + def apply[T <: BaseModule](bc: => T): T = macro InstTransform.apply[T] + def do_apply[T <: BaseModule](bc: => T)(implicit sourceInfo: SourceInfo): T = { if (Builder.readyForModuleConstr) { throwException("Error: Called Module() twice without instantiating a Module." + sourceInfo.makeMessage(" See " + _)) } Builder.readyForModuleConstr = true - val parent: Option[Module] = Builder.currentModule + + val parent: Option[BaseModule] = Builder.currentModule val whenDepth: Int = Builder.whenDepth val clockAndReset: Option[ClockAndReset] = Builder.currentClockAndReset @@ -39,8 +40,7 @@ object Module { // - unset readyForModuleConstr // - reset whenDepth to 0 // - set currentClockAndReset - val m = bc.setRefs() - m._commands.prepend(DefInvalid(childSourceInfo, m.io.ref)) // init module outputs + val module: T = bc // Module is actually invoked here if (Builder.whenDepth != 0) { throwException("Internal Error! When depth is != 0, this should not be possible") @@ -54,23 +54,15 @@ object Module { Builder.whenDepth = whenDepth Builder.currentClockAndReset = clockAndReset // Back to clock and reset scope - val ports = m.computePorts - // Blackbox inherits from Module so we have to match on it first TODO fix - val component = m match { - case bb: BlackBox => - DefBlackBox(bb, bb.name, ports, bb.params) - case mod: Module => - mod._commands.prepend(DefInvalid(childSourceInfo, mod.io.ref)) // init module outputs - DefModule(mod, mod.name, ports, mod._commands) - } - m._component = Some(component) + val component = module.generateComponent() Builder.components += component - // Avoid referencing 'parent' in top module + + // Handle connections at enclosing scope if(!Builder.currentModule.isEmpty) { - pushCommand(DefInstance(sourceInfo, m, ports)) - m.setupInParent(childSourceInfo) + pushCommand(DefInstance(sourceInfo, module, component.ports)) + module.initializeInParent() } - m + module } /** Returns the implicit Clock */ @@ -79,34 +71,122 @@ object Module { def reset: Bool = Builder.forcedReset } -/** Abstract base class for Modules, which behave much like Verilog modules. - * These may contain both logic and state which are written in the Module - * body (constructor). - * - * @note Module instantiations must be wrapped in a Module() call. +/** Abstract base class for Modules, an instantiable organizational unit for RTL. */ -abstract class Module( - override_clock: Option[Clock]=None, override_reset: Option[Bool]=None) - (implicit moduleCompileOptions: CompileOptions) -extends HasId { - // _clock and _reset can be clock and reset in these 2ary constructors - // once chisel2 compatibility issues are resolved - def this(_clock: Clock)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), None)(moduleCompileOptions) - def this(_reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(None, Option(_reset))(moduleCompileOptions) - def this(_clock: Clock, _reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), Option(_reset))(moduleCompileOptions) +// TODO: seal this? +abstract class BaseModule extends HasId { + // + // Builder Internals - this tracks which Module RTL construction belongs to. + // + if (!Builder.readyForModuleConstr) { + throwException("Error: attempted to instantiate a Module without wrapping it in Module().") + } + readyForModuleConstr = false - // This function binds the iodef as a port in the hardware graph - private[chisel3] def Port[T<:Data](iodef: T): iodef.type = { - // Bind each element of the iodef to being a Port - Binding.bind(iodef, PortBinder(this), "Error: iodef") - iodef + Builder.currentModule = Some(this) + Builder.whenDepth = 0 + + // + // Module Construction Internals + // + protected var _closed = false + + // Fresh Namespace because in Firrtl, Modules namespaces are disjoint with the global namespace + private[core] val _namespace = Namespace.empty + protected val _ids = ArrayBuffer[HasId]() + private[chisel3] def addId(d: HasId) { + require(!_closed, "Can't write to module after module close") + _ids += d } - def annotate(annotation: ChiselAnnotation): Unit = { - Builder.annotations += annotation + protected val _ports = new ArrayBuffer[Data]() + + /** Generates the FIRRTL Component (Module or Blackbox) of this Module. + * Also closes the module so no more construction can happen inside. + */ + private[core] def generateComponent(): Component + + /** Sets up this module in the parent context + */ + private[core] def initializeInParent() + + // + // Chisel Internals + // + /** Desired name of this module. Override this to give this module a custom, perhaps parametric, + * name. + */ + def desiredName = this.getClass.getName.split('.').last + + /** Legalized name of this module. */ + final val name = Builder.globalNamespace.name(desiredName) + + /** Called at the Module.apply(...) level after this Module has finished elaborating. + * Returns a map of nodes -> names, for named nodes. + * + * Helper method. + */ + protected def nameIds(rootClass: Class[_]): HashMap[HasId, String] = { + val names = new HashMap[HasId, String]() + + def name(node: HasId, name: String) { + // First name takes priority, like suggestName + // TODO: DRYify with suggestName + if (!names.contains(node)) { + names.put(node, name) + } + } + + /** Recursively suggests names to supported "container" classes + * Arbitrary nestings of supported classes are allowed so long as the + * innermost element is of type HasId + * (Note: Map is Iterable[Tuple2[_,_]] and thus excluded) + */ + def nameRecursively(prefix: String, nameMe: Any): Unit = + nameMe match { + case (id: HasId) => name(id, prefix) + case Some(elt) => nameRecursively(prefix, elt) + case (iter: Iterable[_]) if iter.hasDefiniteSize => + for ((elt, i) <- iter.zipWithIndex) { + nameRecursively(s"${prefix}_${i}", elt) + } + case _ => // Do nothing + } + + /** Scala generates names like chisel3$util$Queue$$ram for private vals + * This extracts the part after $$ for names like this and leaves names + * without $$ unchanged + */ + def cleanName(name: String): String = name.split("""\$\$""").lastOption.getOrElse(name) + + for (m <- getPublicFields(rootClass)) { + nameRecursively(cleanName(m.getName), m.invoke(this)) + } + + // For Module instances we haven't named, suggest the name of the Module + _ids foreach { + case m: BaseModule => name(m, m.desiredName) + case _ => + } + + names } - private[core] var ioDefined: Boolean = false + /** Compatibility function. Allows Chisel2 code which had ports without the IO wrapper to + * compile under Bindings checks. Does nothing in non-compatibility mode. + * + * Should NOT be used elsewhere. This API will NOT last. + * + * TODO: remove this, perhaps by removing Bindings checks in compatibility mode. + */ + def _autoWrapPorts() {} + + // + // BaseModule User API functions + // + protected def annotate(annotation: ChiselAnnotation): Unit = { + Builder.annotations += annotation + } /** * This must wrap the datatype used to set the io field of any Module. @@ -120,32 +200,24 @@ extends HasId { * This will error if any node is bound (e.g. due to logic in a Bundle * constructor, which is considered improper). * + * Also registers a Data as a port, also performing bindings. Cannot be called once ports are + * requested (so that all calls to ports will return the same information). + * Internal API. + * * TODO(twigg): Specifically walk the Data definition to call out which nodes * are problematic. */ - def IO[T<:Data](iodef: T): iodef.type = { - require(!ioDefined, "Another IO definition for this module was already declared!") - ioDefined = true - - Port(iodef) - } - - // Fresh Namespace because in Firrtl, Modules namespaces are disjoint with the global namespace - private[core] val _namespace = Namespace.empty - private[chisel3] val _commands = ArrayBuffer[Command]() - private[core] val _ids = ArrayBuffer[HasId]() - Builder.currentModule = Some(this) - Builder.whenDepth = 0 - if (!Builder.readyForModuleConstr) { - throwException("Error: attempted to instantiate a Module without wrapping it in Module().") + protected def IO[T<:Data](iodef: T): iodef.type = { + require(!_closed, "Can't add more ports after module close") + // Bind each element of the iodef to being a Port + Binding.bind(iodef, PortBinder(this), "Error: iodef") + _ports += iodef + iodef } - readyForModuleConstr = false - /** Desired name of this module. */ - def desiredName = this.getClass.getName.split('.').last - - /** Legalized name of this module. */ - final val name = Builder.globalNamespace.name(desiredName) + // + // Internal Functions + // /** Keep component for signal names */ private[chisel3] var _component: Option[Component] = None @@ -157,91 +229,160 @@ extends HasId { case Some(c) => getRef fullName c } - /** IO for this Module. At the Scala level (pre-FIRRTL transformations), - * connections in and out of a Module may only go through `io` elements. - */ - def io: Record - val clock = Port(Input(Clock())) - val reset = Port(Input(Bool())) +} - // Setup ClockAndReset - Builder.currentClockAndReset = Some(ClockAndReset(clock, reset)) +/** Abstract base class for Modules that contain Chisel RTL. + */ +abstract class UserModule(implicit moduleCompileOptions: CompileOptions) + extends BaseModule { + // + // RTL construction internals + // + protected val _commands = ArrayBuffer[Command]() + private[chisel3] def addCommand(c: Command) { + require(!_closed, "Can't write to module after module close") + _commands += c + } + + // + // Other Internal Functions + // + // For debuggers/testers, TODO: refactor out into proper public API + private var _firrtlPorts: Option[Seq[firrtl.Port]] = None + lazy val getPorts = _firrtlPorts.get - private[chisel3] def addId(d: HasId) { _ids += d } + val compileOptions = moduleCompileOptions - private[core] def ports: Seq[(String,Data)] = Vector( - ("clock", clock), ("reset", reset), ("io", io) - ) + private[core] override def generateComponent(): Component = { + require(!_closed, "Can't generate module more than once") + _closed = true - private[core] def computePorts: Seq[firrtl.Port] = { - // If we're auto-wrapping IO definitions, do so now. - if (!(compileOptions.requireIOWrap || ioDefined)) { - IO(io) + val names = nameIds(classOf[UserModule]) + + // Ports get first naming priority, since they are part of a Module's IO spec + for (port <- _ports) { + require(names.contains(port), s"Unable to name port $port in $this") + port.setRef(ModuleIO(this, _namespace.name(names(port)))) + // Initialize output as unused + _commands.prepend(DefInvalid(UnlocatableSourceInfo, port.ref)) + } + + // Then everything else gets named + for ((node, name) <- names) { + node.suggestName(name) } - for ((name, port) <- ports) yield { - // Port definitions need to know input or output at top-level. - // By FIRRTL semantics, 'flipped' becomes an Input + + // All suggestions are in, force names to every node. + for (id <- _ids) { + id.forceName(default="_T", _namespace) + id._onModuleClose + } + + val firrtlPorts = for (port <- _ports) yield { + // Port definitions need to know input or output at top-level. 'flipped' means Input. val direction = if(Data.isFirrtlFlipped(port)) Direction.Input else Direction.Output firrtl.Port(port, direction) } + _firrtlPorts = Some(firrtlPorts) + + val component = DefModule(this, name, firrtlPorts, _commands) + _component = Some(component) + component } +} - private[core] def setupInParent(implicit sourceInfo: SourceInfo): this.type = { - _parent match { - case Some(p) => { - pushCommand(DefInvalid(sourceInfo, io.ref)) // init instance inputs - clock := override_clock.getOrElse(Builder.forcedClock) - reset := override_reset.getOrElse(Builder.forcedReset) - this - } - case None => this +/** Abstract base class for Modules, which behave much like Verilog modules. + * These may contain both logic and state which are written in the Module + * body (constructor). + * + * @note Module instantiations must be wrapped in a Module() call. + */ +abstract class ImplicitModule()(implicit moduleCompileOptions: CompileOptions) + extends UserModule { + // Implicit clock and reset pins + val clock = IO(Input(Clock())) + val reset = IO(Input(Bool())) + + // Setup ClockAndReset + Builder.currentClockAndReset = Some(ClockAndReset(clock, reset)) + + private[core] def initializeInParent() { + // Don't generate source info referencing parents inside a module, since this interferes with + // module de-duplication in FIRRTL emission. + implicit val sourceInfo = UnlocatableSourceInfo + + for (port <- _ports) { + pushCommand(DefInvalid(sourceInfo, port.ref)) } + + clock := Builder.forcedClock + reset := Builder.forcedReset } +} - private[core] def setRefs(): this.type = { - for ((name, port) <- ports) { - port.setRef(ModuleIO(this, _namespace.name(name))) - } +/** Legacy Module class that restricts IOs to just io, clock, and reset, and provides a constructor + * for threading through explicit clock and reset. + * + * While this class isn't planned to be removed anytime soon (there are benefits to restricting + * IO), the clock and reset constructors will be phased out. Recommendation is to wrap the module + * in a withClock/withReset/withClockAndReset block, or directly hook up clock or reset IO pins. + */ +abstract class LegacyModule( + override_clock: Option[Clock]=None, override_reset: Option[Bool]=None) + (implicit moduleCompileOptions: CompileOptions) + extends ImplicitModule { + // _clock and _reset can be clock and reset in these 2ary constructors + // once chisel2 compatibility issues are resolved + def this(_clock: Clock)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), None)(moduleCompileOptions) + def this(_reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(None, Option(_reset))(moduleCompileOptions) + def this(_clock: Clock, _reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), Option(_reset))(moduleCompileOptions) - /** Recursively suggests names to supported "container" classes - * Arbitrary nestings of supported classes are allowed so long as the - * innermost element is of type HasId - * Currently supported: - * - Iterable - * - Option - * (Note that Map is Iterable[Tuple2[_,_]] and thus excluded) - */ - def nameRecursively(prefix: String, nameMe: Any): Unit = - nameMe match { - case (id: HasId) => id.suggestName(prefix) - case Some(elt) => nameRecursively(prefix, elt) - case (iter: Iterable[_]) if iter.hasDefiniteSize => - for ((elt, i) <- iter.zipWithIndex) { - nameRecursively(s"${prefix}_${i}", elt) - } - case _ => // Do nothing - } - /** Scala generates names like chisel3$util$Queue$$ram for private vals - * This extracts the part after $$ for names like this and leaves names - * without $$ unchanged - */ - def cleanName(name: String): String = name.split("""\$\$""").lastOption.getOrElse(name) - for (m <- getPublicFields(classOf[Module])) { - nameRecursively(cleanName(m.getName), m.invoke(this)) - } + // IO for this Module. At the Scala level (pre-FIRRTL transformations), + // connections in and out of a Module may only go through `io` elements. + def io: Record - // For Module instances we haven't named, suggest the name of the Module - _ids foreach { - case m: Module => m.suggestName(m.desiredName) - case _ => + // Allow access to bindings from the compatibility package + protected def _ioPortBound() = _ports contains io + + protected override def nameIds(rootClass: Class[_]): HashMap[HasId, String] = { + val names = super.nameIds(rootClass) + + // Allow IO naming without reflection + names.put(io, "io") + names.put(clock, "clock") + names.put(reset, "reset") + + names + } + + private[core] override def generateComponent(): Component = { + _autoWrapPorts() // pre-IO(...) compatibility hack + + // Restrict IO to just io, clock, and reset + require(io != null, "Module must have io") + require(_ports contains io, "Module must have io wrapped in IO(...)") + require((_ports contains clock) && (_ports contains reset), "Internal error, module did not have clock or reset as IO") + require(_ports.size == 3, "Module must only have io, clock, and reset as IO") + + super.generateComponent() + } + + private[core] override def initializeInParent() { + // Don't generate source info referencing parents inside a module, since this interferes with + // module de-duplication in FIRRTL emission. + implicit val sourceInfo = UnlocatableSourceInfo + + pushCommand(DefInvalid(sourceInfo, io.ref)) + + override_clock match { + case Some(override_clock) => clock := override_clock + case _ => clock := Builder.forcedClock } - // All suggestions are in, force names to every node. - _ids.foreach(_.forceName(default="_T", _namespace)) - _ids.foreach(_._onModuleClose) - this + override_reset match { + case Some(override_reset) => reset := override_reset + case _ => reset := Builder.forcedReset + } } - // For debuggers/testers - lazy val getPorts = computePorts - val compileOptions = moduleCompileOptions } + diff --git a/chiselFrontend/src/main/scala/chisel3/core/MonoConnect.scala b/chiselFrontend/src/main/scala/chisel3/core/MonoConnect.scala index 9511fdc9eea..60f500cd855 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/MonoConnect.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/MonoConnect.scala @@ -55,7 +55,7 @@ object MonoConnect { * during the recursive decent and then rethrow them with extra information added. * This gives the user a 'path' to where in the connections things went wrong. */ - def connect(sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, sink: Data, source: Data, context_mod: Module): Unit = + def connect(sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, sink: Data, source: Data, context_mod: UserModule): Unit = (sink, source) match { // Handle element case (root case) case (sink_e: Element, source_e: Element) => { @@ -102,12 +102,12 @@ object MonoConnect { // This function checks if element-level connection operation allowed. // Then it either issues it or throws the appropriate exception. - def elemConnect(implicit sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, sink: Element, source: Element, context_mod: Module): Unit = { + def elemConnect(implicit sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, sink: Element, source: Element, context_mod: UserModule): Unit = { import Direction.{Input, Output} // Using extensively so import these // If source has no location, assume in context module // This can occur if is a literal, unbound will error previously - val sink_mod: Module = sink.binding.location.getOrElse(throw UnwritableSinkException) - val source_mod: Module = source.binding.location.getOrElse(context_mod) + val sink_mod: BaseModule = sink.binding.location.getOrElse(throw UnwritableSinkException) + val source_mod: BaseModule = source.binding.location.getOrElse(context_mod) val sink_direction: Option[Direction] = sink.binding.direction val source_direction: Option[Direction] = source.binding.direction diff --git a/chiselFrontend/src/main/scala/chisel3/core/Printf.scala b/chiselFrontend/src/main/scala/chisel3/core/Printf.scala index 81210f45a35..2052e95a8e6 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Printf.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Printf.scala @@ -64,6 +64,7 @@ object printf { // scalastyle:ignore object.name val clock = Builder.forcedClock pushCommand(Printf(sourceInfo, Node(clock), pable)) } + private[chisel3] def printfWithoutReset(fmt: String, data: Bits*)(implicit sourceInfo: SourceInfo): Unit = printfWithoutReset(Printable.pack(fmt, data:_*)) } diff --git a/chiselFrontend/src/main/scala/chisel3/core/Reg.scala b/chiselFrontend/src/main/scala/chisel3/core/Reg.scala index 715bdd70c53..12d0a93978c 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Reg.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Reg.scala @@ -22,7 +22,7 @@ object Reg { val reg = t.chiselCloneType val clock = Node(Builder.forcedClock) - Binding.bind(reg, RegBinder(Builder.forcedModule), "Error: t") + Binding.bind(reg, RegBinder(Builder.forcedUserModule), "Error: t") pushCommand(DefReg(sourceInfo, reg, clock)) reg } @@ -90,7 +90,7 @@ object RegInit { val clock = Node(Builder.forcedClock) val reset = Node(Builder.forcedReset) - Binding.bind(reg, RegBinder(Builder.forcedModule), "Error: t") + Binding.bind(reg, RegBinder(Builder.forcedUserModule), "Error: t") Binding.checkSynthesizable(init, s"'init' ($init)") pushCommand(DefRegInit(sourceInfo, reg, clock, reset, init.ref)) reg diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala index e0cbf302ab4..73556750cce 100644 --- a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala +++ b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala @@ -70,7 +70,7 @@ trait InstanceId { private[chisel3] trait HasId extends InstanceId { private[chisel3] def _onModuleClose: Unit = {} // scalastyle:ignore method.name - private[chisel3] val _parent: Option[Module] = Builder.currentModule + private[chisel3] val _parent: Option[BaseModule] = Builder.currentModule _parent.foreach(_.addId(this)) private[chisel3] val _id: Long = Builder.idGen.next @@ -148,7 +148,7 @@ private[chisel3] class DynamicContext() { val globalNamespace = Namespace.empty val components = ArrayBuffer[Component]() val annotations = ArrayBuffer[ChiselAnnotation]() - var currentModule: Option[Module] = None + var currentModule: Option[BaseModule] = None // Set by object Module.apply before calling class Module constructor // Used to distinguish between no Module() wrapping, multiple wrappings, and rewrapping var readyForModuleConstr: Boolean = false @@ -170,17 +170,24 @@ private[chisel3] object Builder { def annotations: ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations def namingStack: internal.naming.NamingStack = dynamicContext.namingStack - def currentModule: Option[Module] = dynamicContext.currentModule - def currentModule_=(target: Option[Module]): Unit = { + def currentModule: Option[BaseModule] = dynamicContext.currentModule + def currentModule_=(target: Option[BaseModule]): Unit = { dynamicContext.currentModule = target } - def forcedModule: Module = currentModule match { + def forcedModule: BaseModule = currentModule match { case Some(module) => module case None => throwException( "Error: Not in a Module. Likely cause: Missed Module() wrap or bare chisel API call." // A bare api call is, e.g. calling Wire() from the scala console). ) } + def forcedUserModule: UserModule = currentModule match { + case Some(module: UserModule) => module + case _ => throwException( + "Error: Not in a UserModule. Likely cause: Missed Module() wrap, bare chisel API call, or attempting to construct hardware inside a BlackBox." + // A bare api call is, e.g. calling Wire() from the scala console). + ) + } def readyForModuleConstr: Boolean = dynamicContext.readyForModuleConstr def readyForModuleConstr_=(target: Boolean): Unit = { dynamicContext.readyForModuleConstr = target @@ -203,15 +210,12 @@ private[chisel3] object Builder { // TODO(twigg): Ideally, binding checks and new bindings would all occur here // However, rest of frontend can't support this yet. def pushCommand[T <: Command](c: T): T = { - forcedModule match { - case _: BlackBox => throwException("Cannot add hardware to a BlackBox") - case m => m._commands += c - } + forcedUserModule.addCommand(c) c } def pushOp[T <: Data](cmd: DefPrim[T]): T = { // Bind each element of the returned Data to being a Op - Binding.bind(cmd.id, OpBinder(forcedModule), "Error: During op creation, fresh result") + Binding.bind(cmd.id, OpBinder(forcedUserModule), "Error: During op creation, fresh result") pushCommand(cmd).id } @@ -230,7 +234,7 @@ private[chisel3] object Builder { throwException(m) } - def build[T <: Module](f: => T): Circuit = { + def build[T <: UserModule](f: => T): Circuit = { dynamicContextVar.withValue(Some(new DynamicContext())) { errors.info("Elaborating design...") val mod = f diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Error.scala b/chiselFrontend/src/main/scala/chisel3/internal/Error.scala index bba7c806818..25a3ec2a1ca 100644 --- a/chiselFrontend/src/main/scala/chisel3/internal/Error.scala +++ b/chiselFrontend/src/main/scala/chisel3/internal/Error.scala @@ -55,7 +55,7 @@ private[chisel3] class ErrorLog { private def findFirstUserFrame(stack: Array[StackTraceElement]): Option[StackTraceElement] = { def isUserCode(ste: StackTraceElement): Boolean = { def isUserModule(c: Class[_]): Boolean = - c != null && (c == classOf[Module] || isUserModule(c.getSuperclass)) + c != null && (c == classOf[UserModule] || isUserModule(c.getSuperclass)) isUserModule(Class.forName(ste.getClassName)) } diff --git a/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala b/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala index bee728178d8..18df7f519a5 100644 --- a/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala +++ b/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala @@ -98,7 +98,7 @@ case class FPLit(n: BigInt, w: Width, binaryPoint: BinaryPoint) extends LitArg(n } case class Ref(name: String) extends Arg -case class ModuleIO(mod: Module, name: String) extends Arg { +case class ModuleIO(mod: BaseModule, name: String) extends Arg { override def fullName(ctx: Component): String = if (mod eq ctx.id) name else s"${mod.getRef.name}.$name" } @@ -258,7 +258,7 @@ case class DefRegInit(sourceInfo: SourceInfo, id: Data, clock: Arg, reset: Arg, case class DefMemory(sourceInfo: SourceInfo, id: HasId, t: Data, size: Int) extends Definition case class DefSeqMemory(sourceInfo: SourceInfo, id: HasId, t: Data, size: Int) extends Definition case class DefMemPort[T <: Data](sourceInfo: SourceInfo, id: T, source: Node, dir: MemPortDirection, index: Arg, clock: Arg) extends Definition -case class DefInstance(sourceInfo: SourceInfo, id: Module, ports: Seq[Port]) extends Definition +case class DefInstance(sourceInfo: SourceInfo, id: BaseModule, ports: Seq[Port]) extends Definition case class WhenBegin(sourceInfo: SourceInfo, pred: Arg) extends Command case class WhenEnd(sourceInfo: SourceInfo) extends Command case class Connect(sourceInfo: SourceInfo, loc: Node, exp: Arg) extends Command @@ -269,11 +269,11 @@ case class Stop(sourceInfo: SourceInfo, clock: Arg, ret: Int) extends Command case class Port(id: Data, dir: Direction) case class Printf(sourceInfo: SourceInfo, clock: Arg, pable: Printable) extends Command abstract class Component extends Arg { - def id: Module + def id: BaseModule def name: String def ports: Seq[Port] } -case class DefModule(id: Module, name: String, ports: Seq[Port], commands: Seq[Command]) extends Component -case class DefBlackBox(id: Module, name: String, ports: Seq[Port], params: Map[String, Param]) extends Component +case class DefModule(id: UserModule, name: String, ports: Seq[Port], commands: Seq[Command]) extends Component +case class DefBlackBox(id: BaseBlackBox, name: String, ports: Seq[Port], params: Map[String, Param]) extends Component case class Circuit(name: String, components: Seq[Component], annotations: Seq[Annotation] = Seq.empty) diff --git a/src/main/scala/chisel3/compatibility.scala b/src/main/scala/chisel3/compatibility.scala index 40fbe9bf0c4..365c1ba5fe1 100644 --- a/src/main/scala/chisel3/compatibility.scala +++ b/src/main/scala/chisel3/compatibility.scala @@ -152,16 +152,43 @@ package object Chisel { // scalastyle:ignore package.object.name object Bool extends BoolFactory val Mux = chisel3.core.Mux - type BlackBox = chisel3.core.BlackBox + import chisel3.core.Param + abstract class BlackBox(params: Map[String, Param] = Map.empty[String, Param]) extends chisel3.core.BlackBox(params) { + var _ioDefined = false + override protected def IO[T<:Data](iodef: T): iodef.type = { + _ioDefined = true + super.IO(iodef) + } + override def _autoWrapPorts() = { + if (!_ioDefined) { + IO(io) + } + } + } val Mem = chisel3.core.Mem type MemBase[T <: Data] = chisel3.core.MemBase[T] type Mem[T <: Data] = chisel3.core.Mem[T] val SeqMem = chisel3.core.SyncReadMem type SeqMem[T <: Data] = chisel3.core.SyncReadMem[T] + import chisel3.core.CompileOptions + abstract class CompatibilityModule( + override_clock: Option[Clock]=None, override_reset: Option[Bool]=None) + (implicit moduleCompileOptions: CompileOptions) + extends chisel3.core.LegacyModule(override_clock, override_reset) { + def this(_clock: Clock)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), None)(moduleCompileOptions) + def this(_reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(None, Option(_reset))(moduleCompileOptions) + def this(_clock: Clock, _reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), Option(_reset))(moduleCompileOptions) + + override def _autoWrapPorts() = { + if (!_ioPortBound()) { + IO(io) + } + } + } val Module = chisel3.core.Module - type Module = chisel3.core.Module + type Module = CompatibilityModule val printf = chisel3.core.printf diff --git a/src/main/scala/chisel3/internal/firrtl/Emitter.scala b/src/main/scala/chisel3/internal/firrtl/Emitter.scala index eb00e333bee..16b39e35b03 100644 --- a/src/main/scala/chisel3/internal/firrtl/Emitter.scala +++ b/src/main/scala/chisel3/internal/firrtl/Emitter.scala @@ -57,8 +57,8 @@ private class Emitter(circuit: Circuit) { /** Generates the FIRRTL module declaration. */ private def moduleDecl(m: Component): String = m.id match { - case _: BlackBox => newline + s"extmodule ${m.name} : " - case _: Module => newline + s"module ${m.name} : " + case _: chisel3.core.BaseBlackBox => newline + s"extmodule ${m.name} : " + case _: chisel3.core.UserModule => newline + s"module ${m.name} : " } /** Generates the FIRRTL module definition. diff --git a/src/main/scala/chisel3/package.scala b/src/main/scala/chisel3/package.scala index 191b636eb35..a7ccc43b28f 100644 --- a/src/main/scala/chisel3/package.scala +++ b/src/main/scala/chisel3/package.scala @@ -151,7 +151,7 @@ package object chisel3 { // scalastyle:ignore package.object.name type SyncReadMem[T <: Data] = chisel3.core.SyncReadMem[T] val Module = chisel3.core.Module - type Module = chisel3.core.Module + type Module = chisel3.core.LegacyModule val printf = chisel3.core.printf @@ -310,6 +310,11 @@ package object chisel3 { // scalastyle:ignore package.object.name val withClock = chisel3.core.withClock val withReset = chisel3.core.withReset + type BaseModule = chisel3.core.BaseModule + type MultiIOModule = chisel3.core.ImplicitModule + type RawModule = chisel3.core.UserModule + type ExtModule = chisel3.core.ExtModule + // Implicit conversions for BlackBox Parameters implicit def fromIntToIntParam(x: Int): IntParam = IntParam(BigInt(x)) implicit def fromLongToIntParam(x: Long): IntParam = IntParam(BigInt(x)) diff --git a/src/main/scala/chisel3/util/BlackBoxUtils.scala b/src/main/scala/chisel3/util/BlackBoxUtils.scala index 084d58f9fd5..fbcf4a59a24 100644 --- a/src/main/scala/chisel3/util/BlackBoxUtils.scala +++ b/src/main/scala/chisel3/util/BlackBoxUtils.scala @@ -7,7 +7,7 @@ import chisel3.core.ChiselAnnotation import firrtl.transforms.{BlackBoxInline, BlackBoxResource, BlackBoxSourceHelper} trait HasBlackBoxResource extends BlackBox { - self: Module => + self: BlackBox => def setResource(blackBoxResource: String): Unit = { annotate(ChiselAnnotation(self, classOf[BlackBoxSourceHelper], BlackBoxResource(blackBoxResource).serialize)) @@ -15,7 +15,7 @@ trait HasBlackBoxResource extends BlackBox { } trait HasBlackBoxInline extends BlackBox { - self: Module => + self: BlackBox => def setInline(blackBoxName: String, blackBoxInline: String): Unit = { annotate(ChiselAnnotation( diff --git a/src/test/scala/chiselTests/AnalogIntegrationSpec.scala b/src/test/scala/chiselTests/AnalogIntegrationSpec.scala index 92f89e06da6..de717c4f321 100644 --- a/src/test/scala/chiselTests/AnalogIntegrationSpec.scala +++ b/src/test/scala/chiselTests/AnalogIntegrationSpec.scala @@ -31,11 +31,18 @@ class AnalogBlackBox(index: Int) extends BlackBox(Map("index" -> index)) { val io = IO(new AnalogBlackBoxIO(1)) } +// AnalogBlackBox wrapper, which extends Module to present the common io._ interface +class AnalogBlackBoxModule(index: Int) extends Module { + val io = IO(new AnalogBlackBoxIO(1)) + val impl = Module(new AnalogBlackBox(index)) + io <> impl.io +} + // Wraps up n blackboxes, connecing their buses and simply forwarding their ports up class AnalogBlackBoxWrapper(n: Int, idxs: Seq[Int]) extends Module { require(n > 0) val io = IO(new AnalogBlackBoxIO(n)) - val bbs = idxs.map(i => Module(new AnalogBlackBox(i))) + val bbs = idxs.map(i => Module(new AnalogBlackBoxModule(i))) io.bus <> bbs.head.io.bus // Always bulk connect io.bus to first bus io.port <> bbs.flatMap(_.io.port) // Connect ports attach(bbs.map(_.io.bus):_*) // Attach all the buses @@ -58,9 +65,9 @@ abstract class AnalogDUTModule(numBlackBoxes: Int) extends Module { class AnalogDUT extends AnalogDUTModule(5) { // 5 BlackBoxes val mods = Seq( Module(new AnalogBlackBoxWrapper(1, Seq(0))), - Module(new AnalogBlackBox(1)), + Module(new AnalogBlackBoxModule(1)), Module(new AnalogBlackBoxWrapper(2, Seq(2, 3))), // 2 blackboxes - Module(new AnalogBlackBox(4)) + Module(new AnalogBlackBoxModule(4)) ) // Connect all ports to top io.ports <> mods.flatMap(_.io.port) @@ -79,7 +86,7 @@ class AnalogDUT extends AnalogDUTModule(5) { // 5 BlackBoxes class AnalogSmallDUT extends AnalogDUTModule(4) { // 4 BlackBoxes val mods = Seq( Module(new AnalogBlackBoxWrapper(1, Seq(0))), - Module(new AnalogBlackBox(1)), + Module(new AnalogBlackBoxModule(1)), Module(new AnalogBlackBoxWrapper(2, Seq(2, 3))) // 2 BlackBoxes ) // Connect all ports to top diff --git a/src/test/scala/chiselTests/AnalogSpec.scala b/src/test/scala/chiselTests/AnalogSpec.scala index 576a5a1f3c3..5db9ab5305b 100644 --- a/src/test/scala/chiselTests/AnalogSpec.scala +++ b/src/test/scala/chiselTests/AnalogSpec.scala @@ -5,7 +5,7 @@ package chiselTests import chisel3._ import chisel3.util._ import chisel3.testers.BasicTester -import chisel3.experimental.{Analog, attach} +import chisel3.experimental.{Analog, attach, BaseModule} // IO for Modules that just connect bus to out class AnalogReaderIO extends Bundle { @@ -19,7 +19,7 @@ class AnalogWriterIO extends Bundle { } trait AnalogReader { - self: Module => + self: BaseModule => final val io = self.IO(new AnalogReaderIO) } @@ -51,7 +51,7 @@ abstract class AnalogTester extends BasicTester { final val writer = Module(new AnalogWriterBlackBox) writer.io.in := BusValue - final def check(reader: Module with AnalogReader): Unit = + final def check(reader: BaseModule with AnalogReader): Unit = assert(reader.io.out === BusValue) } diff --git a/src/test/scala/chiselTests/BetterNamingTests.scala b/src/test/scala/chiselTests/BetterNamingTests.scala index 301ab5d35a8..a660086f69c 100644 --- a/src/test/scala/chiselTests/BetterNamingTests.scala +++ b/src/test/scala/chiselTests/BetterNamingTests.scala @@ -7,10 +7,11 @@ import chisel3.util._ // Defined outside of the class so we don't get $ in name class Other(w: Int) extends Module { - val io = new Bundle { + val io = IO(new Bundle { val a = UInt(w.W) - } + }) } + // Check the names of the Modules (not instances) class PerNameIndexing(count: Int) extends NamedModuleTester { def genModName(prefix: String, idx: Int): String = if (idx == 0) prefix else s"${prefix}_$idx" diff --git a/src/test/scala/chiselTests/BundleEltsSpec.scala b/src/test/scala/chiselTests/BundleEltsSpec.scala new file mode 100644 index 00000000000..e9f7829cc73 --- /dev/null +++ b/src/test/scala/chiselTests/BundleEltsSpec.scala @@ -0,0 +1,84 @@ +// See LICENSE for license details. + +package chiselTests + +import chisel3._ +import chisel3.internal.firrtl.Width +import chisel3.testers.BasicTester + +import chisel3.experimental._ + +//trait BundleEltsSpecUtils { +// class BundleWidthModule extends Module { +// class BundleWidth[T <: Data](val genType: T, widthx: Width) extends Bundle { +// val gen = genType.chiselCloneType +// val in = Input(UInt(widthx)) +// val out = Output(UInt(widthx)) +// +// def getIntWidth = widthx +//// override def cloneType = new BundleWidth(genType, width).asInstanceOf[this.type] +// +// def GPF(rootClass: Class[_]) = { +// import reflect.runtime._,universe._ +// val im = currentMirror reflect this +// val vals = im.symbol.asClass.typeSignature.members filter (s => s.isTerm && s.asTerm.isAccessor) +// val methods = vals map (im reflectMethod _.asMethod) +// +// // Suggest names to nodes using runtime reflection +// def getValFields(c: Class[_]): Set[java.lang.reflect.Field] = { +// if (c == rootClass) Set() +// else getValFields(c.getSuperclass) ++ c.getDeclaredFields +// } +// val valNames = getValFields(this.getClass).map(_.getName) +// def isPublicVal(m: java.lang.reflect.Method) = +// m.getParameterTypes.isEmpty && valNames.contains(m.getName) && m.getDeclaringClass.isAssignableFrom(rootClass) +// getValFields(this.getClass).map(x => print(s"VN ${x.getName} => ${x.get(this)}\r\n")) +// this.getClass.getMethods.map(x => print(s"GM ${x.getName}\r\n")) +// this.getClass.getMethods.sortWith(_.getName < _.getName).filter(isPublicVal(_)) +// } +// } +// val io = IO(new BundleWidth(UInt(2.W), 2.W)) +//// val int = io.chiselCloneType +// io.out := io.in + 1.U +// +// import chisel3.core.DataMirror +// print(s"GenWidth ${DataMirror.widthOf(io)}\r\n") +// print(s"GPF ${io.GPF(classOf[Bundle])}\r\n") +// } +// +// @chiselName +// @dump +// class MySimpleModule extends Module { +// val io = IO(new Bundle { +// val in = Input(UInt(32.W)) +// val out = Output(UInt(32.W)) +// }) +// +// for (i <- 0 until 1) { +// val a = Wire(init = io.in) +// val b = Wire(init = a) +// io.out := b +// } +// } +// +// @chiselName +// @dump +// class MySimpleAFodule extends Module { +// val io = IO(new Bundle { +// val in = Input(UInt(32.W)) +// val out = Output(UInt(32.W)) +// }) +// +// Seq(io.in).foreach { in => +// val a = Wire(init = in) +// val b = Wire(init = a) +// io.out := b +// } +// } +//} +// +//class BundleEltsSpec extends ChiselFlatSpec with BundleEltsSpecUtils { +// "Bundles with the same fields but in different orders" should "bulk connect" in { +// elaborate { new BundleWidthModule } +// } +//} diff --git a/src/test/scala/chiselTests/CompileOptionsTest.scala b/src/test/scala/chiselTests/CompileOptionsTest.scala index 63e6b9d144e..102653af7ad 100644 --- a/src/test/scala/chiselTests/CompileOptionsTest.scala +++ b/src/test/scala/chiselTests/CompileOptionsTest.scala @@ -14,10 +14,6 @@ class CompileOptionsSpec extends ChiselFlatSpec { abstract class StrictModule extends Module()(chisel3.core.ExplicitCompileOptions.Strict) abstract class NotStrictModule extends Module()(chisel3.core.ExplicitCompileOptions.NotStrict) - // Generate a set of options that do not have requireIOWrap enabled, in order to - // ensure its definition comes from the implicit options passed to the Module constructor. - val strictWithoutIOWrapVal = chisel3.core.ExplicitCompileOptions.Strict.copy(requireIOWrap = false) - class SmallBundle extends Bundle { val f1 = UInt(4.W) val f2 = UInt(5.W) @@ -95,19 +91,6 @@ class CompileOptionsSpec extends ChiselFlatSpec { io.out := io.in(1) } elaborate { new RequireIOWrapModule() } -} - - "A Module with unwrapped IO when compiled with implicit NotStrict.CompileOption " should "not throw an exception" in { - import chisel3.core.ExplicitCompileOptions.NotStrict - - class RequireIOWrapModule extends Module { - val io = new Bundle { - val in = UInt(32.W).asInput - val out = Bool().asOutput - } - io.out := io.in(1) - } - elaborate { new RequireIOWrapModule() } } "A Module with unwrapped IO when compiled with implicit Strict.CompileOption " should "throw an exception" in { @@ -204,63 +187,4 @@ class CompileOptionsSpec extends ChiselFlatSpec { } elaborate { new DirectionLessConnectionModule() } } - - "A Module with wrapped IO when compiled with explicit Strict.CompileOption " should "not throw an exception" in { - implicit val strictWithoutIOWrap = strictWithoutIOWrapVal - class RequireIOWrapModule extends StrictModule { - val io = IO(new Bundle { - val in = UInt(32.W).asInput - val out = Bool().asOutput - }) - io.out := io.in(1) - } - elaborate { - new RequireIOWrapModule() - } - } - - "A Module with unwrapped IO when compiled with explicit NotStrict.CompileOption " should "not throw an exception" in { - implicit val strictWithoutIOWrap = strictWithoutIOWrapVal - class RequireIOWrapModule extends NotStrictModule { - val io = new Bundle { - val in = UInt(32.W).asInput - val out = Bool().asOutput - } - io.out := io.in(1) - } - elaborate { - new RequireIOWrapModule() - } - } - - "A Module with unwrapped IO when compiled with explicit Strict.CompileOption " should "throw an exception" in { - a [BindingException] should be thrownBy { - implicit val strictWithoutIOWrap = strictWithoutIOWrapVal - class RequireIOWrapModule extends StrictModule { - val io = new Bundle { - val in = UInt(32.W).asInput - val out = Bool().asOutput - } - io.out := io.in(1) - } - elaborate { - new RequireIOWrapModule() - } - } - } - - "A Module with unwrapped IO when compiled with an explicit requireIOWrap false " should "not throw an exception" in { - - val strictNotIOWrap = chisel3.core.ExplicitCompileOptions.Strict.copy(requireIOWrap = false, deprecateOldDirectionMethods = false) - - class NotIOWrapModule extends Module()(strictNotIOWrap) { - val io = new Bundle { - val in = UInt(32.W).asInput - val out = Bool().asOutput - } - } - elaborate { - new NotIOWrapModule() - } - } } diff --git a/src/test/scala/chiselTests/ExtModule.scala b/src/test/scala/chiselTests/ExtModule.scala new file mode 100644 index 00000000000..bcf09e602ff --- /dev/null +++ b/src/test/scala/chiselTests/ExtModule.scala @@ -0,0 +1,72 @@ +// See LICENSE for license details. + +package chiselTests + +import java.io.File + +import org.scalatest._ +import chisel3._ +import chisel3.experimental._ +import chisel3.testers.BasicTester +import chisel3.util._ +//import chisel3.core.ExplicitCompileOptions.Strict + +// Avoid collisions with regular BlackBox tests by putting ExtModule blackboxes +// in their own scope. +package ExtModule { + class BlackBoxInverter extends ExtModule { + val in = IO(Input(Bool())) + val out = IO(Output(Bool())) + } + + class BlackBoxPassthrough extends ExtModule { + val in = IO(Input(Bool())) + val out = IO(Output(Bool())) + } +} + +class ExtModuleTester extends BasicTester { + val blackBoxPos = Module(new ExtModule.BlackBoxInverter) + val blackBoxNeg = Module(new ExtModule.BlackBoxInverter) + + blackBoxPos.in := 1.U + blackBoxNeg.in := 0.U + + assert(blackBoxNeg.out === 1.U) + assert(blackBoxPos.out === 0.U) + stop() +} + +/** Instantiate multiple BlackBoxes with similar interfaces but different + * functionality. Used to detect failures in BlackBox naming and module + * deduplication. + */ + +class MultiExtModuleTester extends BasicTester { + val blackBoxInvPos = Module(new ExtModule.BlackBoxInverter) + val blackBoxInvNeg = Module(new ExtModule.BlackBoxInverter) + val blackBoxPassPos = Module(new ExtModule.BlackBoxPassthrough) + val blackBoxPassNeg = Module(new ExtModule.BlackBoxPassthrough) + + blackBoxInvPos.in := 1.U + blackBoxInvNeg.in := 0.U + blackBoxPassPos.in := 1.U + blackBoxPassNeg.in := 0.U + + assert(blackBoxInvNeg.out === 1.U) + assert(blackBoxInvPos.out === 0.U) + assert(blackBoxPassNeg.out === 0.U) + assert(blackBoxPassPos.out === 1.U) + stop() +} + +class ExtModuleSpec extends ChiselFlatSpec { + "A ExtModule inverter" should "work" in { + assertTesterPasses({ new ExtModuleTester }, + Seq("/BlackBoxTest.v")) + } + "Multiple ExtModules" should "work" in { + assertTesterPasses({ new MultiExtModuleTester }, + Seq("/BlackBoxTest.v")) + } +} diff --git a/src/test/scala/chiselTests/MultiIOModule.scala b/src/test/scala/chiselTests/MultiIOModule.scala new file mode 100644 index 00000000000..6a6b4dd6534 --- /dev/null +++ b/src/test/scala/chiselTests/MultiIOModule.scala @@ -0,0 +1,27 @@ +// See LICENSE for license details. + +package chiselTests + +import chisel3._ +import chisel3.experimental.MultiIOModule +import chisel3.testers.BasicTester + +class MultiIOPlusOne extends MultiIOModule { + val in = IO(Input(UInt(32.W))) + val out = IO(Output(UInt(32.W))) + + out := in + 1.asUInt +} + +class MultiIOTester extends BasicTester { + val plusModule = Module(new MultiIOPlusOne) + plusModule.in := 42.U + assert(plusModule.out === 43.U) + stop() +} + +class MultiIOSpec extends ChiselFlatSpec { + "Multiple IOs in MultiIOModule" should "work" in { + assertTesterPasses({ new MultiIOTester }) + } +} diff --git a/src/test/scala/chiselTests/PrintableSpec.scala b/src/test/scala/chiselTests/PrintableSpec.scala index 62784cff8b8..5f58429e0b1 100644 --- a/src/test/scala/chiselTests/PrintableSpec.scala +++ b/src/test/scala/chiselTests/PrintableSpec.scala @@ -101,9 +101,9 @@ class PrintableSpec extends FlatSpec with Matchers { // Submodule IO is a subtle issue because the Chisel element has a different // parent module class MySubModule extends Module { - val io = new Bundle { + val io = IO(new Bundle { val fizz = UInt(32.W) - } + }) } class MyBundle extends Bundle { val foo = UInt(32.W) @@ -128,9 +128,9 @@ class PrintableSpec extends FlatSpec with Matchers { } it should "handle printing ports of submodules" in { class MySubModule extends Module { - val io = new Bundle { + val io = IO(new Bundle { val fizz = UInt(32.W) - } + }) } class MyModule extends BasicTester { val myInst = Module(new MySubModule) diff --git a/src/test/scala/cookbook/CookbookSpec.scala b/src/test/scala/cookbook/CookbookSpec.scala index 554a415f97b..638ebd46756 100644 --- a/src/test/scala/cookbook/CookbookSpec.scala +++ b/src/test/scala/cookbook/CookbookSpec.scala @@ -10,7 +10,7 @@ import chiselTests.ChiselFlatSpec /** Tester for concise cookbook tests * - * Provides a length of test after which the test will pass + * Provides a length of test after which the test will pass */ abstract class CookbookTester(length: Int) extends BasicTester { require(length >= 0, "Simulation length must be non-negative!") From 62bd0e6f0b125fa06485ed46e80cdb2e505dc312 Mon Sep 17 00:00:00 2001 From: ducky Date: Tue, 21 Mar 2017 14:40:12 -0700 Subject: [PATCH 2/6] Address review comments --- .../main/scala/chisel3/core/BlackBox.scala | 28 +-- .../src/main/scala/chisel3/core/Module.scala | 187 ++---------------- .../main/scala/chisel3/core/UserModule.scala | 174 ++++++++++++++++ src/main/scala/chisel3/compatibility.scala | 14 +- .../scala/chiselTests/BundleEltsSpec.scala | 84 -------- 5 files changed, 215 insertions(+), 272 deletions(-) create mode 100644 chiselFrontend/src/main/scala/chisel3/core/UserModule.scala delete mode 100644 src/test/scala/chiselTests/BundleEltsSpec.scala diff --git a/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala b/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala index 621ddd7b285..7a1394bbb3a 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala @@ -63,18 +63,20 @@ abstract class ExtModule(val params: Map[String, Param] = Map.empty[String, Para val names = nameIds(classOf[ExtModule]) // Name ports based on reflection - for (port <- _ports) { + for (port <- getModulePorts) { require(names.contains(port), s"Unable to name port $port in $this") port.setRef(ModuleIO(this, _namespace.name(names(port)))) } // All suggestions are in, force names to every node. - for (id <- _ids) { + // While BlackBoxes are not supposed to have an implementation, we still need to call + // _onModuleClose on all nodes (for example, Aggregates use it for recursive naming). + for (id <- getIds) { id.forceName(default="_T", _namespace) id._onModuleClose } - val firrtlPorts = for (port <- _ports) yield { + val firrtlPorts = for (port <- getModulePorts) yield { // Port definitions need to know input or output at top-level. // By FIRRTL semantics, 'flipped' becomes an Input val direction = if(Data.isFirrtlFlipped(port)) Direction.Input else Direction.Output @@ -89,7 +91,7 @@ abstract class ExtModule(val params: Map[String, Param] = Map.empty[String, Para private[core] def initializeInParent() { implicit val sourceInfo = UnlocatableSourceInfo - _ports.foreach { x: Data => + for (x <- getModulePorts) { pushCommand(DefInvalid(sourceInfo, x.ref)) } } @@ -132,14 +134,17 @@ abstract class ExtModule(val params: Map[String, Param] = Map.empty[String, Para */ abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param]) extends BaseBlackBox { def io: Record + + // Allow access to bindings from the compatibility package + protected def _ioPortBound() = portsContains(io) private[core] override def generateComponent(): Component = { _autoWrapPorts() // pre-IO(...) compatibility hack // Restrict IO to just io, clock, and reset require(io != null, "BlackBox must have io") - require(_ports contains io, "BlackBox must have io wrapped in IO(...)") - require(_ports.size == 1, "BlackBox must only have io as IO") + require(portsContains(io), "BlackBox must have io wrapped in IO(...)") + require(portsSize == 1, "BlackBox must only have io as IO") require(!_closed, "Can't generate module more than once") _closed = true @@ -157,7 +162,7 @@ abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param // of the io bundle, but NOT on the io bundle itself. // Doing so would cause the wrong names to be assigned, since their parent // is now the module itself instead of the io bundle. - for (id <- _ids; if id ne io) { + for (id <- getIds; if id ne io) { id.forceName(default="_T", _namespace) id._onModuleClose } @@ -174,12 +179,9 @@ abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param component } - private[core] def initializeInParent() { - implicit val sourceInfo = UnlocatableSourceInfo - - val namedPorts = io.elements.toSeq - for ((_, port) <- namedPorts) { - pushCommand(DefInvalid(sourceInfo, port.ref)) + private[core] def initializeInParent() { + for ((_, port) <- io.elements) { + pushCommand(DefInvalid(UnlocatableSourceInfo, port.ref)) } } } diff --git a/chiselFrontend/src/main/scala/chisel3/core/Module.scala b/chiselFrontend/src/main/scala/chisel3/core/Module.scala index d2c33cd59c4..4c4c0c01575 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Module.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Module.scala @@ -11,8 +11,7 @@ import java.util.IdentityHashMap import chisel3.internal._ import chisel3.internal.Builder._ import chisel3.internal.firrtl._ -import chisel3.internal.firrtl.{Command => _, _} -import chisel3.internal.sourceinfo.{InstTransform, SourceInfo, UnlocatableSourceInfo} +import chisel3.internal.sourceinfo.{InstTransform, SourceInfo} object Module { /** A wrapper method that all Module instantiations must be wrapped in @@ -31,7 +30,7 @@ object Module { } Builder.readyForModuleConstr = true - val parent: Option[BaseModule] = Builder.currentModule + val parent = Builder.currentModule val whenDepth: Int = Builder.whenDepth val clockAndReset: Option[ClockAndReset] = Builder.currentClockAndReset @@ -40,7 +39,7 @@ object Module { // - unset readyForModuleConstr // - reset whenDepth to 0 // - set currentClockAndReset - val module: T = bc // Module is actually invoked here + val module: T = bc // bc is actually evaluated here if (Builder.whenDepth != 0) { throwException("Internal Error! When depth is != 0, this should not be possible") @@ -93,13 +92,27 @@ abstract class BaseModule extends HasId { // Fresh Namespace because in Firrtl, Modules namespaces are disjoint with the global namespace private[core] val _namespace = Namespace.empty - protected val _ids = ArrayBuffer[HasId]() + private val _ids = ArrayBuffer[HasId]() private[chisel3] def addId(d: HasId) { require(!_closed, "Can't write to module after module close") _ids += d } + protected def getIds = { + require(_closed, "Can't get ids before module close") + _ids.toSeq + } - protected val _ports = new ArrayBuffer[Data]() + private val _ports = new ArrayBuffer[Data]() + // getPorts unfortunately already used for tester compatibility + protected def getModulePorts = { + require(_closed, "Can't get ports before module close") + _ports.toSeq + } + + // These methods allow checking some properties of ports before the module is closed, + // mainly for compatibility purposes. + protected def portsContains(elem: Data): Boolean = _ports contains elem + protected def portsSize: Int = _ports.size /** Generates the FIRRTL Component (Module or Blackbox) of this Module. * Also closes the module so no more construction can happen inside. @@ -163,12 +176,6 @@ abstract class BaseModule extends HasId { nameRecursively(cleanName(m.getName), m.invoke(this)) } - // For Module instances we haven't named, suggest the name of the Module - _ids foreach { - case m: BaseModule => name(m, m.desiredName) - case _ => - } - names } @@ -230,159 +237,3 @@ abstract class BaseModule extends HasId { } } - -/** Abstract base class for Modules that contain Chisel RTL. - */ -abstract class UserModule(implicit moduleCompileOptions: CompileOptions) - extends BaseModule { - // - // RTL construction internals - // - protected val _commands = ArrayBuffer[Command]() - private[chisel3] def addCommand(c: Command) { - require(!_closed, "Can't write to module after module close") - _commands += c - } - - // - // Other Internal Functions - // - // For debuggers/testers, TODO: refactor out into proper public API - private var _firrtlPorts: Option[Seq[firrtl.Port]] = None - lazy val getPorts = _firrtlPorts.get - - val compileOptions = moduleCompileOptions - - private[core] override def generateComponent(): Component = { - require(!_closed, "Can't generate module more than once") - _closed = true - - val names = nameIds(classOf[UserModule]) - - // Ports get first naming priority, since they are part of a Module's IO spec - for (port <- _ports) { - require(names.contains(port), s"Unable to name port $port in $this") - port.setRef(ModuleIO(this, _namespace.name(names(port)))) - // Initialize output as unused - _commands.prepend(DefInvalid(UnlocatableSourceInfo, port.ref)) - } - - // Then everything else gets named - for ((node, name) <- names) { - node.suggestName(name) - } - - // All suggestions are in, force names to every node. - for (id <- _ids) { - id.forceName(default="_T", _namespace) - id._onModuleClose - } - - val firrtlPorts = for (port <- _ports) yield { - // Port definitions need to know input or output at top-level. 'flipped' means Input. - val direction = if(Data.isFirrtlFlipped(port)) Direction.Input else Direction.Output - firrtl.Port(port, direction) - } - _firrtlPorts = Some(firrtlPorts) - - val component = DefModule(this, name, firrtlPorts, _commands) - _component = Some(component) - component - } -} - -/** Abstract base class for Modules, which behave much like Verilog modules. - * These may contain both logic and state which are written in the Module - * body (constructor). - * - * @note Module instantiations must be wrapped in a Module() call. - */ -abstract class ImplicitModule()(implicit moduleCompileOptions: CompileOptions) - extends UserModule { - // Implicit clock and reset pins - val clock = IO(Input(Clock())) - val reset = IO(Input(Bool())) - - // Setup ClockAndReset - Builder.currentClockAndReset = Some(ClockAndReset(clock, reset)) - - private[core] def initializeInParent() { - // Don't generate source info referencing parents inside a module, since this interferes with - // module de-duplication in FIRRTL emission. - implicit val sourceInfo = UnlocatableSourceInfo - - for (port <- _ports) { - pushCommand(DefInvalid(sourceInfo, port.ref)) - } - - clock := Builder.forcedClock - reset := Builder.forcedReset - } -} - -/** Legacy Module class that restricts IOs to just io, clock, and reset, and provides a constructor - * for threading through explicit clock and reset. - * - * While this class isn't planned to be removed anytime soon (there are benefits to restricting - * IO), the clock and reset constructors will be phased out. Recommendation is to wrap the module - * in a withClock/withReset/withClockAndReset block, or directly hook up clock or reset IO pins. - */ -abstract class LegacyModule( - override_clock: Option[Clock]=None, override_reset: Option[Bool]=None) - (implicit moduleCompileOptions: CompileOptions) - extends ImplicitModule { - // _clock and _reset can be clock and reset in these 2ary constructors - // once chisel2 compatibility issues are resolved - def this(_clock: Clock)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), None)(moduleCompileOptions) - def this(_reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(None, Option(_reset))(moduleCompileOptions) - def this(_clock: Clock, _reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), Option(_reset))(moduleCompileOptions) - - // IO for this Module. At the Scala level (pre-FIRRTL transformations), - // connections in and out of a Module may only go through `io` elements. - def io: Record - - // Allow access to bindings from the compatibility package - protected def _ioPortBound() = _ports contains io - - protected override def nameIds(rootClass: Class[_]): HashMap[HasId, String] = { - val names = super.nameIds(rootClass) - - // Allow IO naming without reflection - names.put(io, "io") - names.put(clock, "clock") - names.put(reset, "reset") - - names - } - - private[core] override def generateComponent(): Component = { - _autoWrapPorts() // pre-IO(...) compatibility hack - - // Restrict IO to just io, clock, and reset - require(io != null, "Module must have io") - require(_ports contains io, "Module must have io wrapped in IO(...)") - require((_ports contains clock) && (_ports contains reset), "Internal error, module did not have clock or reset as IO") - require(_ports.size == 3, "Module must only have io, clock, and reset as IO") - - super.generateComponent() - } - - private[core] override def initializeInParent() { - // Don't generate source info referencing parents inside a module, since this interferes with - // module de-duplication in FIRRTL emission. - implicit val sourceInfo = UnlocatableSourceInfo - - pushCommand(DefInvalid(sourceInfo, io.ref)) - - override_clock match { - case Some(override_clock) => clock := override_clock - case _ => clock := Builder.forcedClock - } - - override_reset match { - case Some(override_reset) => reset := override_reset - case _ => reset := Builder.forcedReset - } - } -} - diff --git a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala new file mode 100644 index 00000000000..2813821ca5e --- /dev/null +++ b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala @@ -0,0 +1,174 @@ +// See LICENSE for license details. + +package chisel3.core + +import scala.collection.mutable.{ArrayBuffer, HashMap} +import scala.collection.JavaConversions._ +import scala.language.experimental.macros + +import chisel3.internal._ +import chisel3.internal.Builder._ +import chisel3.internal.firrtl._ +import chisel3.internal.firrtl.{Command => _, _} +import chisel3.internal.sourceinfo.UnlocatableSourceInfo + +/** Abstract base class for Modules that contain Chisel RTL. + */ +abstract class UserModule(implicit moduleCompileOptions: CompileOptions) + extends BaseModule { + // + // RTL construction internals + // + private val _commands = ArrayBuffer[Command]() + private[chisel3] def addCommand(c: Command) { + require(!_closed, "Can't write to module after module close") + _commands += c + } + protected def getCommands = { + require(_closed, "Can't get commands before module close") + _commands.toSeq + } + + // + // Other Internal Functions + // + // For debuggers/testers, TODO: refactor out into proper public API + private var _firrtlPorts: Option[Seq[firrtl.Port]] = None + lazy val getPorts = _firrtlPorts.get + + val compileOptions = moduleCompileOptions + + private[core] override def generateComponent(): Component = { + require(!_closed, "Can't generate module more than once") + _closed = true + + val names = nameIds(classOf[UserModule]) + + // Ports get first naming priority, since they are part of a Module's IO spec + for (port <- getModulePorts) { + require(names.contains(port), s"Unable to name port $port in $this") + port.setRef(ModuleIO(this, _namespace.name(names(port)))) + } + + // Then everything else gets named + for ((node, name) <- names) { + node.suggestName(name) + } + + // All suggestions are in, force names to every node. + for (id <- getIds) { + id match { + case id: BaseModule => id.forceName(default=id.desiredName, _namespace) + case id => id.forceName(default="_T", _namespace) + } + id._onModuleClose + } + + val firrtlPorts = for (port <- getModulePorts) yield { + // Port definitions need to know input or output at top-level. 'flipped' means Input. + val direction = if(Data.isFirrtlFlipped(port)) Direction.Input else Direction.Output + firrtl.Port(port, direction) + } + _firrtlPorts = Some(firrtlPorts) + + // Generate IO invalidation commands to initialize outputs as unused + val invalidateCommands = getModulePorts map {port => DefInvalid(UnlocatableSourceInfo, port.ref)} + + val component = DefModule(this, name, firrtlPorts, invalidateCommands ++ getCommands) + _component = Some(component) + component + } +} + +/** Abstract base class for Modules, which behave much like Verilog modules. + * These may contain both logic and state which are written in the Module + * body (constructor). + * + * @note Module instantiations must be wrapped in a Module() call. + */ +abstract class ImplicitModule(implicit moduleCompileOptions: CompileOptions) + extends UserModule { + // Implicit clock and reset pins + val clock = IO(Input(Clock())) + val reset = IO(Input(Bool())) + + // Setup ClockAndReset + Builder.currentClockAndReset = Some(ClockAndReset(clock, reset)) + + private[core] def initializeInParent() { + implicit val sourceInfo = UnlocatableSourceInfo + + for (port <- getModulePorts) { + pushCommand(DefInvalid(sourceInfo, port.ref)) + } + + clock := Builder.forcedClock + reset := Builder.forcedReset + } +} + +/** Legacy Module class that restricts IOs to just io, clock, and reset, and provides a constructor + * for threading through explicit clock and reset. + * + * While this class isn't planned to be removed anytime soon (there are benefits to restricting + * IO), the clock and reset constructors will be phased out. Recommendation is to wrap the module + * in a withClock/withReset/withClockAndReset block, or directly hook up clock or reset IO pins. + */ +abstract class LegacyModule( + override_clock: Option[Clock]=None, override_reset: Option[Bool]=None) + (implicit moduleCompileOptions: CompileOptions) + extends ImplicitModule { + // _clock and _reset can be clock and reset in these 2ary constructors + // once chisel2 compatibility issues are resolved + def this(_clock: Clock)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), None)(moduleCompileOptions) + def this(_reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(None, Option(_reset))(moduleCompileOptions) + def this(_clock: Clock, _reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), Option(_reset))(moduleCompileOptions) + + // IO for this Module. At the Scala level (pre-FIRRTL transformations), + // connections in and out of a Module may only go through `io` elements. + def io: Record + + // Allow access to bindings from the compatibility package + protected def _ioPortBound() = portsContains(io) + + protected override def nameIds(rootClass: Class[_]): HashMap[HasId, String] = { + val names = super.nameIds(rootClass) + + // Allow IO naming without reflection + names.put(io, "io") + names.put(clock, "clock") + names.put(reset, "reset") + + names + } + + private[core] override def generateComponent(): Component = { + _autoWrapPorts() // pre-IO(...) compatibility hack + + // Restrict IO to just io, clock, and reset + require(io != null, "Module must have io") + require(portsContains(io), "Module must have io wrapped in IO(...)") + require((portsContains(clock)) && (portsContains(reset)), "Internal error, module did not have clock or reset as IO") + require(portsSize == 3, "Module must only have io, clock, and reset as IO") + + super.generateComponent() + } + + private[core] override def initializeInParent() { + // Don't generate source info referencing parents inside a module, since this interferes with + // module de-duplication in FIRRTL emission. + implicit val sourceInfo = UnlocatableSourceInfo + + pushCommand(DefInvalid(sourceInfo, io.ref)) + + override_clock match { + case Some(override_clock) => clock := override_clock + case _ => clock := Builder.forcedClock + } + + override_reset match { + case Some(override_reset) => reset := override_reset + case _ => reset := Builder.forcedReset + } + } +} diff --git a/src/main/scala/chisel3/compatibility.scala b/src/main/scala/chisel3/compatibility.scala index 365c1ba5fe1..778d2c13bfb 100644 --- a/src/main/scala/chisel3/compatibility.scala +++ b/src/main/scala/chisel3/compatibility.scala @@ -154,14 +154,10 @@ package object Chisel { // scalastyle:ignore package.object.name import chisel3.core.Param abstract class BlackBox(params: Map[String, Param] = Map.empty[String, Param]) extends chisel3.core.BlackBox(params) { - var _ioDefined = false - override protected def IO[T<:Data](iodef: T): iodef.type = { - _ioDefined = true - super.IO(iodef) - } - + // This class auto-wraps the BlackBox with IO(...), allowing legacy code (where IO(...) wasn't + // required) to build. override def _autoWrapPorts() = { - if (!_ioDefined) { + if (!_ioPortBound()) { IO(io) } } @@ -177,6 +173,10 @@ package object Chisel { // scalastyle:ignore package.object.name override_clock: Option[Clock]=None, override_reset: Option[Bool]=None) (implicit moduleCompileOptions: CompileOptions) extends chisel3.core.LegacyModule(override_clock, override_reset) { + // This class auto-wraps the Module IO with IO(...), allowing legacy code (where IO(...) wasn't + // required) to build. + // Also provides the clock / reset constructors, which were used before withClock happened. + def this(_clock: Clock)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), None)(moduleCompileOptions) def this(_reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(None, Option(_reset))(moduleCompileOptions) def this(_clock: Clock, _reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), Option(_reset))(moduleCompileOptions) diff --git a/src/test/scala/chiselTests/BundleEltsSpec.scala b/src/test/scala/chiselTests/BundleEltsSpec.scala deleted file mode 100644 index e9f7829cc73..00000000000 --- a/src/test/scala/chiselTests/BundleEltsSpec.scala +++ /dev/null @@ -1,84 +0,0 @@ -// See LICENSE for license details. - -package chiselTests - -import chisel3._ -import chisel3.internal.firrtl.Width -import chisel3.testers.BasicTester - -import chisel3.experimental._ - -//trait BundleEltsSpecUtils { -// class BundleWidthModule extends Module { -// class BundleWidth[T <: Data](val genType: T, widthx: Width) extends Bundle { -// val gen = genType.chiselCloneType -// val in = Input(UInt(widthx)) -// val out = Output(UInt(widthx)) -// -// def getIntWidth = widthx -//// override def cloneType = new BundleWidth(genType, width).asInstanceOf[this.type] -// -// def GPF(rootClass: Class[_]) = { -// import reflect.runtime._,universe._ -// val im = currentMirror reflect this -// val vals = im.symbol.asClass.typeSignature.members filter (s => s.isTerm && s.asTerm.isAccessor) -// val methods = vals map (im reflectMethod _.asMethod) -// -// // Suggest names to nodes using runtime reflection -// def getValFields(c: Class[_]): Set[java.lang.reflect.Field] = { -// if (c == rootClass) Set() -// else getValFields(c.getSuperclass) ++ c.getDeclaredFields -// } -// val valNames = getValFields(this.getClass).map(_.getName) -// def isPublicVal(m: java.lang.reflect.Method) = -// m.getParameterTypes.isEmpty && valNames.contains(m.getName) && m.getDeclaringClass.isAssignableFrom(rootClass) -// getValFields(this.getClass).map(x => print(s"VN ${x.getName} => ${x.get(this)}\r\n")) -// this.getClass.getMethods.map(x => print(s"GM ${x.getName}\r\n")) -// this.getClass.getMethods.sortWith(_.getName < _.getName).filter(isPublicVal(_)) -// } -// } -// val io = IO(new BundleWidth(UInt(2.W), 2.W)) -//// val int = io.chiselCloneType -// io.out := io.in + 1.U -// -// import chisel3.core.DataMirror -// print(s"GenWidth ${DataMirror.widthOf(io)}\r\n") -// print(s"GPF ${io.GPF(classOf[Bundle])}\r\n") -// } -// -// @chiselName -// @dump -// class MySimpleModule extends Module { -// val io = IO(new Bundle { -// val in = Input(UInt(32.W)) -// val out = Output(UInt(32.W)) -// }) -// -// for (i <- 0 until 1) { -// val a = Wire(init = io.in) -// val b = Wire(init = a) -// io.out := b -// } -// } -// -// @chiselName -// @dump -// class MySimpleAFodule extends Module { -// val io = IO(new Bundle { -// val in = Input(UInt(32.W)) -// val out = Output(UInt(32.W)) -// }) -// -// Seq(io.in).foreach { in => -// val a = Wire(init = in) -// val b = Wire(init = a) -// io.out := b -// } -// } -//} -// -//class BundleEltsSpec extends ChiselFlatSpec with BundleEltsSpecUtils { -// "Bundles with the same fields but in different orders" should "bulk connect" in { -// elaborate { new BundleWidthModule } -// } -//} From 398e2525a1fa304911c057e63bb3ef07333ec67e Mon Sep 17 00:00:00 2001 From: ducky Date: Tue, 21 Mar 2017 14:55:43 -0700 Subject: [PATCH 3/6] RawModule tests, fixes to allow RawModule to instantiate --- .../main/scala/chisel3/core/UserModule.scala | 5 +- .../scala/chiselTests/RawModuleSpec.scala | 61 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 src/test/scala/chiselTests/RawModuleSpec.scala diff --git a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala index 2813821ca5e..916ab119c87 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala @@ -78,6 +78,9 @@ abstract class UserModule(implicit moduleCompileOptions: CompileOptions) _component = Some(component) component } + + // There is no initialization to be done by default. + private[core] def initializeInParent() {} } /** Abstract base class for Modules, which behave much like Verilog modules. @@ -95,7 +98,7 @@ abstract class ImplicitModule(implicit moduleCompileOptions: CompileOptions) // Setup ClockAndReset Builder.currentClockAndReset = Some(ClockAndReset(clock, reset)) - private[core] def initializeInParent() { + private[core] override def initializeInParent() { implicit val sourceInfo = UnlocatableSourceInfo for (port <- getModulePorts) { diff --git a/src/test/scala/chiselTests/RawModuleSpec.scala b/src/test/scala/chiselTests/RawModuleSpec.scala new file mode 100644 index 00000000000..66d9c89e5a0 --- /dev/null +++ b/src/test/scala/chiselTests/RawModuleSpec.scala @@ -0,0 +1,61 @@ +// See LICENSE for license details. + +package chiselTests + +import chisel3._ +import chisel3.experimental.{RawModule, withClockAndReset} +import chisel3.testers.BasicTester + +class UnclockedPlusOne extends RawModule { + val in = IO(Input(UInt(32.W))) + val out = IO(Output(UInt(32.W))) + + out := in + 1.asUInt +} + +class RawModuleTester extends BasicTester { + val plusModule = Module(new UnclockedPlusOne) + plusModule.in := 42.U + assert(plusModule.out === 43.U) + stop() +} + +class PlusOneModule extends Module { + val io = IO(new Bundle { + val in = Input(UInt(32.W)) + val out = Output(UInt(32.W)) + }) + io.out := io.in + 1.asUInt +} + +class RawModuleWithImpliitModule extends RawModule { + val in = IO(Input(UInt(32.W))) + val out = IO(Output(UInt(32.W))) + val clk = IO(Input(Clock())) + val rst = IO(Input(Bool())) + + withClockAndReset(clk, rst) { + val plusModule = Module(new PlusOneModule) + plusModule.io.in := in + out := plusModule.io.out + } +} + +class ImplicitModuleInRawModuleTester extends BasicTester { + val plusModule = Module(new RawModuleWithImpliitModule) + plusModule.clk := clock + plusModule.rst := reset + plusModule.in := 42.U + assert(plusModule.out === 43.U) + stop() +} + +class RawModuleSpec extends ChiselFlatSpec { + "RawModule" should "work" in { + assertTesterPasses({ new RawModuleTester }) + } + + "ImplicitModule in a withClock block in a RawModule" should "work" in { + assertTesterPasses({ new ImplicitModuleInRawModuleTester }) + } +} \ No newline at end of file From 3c3108855d0538246a44a5bf3938db3f8ddfc9c6 Mon Sep 17 00:00:00 2001 From: ducky Date: Wed, 22 Mar 2017 13:24:53 -0700 Subject: [PATCH 4/6] Add MultiIO inheritance-composition test / example --- .../scala/chiselTests/MultiIOModule.scala | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/test/scala/chiselTests/MultiIOModule.scala b/src/test/scala/chiselTests/MultiIOModule.scala index 6a6b4dd6534..07e09041e27 100644 --- a/src/test/scala/chiselTests/MultiIOModule.scala +++ b/src/test/scala/chiselTests/MultiIOModule.scala @@ -20,8 +20,39 @@ class MultiIOTester extends BasicTester { stop() } +// Demonstrate multiple IOs with inheritance where the IO is assigned to internally +trait LiteralOutputTrait extends MultiIOModule { + val myLiteralIO = IO(Output(UInt(32.W))) + myLiteralIO := 2.U +} + +// Demonstrate multiple IOs with inheritance where the IO is not assigned +// (and must be assigned by what extends this trait). +trait MultiIOTrait extends MultiIOModule { + val myTraitIO = IO(Output(UInt(32.W))) +} + +// Composition of the two above traits, example of IO composition directly using multiple top-level +// IOs rather than indirectly by constraining the type of the single .io field. +class ComposedMultiIOModule extends MultiIOModule + with LiteralOutputTrait with MultiIOTrait { + val topModuleIO = IO(Input(UInt(32.W))) + myTraitIO := topModuleIO +} + +class ComposedMultiIOTester extends BasicTester { + val composedModule = Module(new ComposedMultiIOModule) + composedModule.topModuleIO := 42.U + assert(composedModule.myTraitIO === 42.U) + assert(composedModule.myLiteralIO === 2.U) + stop() +} + class MultiIOSpec extends ChiselFlatSpec { "Multiple IOs in MultiIOModule" should "work" in { assertTesterPasses({ new MultiIOTester }) } + "Composed MultiIO Modules" should "work" in { + assertTesterPasses({ new ComposedMultiIOTester }) + } } From 51cdfbec21891830416dfab53a390368c607d420 Mon Sep 17 00:00:00 2001 From: ducky Date: Wed, 22 Mar 2017 13:26:37 -0700 Subject: [PATCH 5/6] Remove comment --- src/test/scala/chiselTests/ExtModule.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/scala/chiselTests/ExtModule.scala b/src/test/scala/chiselTests/ExtModule.scala index bcf09e602ff..f8927b9fc66 100644 --- a/src/test/scala/chiselTests/ExtModule.scala +++ b/src/test/scala/chiselTests/ExtModule.scala @@ -9,7 +9,6 @@ import chisel3._ import chisel3.experimental._ import chisel3.testers.BasicTester import chisel3.util._ -//import chisel3.core.ExplicitCompileOptions.Strict // Avoid collisions with regular BlackBox tests by putting ExtModule blackboxes // in their own scope. From e6f405e0bc847e1a35cc8b33e734cd596435b42e Mon Sep 17 00:00:00 2001 From: ducky Date: Thu, 30 Mar 2017 13:19:37 -0700 Subject: [PATCH 6/6] Allow top-level RawModule --- src/main/scala/chisel3/Driver.scala | 11 ++++++----- src/test/scala/chiselTests/ChiselSpec.scala | 7 ++++--- src/test/scala/chiselTests/RawModuleSpec.scala | 4 ++++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/main/scala/chisel3/Driver.scala b/src/main/scala/chisel3/Driver.scala index b2acc946509..8a2256dfb49 100644 --- a/src/main/scala/chisel3/Driver.scala +++ b/src/main/scala/chisel3/Driver.scala @@ -3,6 +3,7 @@ package chisel3 import chisel3.internal.firrtl.Emitter +import chisel3.experimental.RawModule import java.io._ import net.jcazevedo.moultingyaml._ @@ -88,11 +89,11 @@ object Driver extends BackendCompilationUtilities { * @param gen a function that creates a Module hierarchy * @return the resulting Chisel IR in the form of a Circuit (TODO: Should be FIRRTL IR) */ - def elaborate[T <: Module](gen: () => T): Circuit = internal.Builder.build(Module(gen())) + def elaborate[T <: RawModule](gen: () => T): Circuit = internal.Builder.build(Module(gen())) - def emit[T <: Module](gen: () => T): String = Emitter.emit(elaborate(gen)) + def emit[T <: RawModule](gen: () => T): String = Emitter.emit(elaborate(gen)) - def emit[T <: Module](ir: Circuit): String = Emitter.emit(ir) + def emit[T <: RawModule](ir: Circuit): String = Emitter.emit(ir) def dumpFirrtl(ir: Circuit, optName: Option[File]): File = { val f = optName.getOrElse(new File(ir.name + ".fir")) @@ -122,7 +123,7 @@ object Driver extends BackendCompilationUtilities { */ def execute( optionsManager: ExecutionOptionsManager with HasChiselExecutionOptions with HasFirrtlOptions, - dut: () => Module): ChiselExecutionResult = { + dut: () => RawModule): ChiselExecutionResult = { val circuit = elaborate(dut) // this little hack let's us set the topName with the circuit name if it has not been set from args @@ -173,7 +174,7 @@ object Driver extends BackendCompilationUtilities { * @param dut The device under test * @return An execution result with useful stuff, or failure with message */ - def execute(args: Array[String], dut: () => Module): ChiselExecutionResult = { + def execute(args: Array[String], dut: () => RawModule): ChiselExecutionResult = { val optionsManager = new ExecutionOptionsManager("chisel3") with HasChiselExecutionOptions with HasFirrtlOptions optionsManager.parse(args) match { diff --git a/src/test/scala/chiselTests/ChiselSpec.scala b/src/test/scala/chiselTests/ChiselSpec.scala index 584f134c37a..143a14950bf 100644 --- a/src/test/scala/chiselTests/ChiselSpec.scala +++ b/src/test/scala/chiselTests/ChiselSpec.scala @@ -7,6 +7,7 @@ import org.scalatest._ import org.scalatest.prop._ import org.scalacheck._ import chisel3._ +import chisel3.experimental.RawModule import chisel3.testers._ import firrtl.{ CommonOptions, @@ -27,21 +28,21 @@ trait ChiselRunners extends Assertions { def assertTesterFails(t: => BasicTester, additionalVResources: Seq[String] = Seq()): Unit = { assert(!runTester(t, additionalVResources)) } - def elaborate(t: => Module): Unit = Driver.elaborate(() => t) + def elaborate(t: => RawModule): Unit = Driver.elaborate(() => t) /** Given a generator, return the Firrtl that it generates. * * @param t Module generator * @return Firrtl representation as a String */ - def generateFirrtl(t: => Module): String = Driver.emit(() => t) + def generateFirrtl(t: => RawModule): String = Driver.emit(() => t) /** Compiles a Chisel Module to Verilog * NOTE: This uses the "test_run_dir" as the default directory for generated code. * @param t the generator for the module * @return the Verilog code as a string. */ - def compile(t: => Module): String = { + def compile(t: => RawModule): String = { val manager = new ExecutionOptionsManager("compile") with HasFirrtlOptions with HasChiselExecutionOptions { commonOptions = CommonOptions(targetDirName = "test_run_dir") diff --git a/src/test/scala/chiselTests/RawModuleSpec.scala b/src/test/scala/chiselTests/RawModuleSpec.scala index 66d9c89e5a0..180a1c0459c 100644 --- a/src/test/scala/chiselTests/RawModuleSpec.scala +++ b/src/test/scala/chiselTests/RawModuleSpec.scala @@ -51,6 +51,10 @@ class ImplicitModuleInRawModuleTester extends BasicTester { } class RawModuleSpec extends ChiselFlatSpec { + "RawModule" should "elaborate" in { + elaborate { new RawModuleWithImpliitModule } + } + "RawModule" should "work" in { assertTesterPasses({ new RawModuleTester }) }