Merge branch 'master' into 3.5-release

1 files changed, 207 insertions, 21 deletions

diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala
index d34211f1..3ae48821 100644
--- a/core/src/main/scala/chisel3/Module.scala
+++ b/core/src/main/scala/chisel3/Module.scala
@@ -4,9 +4,7 @@ package chisel3
 
 import scala.collection.immutable.ListMap
 import scala.collection.mutable.{ArrayBuffer, HashMap}
-import scala.collection.JavaConversions._
 import scala.language.experimental.macros
-import java.util.IdentityHashMap
 
 import chisel3.internal._
 import chisel3.internal.Builder._
@@ -14,6 +12,7 @@ import chisel3.internal.firrtl._
 import chisel3.internal.sourceinfo.{InstTransform, SourceInfo, UnlocatableSourceInfo}
 import chisel3.experimental.BaseModule
 import _root_.firrtl.annotations.{IsModule, ModuleName, ModuleTarget}
+import _root_.firrtl.AnnotationSeq
 
 object Module extends SourceInfoDoc {
   /** A wrapper method that all Module instantiations must be wrapped in
@@ -65,13 +64,18 @@ object Module extends SourceInfoDoc {
     Builder.currentClock = saveClock   // Back to clock and reset scope
     Builder.currentReset = saveReset
 
-    val component = module.generateComponent()
-    Builder.components += component
+    // Only add the component if the module generates one
+    val componentOpt = module.generateComponent()
+    for (component <- componentOpt) {
+      Builder.components += component
+    }
 
     Builder.setPrefix(savePrefix)
 
     // Handle connections at enclosing scope
-    if(!Builder.currentModule.isEmpty) {
+    // We use _component because Modules that don't generate them may still have one
+    if (Builder.currentModule.isDefined && module._component.isDefined) {
+      val component = module._component.get
       pushCommand(DefInstance(sourceInfo, module, component.ports))
       module.initializeInParent(compileOptions)
     }
@@ -117,7 +121,8 @@ abstract class Module(implicit moduleCompileOptions: CompileOptions) extends Raw
 
   private[chisel3] def mkReset: Reset = {
     // Top module and compatibility mode use Bool for reset
-    val inferReset = _parent.isDefined && moduleCompileOptions.inferModuleReset
+    // Note that a Definition elaboration will lack a parent, but still not be a Top module
+    val inferReset = (_parent.isDefined || Builder.inDefinition) && moduleCompileOptions.inferModuleReset
     if (inferReset) Reset() else Bool()
   }
 
@@ -138,6 +143,8 @@ abstract class Module(implicit moduleCompileOptions: CompileOptions) extends Raw
 
 package experimental {
 
+  import chisel3.internal.requireIsChiselType // Fix ambiguous import
+
   object IO {
     /** Constructs a port for the current Module
       *
@@ -175,8 +182,116 @@ package experimental {
 
 package internal {
   import chisel3.experimental.BaseModule
+  import chisel3.experimental.hierarchy.IsInstantiable
 
   object BaseModule {
+    /** Represents a clone of an underlying object. This is used to support CloneModuleAsRecord and Instance/Definition.
+      *
+      * @note We don't actually "clone" anything in the traditional sense but is a placeholder so we lazily clone internal state
+      */
+    private [chisel3] trait IsClone[+T] {
+      // Underlying object of which this is a clone of
+      val _proto: T
+      def getProto: T = _proto
+      def isACloneOf(a: Any): Boolean = this == a || _proto == a
+    }
+
+    // Private internal class to serve as a _parent for Data in cloned ports
+    private[chisel3] class ModuleClone[T <: BaseModule] (val _proto: T) extends PseudoModule with IsClone[T] {
+      override def toString = s"ModuleClone(${_proto})"
+      def getPorts = _portsRecord
+      // ClonePorts that hold the bound ports for this module
+      // Used for setting the refs of both this module and the Record
+      private[BaseModule] var _portsRecord: Record = _
+      // This is necessary for correctly supporting .toTarget on a Module Clone. If it is made from the
+      // Instance/Definition API, it should return an instanceTarget. If made from CMAR, it should return a
+      // ModuleTarget.
+      private[chisel3]    var _madeFromDefinition: Boolean = false
+      // Don't generate a component, but point to the one for the cloned Module
+      private[chisel3] def generateComponent(): Option[Component] = {
+        require(!_closed, "Can't generate module more than once")
+        _closed = true
+        _component = _proto._component
+        None
+      }
+      // Maps proto ports to module clone's ports
+      private[chisel3] lazy val ioMap: Map[Data, Data] = {
+        val name2Port = getPorts.elements
+        _proto.getChiselPorts.map { case (name, data) => data -> name2Port(name) }.toMap
+      }
+      // This module doesn't actually exist in the FIRRTL so no initialization to do
+      private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = ()
+
+      // Name of this instance's module is the same as the proto's name
+      override def desiredName: String = _proto.name
+
+      private[chisel3] def setRefAndPortsRef(namespace: Namespace): Unit = {
+        val record = _portsRecord
+        // Use .forceName to re-use default name resolving behavior
+        record.forceName(None, default=this.desiredName, namespace)
+        // Now take the Ref that forceName set and convert it to the correct Arg
+        val instName = record.getRef match {
+          case Ref(name) => name
+          case bad => throwException(s"Internal Error! Cloned-module Record $record has unexpected ref $bad")
+        }
+        // Set both the record and the module to have the same instance name
+        record.setRef(ModuleCloneIO(_proto, instName), force=true) // force because we did .forceName first
+        this.setRef(Ref(instName))
+      }
+    }
+
+    /** Represents a module viewed from a different instance context.
+      *
+      * @note Why do we need both ModuleClone and InstanceClone? If we are annotating a reference in a module-clone,
+      * all submodules must be also be 'cloned' so the toTarget can be computed properly. However, we don't need separate
+      * connectable ports for this instance; all that's different from the proto is the parent.
+      *
+      * @note In addition, the instance name of an InstanceClone is going to be the SAME as the proto, but this is not true
+      * for ModuleClone.
+      */
+    private[chisel3] final class InstanceClone[T <: BaseModule] (val _proto: T, val instName: () => String) extends PseudoModule with IsClone[T] {
+      override def toString = s"InstanceClone(${_proto})"
+      // No addition components are generated
+      private[chisel3] def generateComponent(): Option[Component] = None
+      // Necessary for toTarget to work
+      private[chisel3] def setAsInstanceRef(): Unit = { this.setRef(Ref(instName())) }
+      // This module doesn't acutally exist in the FIRRTL so no initialization to do
+      private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = ()
+      // Instance name is the same as proto's instance name
+      override def instanceName = instName()
+      // Module name is the same as proto's module name
+      override def desiredName: String = _proto.name
+    }
+
+    /** Represents a Definition root module, when accessing something from a definition
+      *
+      * @note This is necessary to distinguish between the toTarget behavior for a Module returned from a Definition,
+      * versus a normal Module. A normal Module.toTarget will always return a local target. If calling toTarget
+      * on a Module returned from a Definition (and thus wrapped in an Instance), we need to return the non-local
+      * target whose root is the Definition. This DefinitionClone is used to represent the root parent of the
+      * InstanceClone (which represents the returned module).
+      */
+    private[chisel3] class DefinitionClone[T <: BaseModule] (val _proto: T) extends PseudoModule with IsClone[T] {
+      override def toString = s"DefinitionClone(${_proto})"
+      // No addition components are generated
+      private[chisel3] def generateComponent(): Option[Component] = None
+      // Necessary for toTarget to work
+      private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = ()
+      // Module name is the same as proto's module name
+      override def desiredName: String = _proto.name
+    }
+
+    /** @note If we are cloning a non-module, we need another object which has the proper _parent set!
+      */
+    private[chisel3] final class InstantiableClone[T <: IsInstantiable] (val _proto: T) extends IsClone[T] {
+      private[chisel3] var _parent: Option[BaseModule] = internal.Builder.currentModule
+    }
+
+    /** Record type returned by CloneModuleAsRecord
+      *
+      * @note These are not true Data (the Record doesn't correspond to anything in the emitted
+      * FIRRTL yet its elements *do*) so have some very specialized behavior.
+      */
     private[chisel3] class ClonePorts (elts: Data*)(implicit compileOptions: CompileOptions) extends Record {
       val elements = ListMap(elts.map(d => d.instanceName -> d.cloneTypeFull): _*)
       def apply(field: String) = elements(field)
@@ -185,12 +300,20 @@ package internal {
 
     private[chisel3] def cloneIORecord(proto: BaseModule)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): ClonePorts = {
       require(proto.isClosed, "Can't clone a module before module close")
+      // Fake Module to serve as the _parent of the cloned ports
+      // We make this before clonePorts because we want it to come up first in naming in
+      // currentModule
+      val cloneParent = Module(new ModuleClone(proto))
+      require(proto.isClosed, "Can't clone a module before module close")
+      require(cloneParent.getOptionRef.isEmpty, "Can't have ref set already!")
+      // Fake Module to serve as the _parent of the cloned ports
+      // We don't create this inside the ModuleClone because we need the ref to be set by the
+      // currentModule (and not clonePorts)
       val clonePorts = new ClonePorts(proto.getModulePorts: _*)
-      clonePorts.bind(WireBinding(Builder.forcedUserModule, Builder.currentWhen()))
-      val cloneInstance = new DefInstance(sourceInfo, proto, proto._component.get.ports) {
-        override def name = clonePorts.getRef.name
-      }
-      pushCommand(cloneInstance)
+      clonePorts.bind(PortBinding(cloneParent))
+      clonePorts.setAllParents(Some(cloneParent))
+      cloneParent._portsRecord = clonePorts
+      // Normally handled during Module construction but ClonePorts really lives in its parent's parent
       if (!compileOptions.explicitInvalidate) {
         pushCommand(DefInvalid(sourceInfo, clonePorts.ref))
       }
@@ -205,10 +328,21 @@ package internal {
 
 package experimental {
 
+  import chisel3.experimental.hierarchy.IsInstantiable
+
+  object BaseModule {
+    implicit class BaseModuleExtensions[T <: BaseModule](b: T) {
+      import chisel3.experimental.hierarchy.{Instance, Definition}
+      def toInstance: Instance[T] = new Instance(Left(b))
+      def toDefinition: Definition[T] = new Definition(Left(b))
+    }
+  }
   /** Abstract base class for Modules, an instantiable organizational unit for RTL.
     */
   // TODO: seal this?
-  abstract class BaseModule extends HasId {
+  abstract class BaseModule extends HasId with IsInstantiable {
+    _parent.foreach(_.addId(this))
+
     //
     // Builder Internals - this tracks which Module RTL construction belongs to.
     //
@@ -240,7 +374,15 @@ package experimental {
       }
     }
 
-    protected def getIds = {
+    // Returns the last id contained within a Module
+    private[chisel3] def _lastId: Long = _ids.last match {
+      case mod: BaseModule => mod._lastId
+      case _ =>
+        // Ideally we could just take last._id, but Records store and thus bind their Data in reverse order
+        _ids.maxBy(_._id)._id
+    }
+
+    private[chisel3] def getIds = {
       require(_closed, "Can't get ids before module close")
       _ids.toSeq
     }
@@ -267,7 +409,7 @@ package experimental {
     /** Generates the FIRRTL Component (Module or Blackbox) of this Module.
       * Also closes the module so no more construction can happen inside.
       */
-    private[chisel3] def generateComponent(): Component
+    private[chisel3] def generateComponent(): Option[Component]
 
     /** Sets up this module in the parent context
       */
@@ -305,9 +447,12 @@ package experimental {
 
     /** Legalized name of this module. */
     final lazy val name = try {
-      // If this is a module aspect, it should share the same name as the original module
-      // Thus, the desired name should be returned without uniquification
-      if(this.isInstanceOf[ModuleAspect]) desiredName else Builder.globalNamespace.name(desiredName)
+      // PseudoModules are not "true modules" and thus should share
+      // their original modules names without uniquification
+      this match {
+        case _: PseudoModule => desiredName
+        case _ => Builder.globalNamespace.name(desiredName)
+      }
     } catch {
       case e: NullPointerException => throwException(
         s"Error: desiredName of ${this.getClass.getName} is null. Did you evaluate 'name' before all values needed by desiredName were available?", e)
@@ -318,13 +463,36 @@ package experimental {
       *
       * @note Should not be called until circuit elaboration is complete
       */
-    final def toNamed: ModuleName = toTarget.toNamed
+    final def toNamed: ModuleName = ModuleTarget(this.circuitName, this.name).toNamed
 
     /** Returns a FIRRTL ModuleTarget that references this object
       *
       * @note Should not be called until circuit elaboration is complete
       */
-    final def toTarget: ModuleTarget = ModuleTarget(this.circuitName, this.name)
+    final def toTarget: ModuleTarget = this match {
+      case m: internal.BaseModule.InstanceClone[_] => throwException(s"Internal Error! It's not legal to call .toTarget on an InstanceClone. $m")
+      case m: internal.BaseModule.DefinitionClone[_] => throwException(s"Internal Error! It's not legal to call .toTarget on an DefinitionClone. $m")
+      case _ => ModuleTarget(this.circuitName, this.name)
+    }
+
+    /** Returns the real target of a Module which may be an [[InstanceTarget]]
+      *
+      * BaseModule.toTarget returns a ModuleTarget because the classic Module(new MyModule) API elaborates
+      * Modules in a way that there is a 1:1 relationship between instances and elaborated definitions
+      *
+      * Instance/Definition introduced special internal modules [[InstanceClone]] and [[ModuleClone]] that
+      * do not have this 1:1 relationship so need the ability to return [[InstanceTarget]]s.
+      * Because users can never actually get references to these underlying objects, we can maintain
+      * BaseModule.toTarget's API returning [[ModuleTarget]] while providing an internal API for getting
+      * the correct [[InstanceTarget]]s whenever using the Definition/Instance API.
+      */
+    private[chisel3] def getTarget: IsModule = this match {
+      case m: internal.BaseModule.InstanceClone[_] if m._parent.nonEmpty => m._parent.get.getTarget.instOf(instanceName, name)
+      case m: internal.BaseModule.ModuleClone[_] if m._madeFromDefinition => m._parent.get.getTarget.instOf(instanceName, name)
+      // Without this, we get the wrong CircuitName for the Definition
+      case m: internal.BaseModule.DefinitionClone[_] if m._circuit.nonEmpty => ModuleTarget(this._circuit.get.circuitName, this.name)
+      case _ => this.toTarget
+    }
 
     /** Returns a FIRRTL ModuleTarget that references this object
       *
@@ -332,8 +500,15 @@ package experimental {
       */
     final def toAbsoluteTarget: IsModule = {
       _parent match {
-        case Some(parent) => parent.toAbsoluteTarget.instOf(this.instanceName, toTarget.module)
-        case None => toTarget
+        case Some(parent) => parent.toAbsoluteTarget.instOf(this.instanceName, name)
+        case None =>
+          // FIXME Special handling for Views - evidence of "weirdness" of .toAbsoluteTarget
+          // In theory, .toAbsoluteTarget should not be necessary, .toTarget combined with the
+          // target disambiguation in FIRRTL's deduplication transform should ensure that .toTarget
+          // is always unambigous. However, legacy workarounds for Chisel's lack of an instance API
+          // have lead some to use .toAbsoluteTarget as a workaround. A proper instance API will make
+          // it possible to deprecate and remove .toAbsoluteTarget
+          if (this == ViewParent) ViewParent.absoluteTarget else getTarget
       }
     }
 
@@ -382,6 +557,17 @@ package experimental {
       names
     }
 
+    /** Invokes _onModuleClose on HasIds found via reflection but not bound to hardware
+      * (thus not part of _ids)
+      * This maintains old naming behavior for non-hardware Data
+      */
+    private[chisel3] def closeUnboundIds(names: HashMap[HasId, String]): Unit = {
+      val idLookup = _ids.toSet
+      for ((id, _) <- names if !idLookup(id)) {
+        id._onModuleClose
+      }
+    }
+
     /** Compatibility function. Allows Chisel2 code which had ports without the IO wrapper to
       * compile under Bindings checks. Does nothing in non-compatibility mode.
       *