Programmatic Port Creation (#833)

Add chisel3.experimental.IO for programmatic port creation in Raw and
MultiIOModules. suggestName is required to name ports that cannot be
named by reflection. Two ports cannot be given the same name.

6 files changed, 146 insertions, 24 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Module.scala b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
index 2f365dd7..c4a48fb4 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Module.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
@@ -79,6 +79,40 @@ object Module {
   def currentModule: Option[BaseModule] = Builder.currentModule
 }
 
+object IO {
+  /** Constructs a port for the current Module
+    *
+    * This must wrap the datatype used to set the io field of any Module.
+    * i.e. All concrete modules must have defined io in this form:
+    * [lazy] val io[: io type] = IO(...[: io type])
+    *
+    * Items in [] are optional.
+    *
+    * The granted iodef must be a chisel type and not be bound to hardware.
+    *
+    * 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.
+    */
+  def apply[T<:Data](iodef: T): T = {
+    val module = Module.currentModule.get // Impossible to fail
+    require(!module.isClosed, "Can't add more ports after module close")
+    requireIsChiselType(iodef, "io type")
+
+    // Clone the IO so we preserve immutability of data types
+    val iodefClone = try {
+      iodef.cloneTypeFull
+    } catch {
+      // For now this is going to be just a deprecation so we don't suddenly break everyone's code
+      case e: AutoClonetypeException =>
+        Builder.deprecated(e.getMessage, Some(s"${iodef.getClass}"))
+        iodef
+    }
+    module.bindIoInPlace(iodefClone)
+    iodefClone
+  }
+}
+
 /** Abstract base class for Modules, an instantiable organizational unit for RTL.
   */
 // TODO: seal this?
@@ -99,6 +133,9 @@ abstract class BaseModule extends HasId {
   //
   protected var _closed = false
 
+  /** Internal check if a Module is closed */
+  private[core] def isClosed = _closed
+
   // Fresh Namespace because in Firrtl, Modules namespaces are disjoint with the global namespace
   private[core] val _namespace = Namespace.empty
   private val _ids = ArrayBuffer[HasId]()
@@ -243,6 +280,8 @@ abstract class BaseModule extends HasId {
     iodef.bind(PortBinding(this))
     _ports += iodef
   }
+  /** Private accessor for _bindIoInPlace */
+  private[core] def bindIoInPlace(iodef: Data): Unit = _bindIoInPlace(iodef)
 
   /**
    * This must wrap the datatype used to set the io field of any Module.
@@ -260,22 +299,7 @@ abstract class BaseModule extends HasId {
    * TODO(twigg): Specifically walk the Data definition to call out which nodes
    * are problematic.
    */
-  protected def IO[T<:Data](iodef: T): T = {
-    require(!_closed, "Can't add more ports after module close")
-    requireIsChiselType(iodef, "io type")
-
-    // Clone the IO so we preserve immutability of data types
-    val iodefClone = try {
-      iodef.cloneTypeFull
-    } catch {
-      // For now this is going to be just a deprecation so we don't suddenly break everyone's code
-      case e: AutoClonetypeException =>
-        Builder.deprecated(e.getMessage, Some(s"${iodef.getClass}"))
-        iodef
-    }
-    _bindIoInPlace(iodefClone)
-    iodefClone
-  }
+  protected def IO[T<:Data](iodef: T): T = chisel3.core.IO.apply(iodef)
 
   //
   // Internal Functions
diff --git a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala
index 17b8a09e..831b3707 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala
@@ -38,6 +38,22 @@ abstract class UserModule(implicit moduleCompileOptions: CompileOptions)
 
   val compileOptions = moduleCompileOptions
 
+  private[chisel3] def namePorts(names: HashMap[HasId, String]): Unit = {
+    for (port <- getModulePorts) {
+      port.suggestedName.orElse(names.get(port)) match {
+        case Some(name) =>
+          if (_namespace.contains(name)) {
+            Builder.error(s"""Unable to name port $port to "$name" in $this,""" +
+              " name is already taken by another port!")
+          }
+          port.setRef(ModuleIO(this, _namespace.name(name)))
+        case None => Builder.error(s"Unable to name port $port in $this, " +
+          "try making it a public field of the Module")
+      }
+    }
+  }
+
+
   private[core] override def generateComponent(): Component = {
     require(!_closed, "Can't generate module more than once")
     _closed = true
@@ -45,10 +61,7 @@ abstract class UserModule(implicit moduleCompileOptions: CompileOptions)
     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))))
-    }
+    namePorts(names)
 
     // Then everything else gets named
     for ((node, name) <- names) {
@@ -170,6 +183,15 @@ abstract class LegacyModule(implicit moduleCompileOptions: CompileOptions)
     names
   }
 
