Fix CloneModuleAsRecord support for .toTarget

6 files changed, 110 insertions, 29 deletions

diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala
index 8ba4b612..0c42600f 100644
--- a/core/src/main/scala/chisel3/BlackBox.scala
+++ b/core/src/main/scala/chisel3/BlackBox.scala
@@ -62,7 +62,7 @@ package experimental {
     * @note The parameters API is experimental and may change
     */
   abstract class ExtModule(val params: Map[String, Param] = Map.empty[String, Param]) extends BaseBlackBox {
-    private[chisel3] override def generateComponent(): Component = {
+    private[chisel3] override def generateComponent(): Option[Component] = {
       require(!_closed, "Can't generate module more than once")
       _closed = true
 
@@ -86,7 +86,7 @@ package experimental {
       val firrtlPorts = getModulePorts map {port => Port(port, port.specifiedDirection)}
       val component = DefBlackBox(this, name, firrtlPorts, SpecifiedDirection.Unspecified, params)
       _component = Some(component)
-      component
+      _component
     }
 
     private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = {
@@ -145,7 +145,7 @@ abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param
   // Allow access to bindings from the compatibility package
   protected def _compatIoPortBound() = portsContains(_io)
 
-  private[chisel3] override def generateComponent(): Component = {
+  private[chisel3] override def generateComponent(): Option[Component] = {
     _compatAutoWrapPorts()  // pre-IO(...) compatibility hack
 
     // Restrict IO to just io, clock, and reset
@@ -178,7 +178,7 @@ abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param
     val firrtlPorts = namedPorts map {namedPort => Port(namedPort._2, namedPort._2.specifiedDirection)}
     val component = DefBlackBox(this, name, firrtlPorts, _io.specifiedDirection, params)
     _component = Some(component)
-    component
+    _component
   }
 
   private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = {
diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala
index b204be8d..77735583 100644
--- a/core/src/main/scala/chisel3/Module.scala
+++ b/core/src/main/scala/chisel3/Module.scala
@@ -64,13 +64,18 @@ object Module extends SourceInfoDoc {
     Builder.currentClock = saveClock   // Back to clock and reset scope
     Builder.currentReset = saveReset
 
-    val component = module.generateComponent()
-    Builder.components += component
+    // Only add the component if the module generates one
+    val componentOpt = module.generateComponent()
+    for (component <- componentOpt) {
+      Builder.components += component
+    }
 
     Builder.setPrefix(savePrefix)
 
     // Handle connections at enclosing scope
-    if(!Builder.currentModule.isEmpty) {
+    // We use _component because Modules that don't generate them may still have one
+    if (Builder.currentModule.isDefined && module._component.isDefined) {
+      val component = module._component.get
       pushCommand(DefInstance(sourceInfo, module, component.ports))
       module.initializeInParent(compileOptions)
     }
@@ -178,20 +183,73 @@ package internal {
   import chisel3.experimental.BaseModule
 
   object BaseModule {
-    private[chisel3] class ClonePorts (elts: Data*)(implicit compileOptions: CompileOptions) extends Record {
+    // Private internal class to serve as a _parent for Data in cloned ports
+    private[chisel3] class ModuleClone(proto: BaseModule) extends BaseModule {
+      // Don't generate a component, but point to the one for the cloned Module
+      private[chisel3] def generateComponent(): Option[Component] = {
+        _component = proto._component
+        None
+      }
+      // This module doesn't acutally exist in the FIRRTL so no initialization to do
+      private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = ()
+
+      override def desiredName: String = proto.name
+    }
+
+    /** Record type returned by CloneModuleAsRecord
+      *
+      * @note These are not true Data (the Record doesn't correspond to anything in the emitted
+      * FIRRTL yet its elements *do*) so have some very specialized behavior.
+      * @param proto Optional pointer to the Module we are a clone of. Set for first instance, unset
+      *              for clones
+      */
+    private[chisel3] class ClonePorts (proto: Option[BaseModule], elts: Data*)(implicit compileOptions: CompileOptions) extends Record {
       val elements = ListMap(elts.map(d => d.instanceName -> d.cloneTypeFull): _*)
       def apply(field: String) = elements(field)
-      override def cloneType = (new ClonePorts(elts: _*)).asInstanceOf[this.type]
+      override def cloneType = (new ClonePorts(None, elts: _*)).asInstanceOf[this.type]
+
+      // Because ClonePorts instances are *not* created inside of their parent module, but rather,
+      // their parent's parent, we have to intercept the standard setRef and replace it with our own
+      // special Ref type.
+      // This only applies to ClonePorts created in cloneIORecord, any clones of these Records have
+      // normal behavior.
+      // Also, the name of ClonePorts Records needs to be propagated to their parent ModuleClone
+      // since we have no other way of setting the instance name for those.
+      private[chisel3] override def setRef(imm: Arg, force: Boolean): Unit = {
+        val immx = (proto, imm) match {
+          case (Some(mod), Ref(name)) =>
+            // Our _parent is a ModuleClone that needs its ref to match ours for .toAbsoluteTarget
+            _parent.foreach(_.setRef(Ref(name), force=true))
+            // Return a specialize-ref that will do the right thing
+            ModuleCloneIO(mod, name)
+          case _ => imm
+        }
+        super.setRef(immx, force)
+      }
+    }
+
+    // Recursively set the parent of the start Data and any children (eg. in an Aggregate)
+    private def setAllParents(start: Data, parent: Option[BaseModule]): Unit = {
+      def rec(data: Data): Unit = {
+        data._parent = parent
+        data match {
+        case _: Element =>
+        case agg: Aggregate =>
+          agg.getElements.foreach(rec)
+        }
+      }
+      rec(start)
     }
 
     private[chisel3] def cloneIORecord(proto: BaseModule)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): ClonePorts = {
       require(proto.isClosed, "Can't clone a module before module close")
-      val clonePorts = new ClonePorts(proto.getModulePorts: _*)
-      clonePorts.bind(WireBinding(Builder.forcedUserModule, Builder.currentWhen()))
-      val cloneInstance = new DefInstance(sourceInfo, proto, proto._component.get.ports) {
-        override def name = clonePorts.getRef.name
-      }
-      pushCommand(cloneInstance)
+      // We don't create this inside the ModuleClone because we need the ref to be set by the
+      // currentModule (and not clonePorts)
+      val clonePorts = new ClonePorts(Some(proto), proto.getModulePorts: _*)
+      val cloneParent = Module(new ModuleClone(proto))
+      clonePorts.bind(PortBinding(cloneParent))
+      setAllParents(clonePorts, Some(cloneParent))
+      // Normally handled during Module construction but ClonePorts really lives in its parent's parent
       if (!compileOptions.explicitInvalidate) {
         pushCommand(DefInvalid(sourceInfo, clonePorts.ref))
       }
@@ -270,7 +328,7 @@ package experimental {
     /** Generates the FIRRTL Component (Module or Blackbox) of this Module.
       * Also closes the module so no more construction can happen inside.
       */
-    private[chisel3] def generateComponent(): Component
+    private[chisel3] def generateComponent(): Option[Component]
 
     /** Sets up this module in the parent context
       */
@@ -308,9 +366,12 @@ package experimental {
 
     /** Legalized name of this module. */
     final lazy val name = try {
-      // If this is a module aspect, it should share the same name as the original module
-      // Thus, the desired name should be returned without uniquification
-      if(this.isInstanceOf[ModuleAspect]) desiredName else Builder.globalNamespace.name(desiredName)
+      // ModuleAspects and ModuleClones are not "true modules" and thus should share
+      // their original modules names without uniquification
+      this match {
+        case (_: ModuleAspect | _: internal.BaseModule.ModuleClone) => desiredName
+        case _ => Builder.globalNamespace.name(desiredName)
+      }
     } catch {
       case e: NullPointerException => throwException(
         s"Error: desiredName of ${this.getClass.getName} is null. Did you evaluate 'name' before all values needed by desiredName were available?", e)
diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala
index de93e781..5bcd4dbd 100644
--- a/core/src/main/scala/chisel3/RawModule.scala
+++ b/core/src/main/scala/chisel3/RawModule.scala
@@ -57,7 +57,7 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions)
   }
 
 
-  private[chisel3] override def generateComponent(): Component = {
+  private[chisel3] override def generateComponent(): Option[Component] = {
     require(!_closed, "Can't generate module more than once")
     _closed = true
 
@@ -130,7 +130,7 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions)
     }
     val component = DefModule(this, name, firrtlPorts, invalidateCommands ++ getCommands)
     _component = Some(component)
-    component
+    _component
   }
 
   private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = {
@@ -221,7 +221,7 @@ package object internal {
     // Allow access to bindings from the compatibility package
     protected def _compatIoPortBound() = portsContains(_io)
 
-    private[chisel3] override def generateComponent(): Component = {
+    private[chisel3] override def generateComponent(): Option[Component] = {
       _compatAutoWrapPorts()  // pre-IO(...) compatibility hack
 
       // Restrict IO to just io, clock, and reset
diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index e1e4d460..35c4bdf9 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -84,7 +84,7 @@ trait InstanceId {
 
 private[chisel3] trait HasId extends InstanceId {
   private[chisel3] def _onModuleClose: Unit = {}
-  private[chisel3] val _parent: Option[BaseModule] = Builder.currentModule
+  private[chisel3] var _parent: Option[BaseModule] = Builder.currentModule
 
   private[chisel3] val _id: Long = Builder.idGen.next
 
diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala
index 40d3691c..093d4848 100644
--- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala
+++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala
@@ -24,16 +24,24 @@ private[chisel3] object Converter {
     case Percent => ("%%", List.empty)
   }
 
+  private def reportInternalError(msg: String): Nothing = {
+    val link = "https://github.com/chipsalliance/chisel3/issues/new"
+    val fullMsg = s"Internal Error! $msg This is a bug in Chisel, please file an issue at '$link'"
+    throwException(fullMsg)
+  }
+
   def getRef(id: HasId, sourceInfo: SourceInfo): Arg =
     id.getOptionRef.getOrElse {
       val module = id._parent.map(m => s" '$id' was defined in module '$m'.").getOrElse("")
       val loc = sourceInfo.makeMessage(" " + _)
-      val link = "https://github.com/chipsalliance/chisel3/issues/new"
-      val msg = s"Internal error! Could not get ref for '$id'$loc!$module " +
-                s"This is a bug in Chisel, please file an issue at '$link'."
-      throwException(msg)
+      reportInternalError(s"Could not get ref for '$id'$loc!$module")
     }
 
+  private def clonedModuleIOError(mod: BaseModule, name: String, sourceInfo: SourceInfo): Nothing = {
+    val loc = sourceInfo.makeMessage(" " + _)
+    reportInternalError(s"Trying to convert a cloned IO of $mod inside of $mod itself$loc!")
+  }
+
   def convert(info: SourceInfo): fir.Info = info match {
     case _: NoSourceInfo => fir.NoInfo
     case SourceLine(fn, line, col) => fir.FileInfo(fir.StringLit(s"$fn $line:$col"))
@@ -65,6 +73,9 @@ private[chisel3] object Converter {
     case ModuleIO(mod, name) =>
       if (mod eq ctx.id) fir.Reference(name, fir.UnknownType)
       else fir.SubField(fir.Reference(getRef(mod, info).name, fir.UnknownType), name, fir.UnknownType)
+    case ModuleCloneIO(mod, name) =>
+      if (mod eq ctx.id) clonedModuleIOError(mod, name, info)
+      else fir.Reference(name)
     case u @ ULit(n, UnknownWidth()) =>
       fir.UIntLiteral(n, fir.IntWidth(u.minWidth))
     case ULit(n, w) =>
diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala
index 5dc72a43..1d77802b 100644
--- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala
+++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala
@@ -165,9 +165,18 @@ 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"
 }
-case class Slot(imm: Node, name: String) extends Arg {
+// For use with CloneModuleAsRecord
+// Note that `name` is the name of the module instance whereas in ModuleIO it's the name of the port
+// The names of ports inside of a ModuleCloneIO are the names of the Slots
+case class ModuleCloneIO(mod: BaseModule, name: String) extends Arg {
   override def fullName(ctx: Component): String =
-    if (imm.fullName(ctx).isEmpty) name else s"${imm.fullName(ctx)}.${name}"
+    if (mod eq ctx.id) "" else name
+}
+case class Slot(imm: Node, name: String) extends Arg {
+  override def fullName(ctx: Component): String = {
+    val immName = imm.fullName(ctx)
+    if (immName.isEmpty) name else s"$immName.$name"
+  }
 }
 case class Index(imm: Arg, value: Arg) extends Arg {
   def name: String = s"[$value]"