Make ExtModule port naming consistent with Module (#2548) (#2549)

ExtModule now uses the same namePorts implementation as regular Modules.
Previously, ExtModules only allowed port naming via runtime reflection.
This meant that .suggestName and other naming APIs do not work. It also
breaks FlatIO for ExtModule which is a potential replacement API for
BlackBox's special `val io` handling.

(cherry picked from commit 83cccfb782d9141bf2c843246c2a525c62392924)

Co-authored-by: Jack Koenig <koenig@sifive.com>

4 files changed, 67 insertions, 26 deletions

diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala
index f3fc2711..f618901f 100644
--- a/core/src/main/scala/chisel3/BlackBox.scala
+++ b/core/src/main/scala/chisel3/BlackBox.scala
@@ -71,11 +71,8 @@ package experimental {
 
       val names = nameIds(classOf[ExtModule])
 
-      // Name ports based on reflection
-      for (port <- getModulePorts) {
-        require(names.contains(port), s"Unable to name port $port in $this")
-        port.setRef(ModuleIO(this, _namespace.name(names(port))))
-      }
+      // Ports are named in the same way as regular Modules
+      namePorts(names)
 
       // All suggestions are in, force names to every node.
       // While BlackBoxes are not supposed to have an implementation, we still need to call
diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala
index 84139630..3382cd1b 100644
--- a/core/src/main/scala/chisel3/Module.scala
+++ b/core/src/main/scala/chisel3/Module.scala
@@ -486,6 +486,27 @@ package experimental {
       */
     private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit
 
+    private[chisel3] def namePorts(names: HashMap[HasId, String]): Unit = {
+      for (port <- getModulePorts) {
+        port._computeName(None, None).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"
+            )
+            port.setRef(ModuleIO(this, "<UNNAMED>"))
+        }
+      }
+    }
+
     //
     // Chisel Internals
     //
diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala
index 12b6a76a..c257f0c6 100644
--- a/core/src/main/scala/chisel3/RawModule.scala
+++ b/core/src/main/scala/chisel3/RawModule.scala
@@ -43,27 +43,6 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions) extends
 
   val compileOptions = moduleCompileOptions
 
-  private[chisel3] def namePorts(names: HashMap[HasId, String]): Unit = {
-    for (port <- getModulePorts) {
-      port._computeName(None, None).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"
-          )
-          port.setRef(ModuleIO(this, "<UNNAMED>"))
-      }
-    }
-  }
-
   private[chisel3] override def generateComponent(): Option[Component] = {
     require(!_closed, "Can't generate module more than once")
     _closed = true
diff --git a/src/test/scala/chiselTests/ExtModule.scala b/src/test/scala/chiselTests/ExtModule.scala
index 1dbd7447..b5a8ff7c 100644
--- a/src/test/scala/chiselTests/ExtModule.scala
+++ b/src/test/scala/chiselTests/ExtModule.scala
@@ -59,6 +59,35 @@ class MultiExtModuleTester extends BasicTester {
   stop()
 }
 
+class ExtModuleWithSuggestName extends ExtModule {
+  val in = IO(Input(UInt(8.W)))
+  in.suggestName("foo")
+  val out = IO(Output(UInt(8.W)))
+}
+
+class ExtModuleWithSuggestNameTester extends Module {
+  val in = IO(Input(UInt(8.W)))
+  val out = IO(Output(UInt(8.W)))
+  val inst = Module(new ExtModuleWithSuggestName)
+  inst.in := in
+  out := inst.out
+}
+
+class SimpleIOBundle extends Bundle {
+  val in = Input(UInt(8.W))
+  val out = Output(UInt(8.W))
+}
+
+class ExtModuleWithFlatIO extends ExtModule {
+  val badIO = FlatIO(new SimpleIOBundle)
+}
+
+class ExtModuleWithFlatIOTester extends Module {
+  val io = IO(new SimpleIOBundle)
+  val inst = Module(new ExtModuleWithFlatIO)
+  io <> inst.badIO
+}
+
 class ExtModuleSpec extends ChiselFlatSpec {
   "A ExtModule inverter" should "work" in {
     assertTesterPasses({ new ExtModuleTester }, Seq("/chisel3/BlackBoxTest.v"), TesterDriver.verilatorOnly)
@@ -73,4 +102,19 @@ class ExtModuleSpec extends ChiselFlatSpec {
       assert(DataMirror.modulePorts(m) == Seq("in" -> m.in, "out" -> m.out))
     })
   }
+
+  behavior.of("ExtModule")
+
+  it should "work with .suggestName (aka it should not require reflection for naming)" in {
+    val chirrtl = ChiselStage.emitChirrtl(new ExtModuleWithSuggestNameTester)
+    chirrtl should include("input foo : UInt<8>")
+    chirrtl should include("inst.foo <= in")
+  }
+
+  it should "work with FlatIO" in {
+    val chirrtl = ChiselStage.emitChirrtl(new ExtModuleWithFlatIOTester)
+    chirrtl should include("io.out <= inst.out")
+    chirrtl should include("inst.in <= io.in")
+    chirrtl shouldNot include("badIO")
+  }
 }