+  private[chisel3] override def namePorts(names: HashMap[HasId, String]): Unit = {
+    for (port <- getModulePorts) {
+      // This should already have been caught
+      if (!names.contains(port)) throwException(s"Unable to name port $port in $this")
+      val name = names(port)
+      port.setRef(ModuleIO(this, _namespace.name(name)))
+    }
+  }
+
   private[core] override def generateComponent(): Component = {
     _compatAutoWrapPorts()  // pre-IO(...) compatibility hack
 
diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
index 5e456030..62d8f9b5 100644
--- a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
+++ b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
@@ -94,6 +94,7 @@ private[chisel3] trait HasId extends InstanceId {
     for(hook <- postname_hooks) { hook(name) }
     this
   }
+  private[chisel3] def suggestedName: Option[String] = suggested_name
   private[chisel3] def addPostnameHook(hook: String=>Unit): Unit = postname_hooks += hook
 
   // Uses a namespace to convert suggestion into a true name
diff --git a/src/main/scala/chisel3/package.scala b/src/main/scala/chisel3/package.scala
index b3a9f54b..5f0e31de 100644
--- a/src/main/scala/chisel3/package.scala
+++ b/src/main/scala/chisel3/package.scala
@@ -426,6 +426,8 @@ package object chisel3 {    // scalastyle:ignore package.object.name
     type RawModule = chisel3.core.UserModule
     type ExtModule = chisel3.core.ExtModule
 
+    val IO = chisel3.core.IO
+
     // 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/test/scala/chiselTests/NamingAnnotationTest.scala b/src/test/scala/chiselTests/NamingAnnotationTest.scala
index a7b9b75c..07962aaf 100644
--- a/src/test/scala/chiselTests/NamingAnnotationTest.scala
+++ b/src/test/scala/chiselTests/NamingAnnotationTest.scala
@@ -4,16 +4,14 @@ package chiselTests
 
 import chisel3._
 import chisel3.internal.InstanceId
-import chisel3.experimental.{chiselName, dump}
+import chisel3.experimental.{chiselName, dump, MultiIOModule}
 import org.scalatest._
 import org.scalatest.prop._
 import chisel3.testers.BasicTester
 
 import scala.collection.mutable.ListBuffer
 
-trait NamedModuleTester extends Module {
-  val io = IO(new Bundle() {})  // Named module testers don't need IO
-
+trait NamedModuleTester extends MultiIOModule {
   val expectedNameMap = ListBuffer[(InstanceId, String)]()
   val expectedModuleNameMap = ListBuffer[(Module, String)]()
 
diff --git a/src/test/scala/chiselTests/experimental/ProgrammaticPortsSpec.scala b/src/test/scala/chiselTests/experimental/ProgrammaticPortsSpec.scala
new file mode 100644
index 00000000..d17bfd32
--- /dev/null
+++ b/src/test/scala/chiselTests/experimental/ProgrammaticPortsSpec.scala
@@ -0,0 +1,75 @@
+// See LICENSE for license details.
+
+package chiselTests
+package experimental
+
+import chisel3._
+import chisel3.experimental.MultiIOModule
+
+// NOTE This is currently an experimental API and subject to change
+// Example using a private port
+class PrivatePort extends NamedModuleTester {
+  private val port = expectName(IO(Input(UInt(8.W))), "foo")
+  port.suggestName("foo")
+}
+
+// Example of using composition to add ports to a Module
+class CompositionalPort(module: NamedModuleTester, name: String) {
+  import chisel3.experimental.IO
+  val foo = module.expectName(IO(Output(Bool())), name)
+  foo.suggestName(name)
+  foo := true.B
+}
+
+class CompositionalPortTester extends NamedModuleTester {
+  val a = new CompositionalPort(this, "cheese")
+  val b = new CompositionalPort(this, "tart")
+}
+
+class PortsWinTester extends NamedModuleTester {
+  val wire = expectName(Wire(UInt()), "wire_1")
+  val foo = expectName(Wire(UInt()).suggestName("wire"), "wire_2")
+  val output = expectName(IO(Output(UInt())).suggestName("wire"), "wire")
+}
+
+class ProgrammaticPortsSpec extends ChiselFlatSpec {
+
+  private def doTest(testMod: => NamedModuleTester): Unit = {
+    var module: NamedModuleTester = null
+    elaborate { module = testMod; module }
+    assert(module.getNameFailures() == Nil)
+  }
+
+  "Programmatic port creation" should "be supported" in {
+    doTest(new PrivatePort)
+  }
+
+  "Calling IO outside of a Module definition" should "be supported" in {
+    doTest(new CompositionalPortTester)
+  }
+
+  "Ports" should "always win over internal components in naming" in {
+    doTest(new PortsWinTester)
+  }
+
+  "LegacyModule" should "ignore suggestName on ports" in {
+    doTest(new Module with NamedModuleTester {
+      val io = IO(new Bundle {
+        val foo = Output(UInt(8.W))
+      })
+      expectName(io.suggestName("cheese"), "io")
+      expectName(clock.suggestName("tart"), "clock")
+      expectName(reset.suggestName("teser"), "reset")
+    })
+  }
+
+  "SuggestName collisions on ports" should "be illegal" in {
+    a [ChiselException] should be thrownBy {
+      elaborate(new MultiIOModule {
+        val foo = IO(UInt(8.W)).suggestName("apple")
+        val bar = IO(UInt(8.W)).suggestName("apple")
+      })
+    }
+  }
+}
+