[core] Deprecate BlackBox in favor of ExtModule

Deprecate BlackBox and tell users to switch to ExtModule.  BlackBox has a
ton of baggage associated with it, due to it mandating, at runtime, a
single `val io` call and no additional calls to `IO` while also magically
removing the `io_` prefix in the generated FIRRTL.  `ExtModule` is far
cleaner and works as a user would expect.  You can call `IO` as much as
you want.  No magic prefixing happens.

This is slightly unfortunate as one could argue that `BlackBox` is a good
name.  However, you can also argue (more strongly I would add) that this
is an "external module" and it would be better to call it as such.  If
this is controversial, we can consider a migration back from `ExtModule`
to `BlackBox`.

Migrate some tests that were only testing blackboxes to be duplicated in
the external module tests.  This will allow us to just delete this test
later.

This has three motivations:

1. `BlackBox` has always been janky for the reasons above and `ExtModule`
is the saner generator API.

2. It's bad to have two similar, though subtly different ways of doing
things.

3. The singular IO that `BlackBox` forces users to go through means that
`BlackBox` cannot have domain information attached to it.  This
essentially means that `BlackBox` will be essentially unusable until
domains gain support for being in subfields.  It's easier to just
deprecate `BlackBox` than to require that domains gain this complicated
feature addition just now.

Signed-off-by: Schuyler Eldridge <[email protected]>

Author: seldridge

core/src/main/scala/chisel3/BlackBox.scala

@@ -46,6 +46,10 @@ import scala.collection.mutable
   * }}}
   * @note The parameters API is experimental and may change
   */
+@deprecated(
+  "use `chisel3.ExtModule` instead. To migrate replace `val io = { ... }` with `val io = FlatIO { ... }`, `object io { ... }; locally { io }`, or manually flatten your IO as `ExtModule` allows calling `IO` more than once.",
+  "7.5.0"
+)
 abstract class BlackBox(
   val params:                               Map[String, Param] = Map.empty[String, Param],
   override protected final val knownLayers: Seq[Layer] = Seq.empty[Layer]

core/src/main/scala/chisel3/Module.scala

@@ -2,6 +2,7 @@
 
 package chisel3
 
+import scala.annotation.nowarn
 import scala.collection.immutable.{ListMap, VectorBuilder}
 import scala.collection.mutable.{ArrayBuffer, HashMap, LinkedHashSet}
 
@@ -370,7 +371,7 @@ package internal {
       // currentModule (and not clonePorts)
       val clonePorts = proto match {
         // BlackBox needs special handling for its pseduo-io Bundle
-        case b: BlackBox =>
+        case b: BlackBox @nowarn("cat=deprecation") =>
           new ClonePorts(dataPorts :+ ("io" -> b._io.get): _*)
         case _ => new ClonePorts(dataPorts: _*)
       }

core/src/main/scala/chisel3/experimental/hierarchy/ModuleClone.scala

@@ -8,6 +8,7 @@ import chisel3.internal.firrtl.ir.{Component, ModuleCloneIO, Ref}
 import chisel3.internal.{throwException, Namespace}
 import chisel3.layer.Layer
 import chisel3._
+import scala.annotation.nowarn
 
 // Private internal class to serve as a _parent for Data in cloned ports
 private[chisel3] class ModuleClone[T <: BaseModule](val getProto: T)(implicit si: SourceInfo)
@@ -37,7 +38,7 @@ private[chisel3] class ModuleClone[T <: BaseModule](val getProto: T)(implicit si
   private[chisel3] lazy val ioMap: Map[Data, Data] = {
     getProto match {
       // BlackBox needs special handling for its pseduo-io Bundle
-      case protoBB: BlackBox =>
+      case protoBB: BlackBox @nowarn("cat=deprecation") =>
         Map(protoBB._io.get -> getPorts._elements("io"))
       case _ =>
         val name2Port = getPorts._elements
@@ -64,7 +65,7 @@ private[chisel3] class ModuleClone[T <: BaseModule](val getProto: T)(implicit si
     record.setRef(ref, force = true) // force because we did .forceName first
     getProto match {
       // BlackBox needs special handling for its pseduo-io Bundle
-      case _: BlackBox =>
+      case _: BlackBox @nowarn("cat=deprecation") =>
         // Override the io Bundle's ref so that it thinks it is the top for purposes of
         // generating FIRRTL
         record._elements("io").setRef(ref, force = true)

core/src/main/scala/chisel3/internal/BiConnect.scala

@@ -11,6 +11,7 @@ import chisel3.internal.Builder.pushCommand
 import chisel3.internal.firrtl.ir.{Connect, DefInvalid}
 import chisel3.internal.firrtl.Converter
 import chisel3.internal.MonoConnect.reportIfReadOnly
+import scala.annotation.nowarn
 
 import _root_.firrtl.passes.CheckTypes
 
@@ -309,8 +310,8 @@ private[chisel3] object BiConnect {
 
     // do not bulk connect the 'io' pseudo-bundle of a BlackBox since it will be decomposed in FIRRTL
     def blackBoxCheck = Seq(source, sink).map(_._parent).forall {
-      case Some(_: BlackBox) => false
-      case _                 => true
+      case Some(_: BlackBox @nowarn("cat=deprecation")) => false
+      case _                                            => true
     }
 
     typeCheck && contextCheck && bindingCheck && flowSinkCheck && flowSourceCheck && sourceAndSinkNotLiteralOrViewCheck && blackBoxCheck

src/main/scala-2/chisel3/interface/Interface.scala

@@ -95,13 +95,24 @@ trait Interface extends InterfaceCommon { self: Singleton =>
     /** The black box that has the same ports as this interface. This is what is
       * instantiated by any user of this interface, i.e., a test harness.
       */
+    @deprecated("use `ExtModule` instead", "7.5.0")
     final class BlackBox extends chisel3.BlackBox with Entity {
       final val io = IO(ports)
 
       /** Return the properties of this instance.  This requires brining a conformance into scope. */
       override final def properties[B <: BaseModule: Conformance]: Properties = implicitly[Conformance[B]].properties
     }
 
+    /** The black box that has the same ports as this interface. This is what is
+      * instantiated by any user of this interface, i.e., a test harness.
+      */
+    final class ExtModule extends chisel3.ExtModule with Entity {
+      final val io = FlatIO(ports)
+
+      /** Return the properties of this instance.  This requires brining a conformance into scope. */
+      override final def properties[B <: BaseModule: Conformance]: Properties = implicitly[Conformance[B]].properties
+    }
+
     /** The module that wraps any module which conforms to this Interface.
       */
     final class Module[B <: BaseModule](

src/main/scala/chisel3/util/BlackBoxUtils.scala

@@ -6,6 +6,14 @@ import chisel3._
 import chisel3.experimental.BlackBoxHelpers.BlackBoxInlineAnnoHelpers
 import firrtl.transforms.{BlackBoxInlineAnno, BlackBoxNotFoundException, BlackBoxPathAnno}
 
+private object BlackBoxUtils {
+  final val message =
+    "this trait will be removed in Chisel 8, please switch from `BlackBox` to `ExtModule` which has the methods of this trait already available"
+  final val since = "7.5.0"
+}
+import BlackBoxUtils._
+
+@deprecated(message, since)
 trait HasBlackBoxResource extends BlackBox {
   self: BlackBox =>
 
@@ -23,6 +31,7 @@ trait HasBlackBoxResource extends BlackBox {
   }
 }
 
+@deprecated(message, since)
 trait HasBlackBoxInline extends BlackBox {
   self: BlackBox =>
 
@@ -36,6 +45,7 @@ trait HasBlackBoxInline extends BlackBox {
   }
 }
 
+@deprecated(message, since)
 trait HasBlackBoxPath extends BlackBox {
   self: BlackBox =>
 

src/test/scala-2/chiselTests/AnalogIntegrationSpec.scala

@@ -3,10 +3,10 @@
 package chiselTests
 
 import chisel3._
+import chisel3.experimental.{attach, Analog}
 import chisel3.util._
 import chisel3.simulator.scalatest.ChiselSim
 import chisel3.simulator.stimulus.RunUntilFinished
-import chisel3.experimental._
 import org.scalatest.flatspec.AnyFlatSpec
 
 /* This test is different from AnalogSpec in that it uses more complicated black boxes that can each
@@ -29,8 +29,8 @@ class AnalogBlackBoxIO(val n: Int) extends Bundle {
 
 // Assigns bus to out
 // Assigns in.bits + index to bus when in.valid
-class AnalogBlackBox(index: Int) extends BlackBox(Map("index" -> index)) with HasBlackBoxResource {
-  val io = IO(new AnalogBlackBoxIO(1))
+class AnalogBlackBox(index: Int) extends ExtModule(Map("index" -> index)) {
+  val io = FlatIO(new AnalogBlackBoxIO(1))
 
   addResource("/chisel3/AnalogBlackBox.v")
 }

src/test/scala-2/chiselTests/AnalogSpec.scala

@@ -27,23 +27,23 @@ trait AnalogReader {
   def bus: Analog
 }
 
-class AnalogReaderBlackBox extends BlackBox with AnalogReader with HasBlackBoxResource {
-  val io = IO(new AnalogReaderIO)
+class AnalogReaderBlackBox extends ExtModule with AnalogReader {
+  val io = FlatIO(new AnalogReaderIO)
   def out = io.out
   def bus = io.bus
 
   addResource("/chisel3/AnalogBlackBox.v")
 }
 
 class AnalogReaderWrapper extends Module with AnalogReader {
-  val io = IO(new AnalogReaderIO)
+  val io = FlatIO(new AnalogReaderIO)
   def out = io.out
   def bus = io.bus
   val mod = Module(new AnalogReaderBlackBox)
   io <> mod.io
 }
-class AnalogWriterBlackBox extends BlackBox {
-  val io = IO(new AnalogWriterIO)
+class AnalogWriterBlackBox extends ExtModule {
+  val io = FlatIO(new AnalogWriterIO)
 }
 // Connects two Analog ports
 class AnalogConnector extends Module {

src/test/scala-2/chiselTests/BlackBox.scala

@@ -12,7 +12,9 @@ import chisel3.testing.scalatest.FileCheck
 import chisel3.util._
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
+import scala.annotation.nowarn
 
+@nowarn("cat=deprecation")
 class BlackBoxInverter extends BlackBox with HasBlackBoxResource {
   val io = IO(new Bundle() {
     val in = Input(Bool())
@@ -24,6 +26,7 @@ class BlackBoxInverter extends BlackBox with HasBlackBoxResource {
 
 // Due to the removal of "val io", this technically works
 // This style is discouraged, please use "val io"
+@nowarn("cat=deprecation")
 class BlackBoxInverterSuggestName extends BlackBox with HasBlackBoxResource {
   override def desiredName: String = "BlackBoxInverter"
   val foo = IO(new Bundle() {
@@ -34,6 +37,7 @@ class BlackBoxInverterSuggestName extends BlackBox with HasBlackBoxResource {
   addResource("/chisel3/BlackBoxInverter.v")
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxPassthrough extends BlackBox with HasBlackBoxResource {
   val io = IO(new Bundle() {
     val in = Input(Bool())
@@ -44,6 +48,7 @@ class BlackBoxPassthrough extends BlackBox with HasBlackBoxResource {
 }
 
 // Test Flip on top-level IO
+@nowarn("cat=deprecation")
 class BlackBoxPassthrough2 extends BlackBox with HasBlackBoxResource {
   val io = IO(Flipped(new Bundle() {
     val in = Output(Bool())
@@ -53,6 +58,7 @@ class BlackBoxPassthrough2 extends BlackBox with HasBlackBoxResource {
   addResource("/chisel3/BlackBoxPassthrough2.v")
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxRegister extends BlackBox with HasBlackBoxResource {
   val io = IO(new Bundle() {
     val clock = Input(Clock())
@@ -135,6 +141,7 @@ class BlackBoxWithClockTester extends Module {
   when(end) { stop() }
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxConstant(value: Int)
     extends BlackBox(Map("VALUE" -> value, "WIDTH" -> log2Ceil(value + 1)))
     with HasBlackBoxResource {
@@ -146,6 +153,7 @@ class BlackBoxConstant(value: Int)
   addResource("/chisel3/BlackBoxConstant.v")
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxStringParam(str: String) extends BlackBox(Map("STRING" -> str)) with HasBlackBoxResource {
   val io = IO(new Bundle {
     val out = UInt(32.W)
@@ -154,6 +162,7 @@ class BlackBoxStringParam(str: String) extends BlackBox(Map("STRING" -> str)) wi
   addResource("/chisel3/BlackBoxStringParam.v")
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxRealParam(dbl: Double) extends BlackBox(Map("REAL" -> dbl)) with HasBlackBoxResource {
   val io = IO(new Bundle {
     val out = UInt(64.W)
@@ -162,6 +171,7 @@ class BlackBoxRealParam(dbl: Double) extends BlackBox(Map("REAL" -> dbl)) with H
   addResource("/chisel3/BlackBoxRealParam.v")
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxTypeParam(w: Int, raw: String) extends BlackBox(Map("T" -> RawParam(raw))) with HasBlackBoxResource {
   val io = IO(new Bundle {
     val out = UInt(w.W)
@@ -170,6 +180,7 @@ class BlackBoxTypeParam(w: Int, raw: String) extends BlackBox(Map("T" -> RawPara
   addResource("/chisel3/BlackBoxTypeParam.v")
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxNoIO extends BlackBox with HasBlackBoxResource {
   // Whoops! typo
   val ioo = IO(new Bundle {
@@ -179,6 +190,7 @@ class BlackBoxNoIO extends BlackBox with HasBlackBoxResource {
   addResource("/chisel3/BlackBoxNoIO.v")
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxUIntIO extends BlackBox with HasBlackBoxResource {
   val io = IO(Output(UInt(8.W)))
 
@@ -253,7 +265,7 @@ class BlackBoxSpec extends AnyFlatSpec with Matchers with ChiselSim with FileChe
 
   "A Blackbox with Flipped IO" should "work" in {
     class Top extends RawModule {
-      val inst = Module(new BlackBox {
+      val inst = Module(new BlackBox @nowarn("cat=deprecation") {
         override def desiredName: String = "MyBB"
         val io = IO(Flipped(new Bundle {
           val in = Bool()
@@ -289,6 +301,7 @@ class BlackBoxSpec extends AnyFlatSpec with Matchers with ChiselSim with FileChe
 
   "BlackBoxes" should "sort the verilog output of their param map by param key" in {
 
+    @nowarn("cat=deprecation")
     class ParameterizedBlackBox(m: Map[String, Param]) extends BlackBox(m) {
       val io = IO(new Bundle {
         val out = Output(Clock())
@@ -360,17 +373,21 @@ class BlackBoxSpec extends AnyFlatSpec with Matchers with ChiselSim with FileChe
 
     object A extends layer.Layer(layer.LayerConfig.Extract())
 
-    sealed trait NoIo { this: BlackBox =>
+    sealed trait NoIo { this: BlackBox @nowarn("cat=deprecation") =>
       final val io = IO(new Bundle {})
     }
 
     // No known layers
+    @nowarn("cat=deprecation")
     class Bar extends BlackBox(knownLayers = Seq.empty) with NoIo
     // Single known layer, built-in
+    @nowarn("cat=deprecation")
     class Baz extends BlackBox(knownLayers = Seq(layers.Verification)) with NoIo
     // Multiple known layers
+    @nowarn("cat=deprecation")
     class Qux extends BlackBox(knownLayers = Seq(layers.Verification, layers.Verification.Assert)) with NoIo
     // Single known layer, user-defined and should be added to the circuit
+    @nowarn("cat=deprecation")
     class Quz extends BlackBox(knownLayers = Seq(A)) with NoIo
 
     class Foo extends Module {
@@ -398,6 +415,7 @@ class BlackBoxSpec extends AnyFlatSpec with Matchers with ChiselSim with FileChe
 
   they should "allow updates to knownLayers via adding layer-colored probe ports or via addLayer" in {
 
+    @nowarn("cat=deprecation")
     class Bar extends BlackBox {
       final val io = IO {
         new Bundle {
@@ -408,6 +426,7 @@ class BlackBoxSpec extends AnyFlatSpec with Matchers with ChiselSim with FileChe
 
     object A extends layer.Layer(layer.LayerConfig.Extract())
 
+    @nowarn("cat=deprecation")
     class Baz extends BlackBox(knownLayers = Seq(A)) {
       final val io = IO {
         new Bundle {
@@ -416,6 +435,7 @@ class BlackBoxSpec extends AnyFlatSpec with Matchers with ChiselSim with FileChe
       }
     }
 
+    @nowarn("cat=deprecation")
     class Qux extends BlackBox {
       final val io = IO(new Bundle {})
       layer.addLayer(A)

src/test/scala-2/chiselTests/BlackBoxImpl.scala

@@ -13,7 +13,9 @@ import org.scalacheck.Test.Failed
 import org.scalatest.Succeeded
 import org.scalatest.freespec.AnyFreeSpec
 import org.scalatest.matchers.should.Matchers
+import scala.annotation.nowarn
 
+@nowarn("cat=deprecation")
 class BlackBoxAdd(n: Int) extends HasBlackBoxInline {
   val io = IO(new Bundle {
     val in = Input(UInt(16.W))
@@ -44,6 +46,7 @@ class UsesBlackBoxAddViaInline extends Module {
   io.out := blackBoxAdd.io.out
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxMinus extends HasBlackBoxResource {
   val io = IO(new Bundle {
     val in1 = Input(UInt(16.W))
@@ -53,6 +56,7 @@ class BlackBoxMinus extends HasBlackBoxResource {
   addResource("/chisel3/BlackBoxMinus.v")
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxMinusPath extends HasBlackBoxPath {
   val io = IO(new Bundle {
     val in1 = Input(UInt(16.W))
@@ -90,6 +94,7 @@ class UsesBlackBoxMinusViaPath extends Module {
   io.out := mod0.io.out
 }
 
+@nowarn("cat=deprecation")
 class BlackBoxResourceNotFound extends HasBlackBoxResource {
   val io = IO(new Bundle {})
   addResource("/missing.resource")