Rewrite BlackBox IO contract, replace _clock|_reset

The old blackbox behavior still emitted extmodules that have a
clk, reset pin and prepended all io's with io_ (ultimately). Most
verilog modules do not follow this distinction (or use a slightly
different name for clock and so on).

Thus, instead BlackBox has been rewritten to not assume a clk or
reset pin. Instead, the io Bundle specified is flattened directly
into the Module.ports declaration. The tests have been rewritten
to compensate for this. Also, added a test that uses the clock pin.

As a secondary change, the _clock and _reset module parameters were
bad for two reasons. One, they used null as a default, which is a
scala best practices violation. Two, they were just not good names.

Instead the primary constructor has been rewritten to take an
Option[Clock] called override_clock and an Option[Bool] called
override_reset, which default to None. (Note how the getOrElse call
down below is much more natural now.)

However, users may not want to specify the Some(their_clock) so I
also added secondary constructors that take parameters named clock
and reset and wrap them into Some calls into the primary constructor.
This is a better UX because now you can just stipulate clock=blah in
instantiation of that module in symmetry with using the clock in the
definition of the module by invoking clock.

PS: We could also back out of allowing any overrides via the Module
constructor and just require the instantiating Module to do
submodule.clock := newclock, etc.

1 files changed, 32 insertions, 2 deletions

diff --git a/src/main/scala/Chisel/BlackBox.scala b/src/main/scala/Chisel/BlackBox.scala
index ae0c59ba..48887271 100644
--- a/src/main/scala/Chisel/BlackBox.scala
+++ b/src/main/scala/Chisel/BlackBox.scala
@@ -2,6 +2,9 @@
 
 package Chisel
 
+import internal.Builder.pushCommand
+import internal.firrtl.{ModuleIO, DefInvalid}
+
 /** Defines a black box, which is a module that can be referenced from within
   * Chisel, but is not defined in the emitted Verilog. Useful for connecting
   * to RTL modules defined outside Chisel.
@@ -12,10 +15,37 @@ package Chisel
   * }}}
   */
 // REVIEW TODO: make Verilog parameters part of the constructor interface?
-abstract class BlackBox(_clock: Clock = null, _reset: Bool = null)
-    extends Module(_clock = _clock, _reset = _reset) {
+abstract class BlackBox extends Module {
+  // Don't bother taking override_clock|reset, clock/reset locked out anyway
   // TODO: actually implement this.
   def setVerilogParameters(s: String): Unit = {}
 
   // The body of a BlackBox is empty, the real logic happens in firrtl/Emitter.scala
+  // Bypass standard clock, reset, io port declaration by flattening io
+  // TODO(twigg): ? Really, overrides are bad, should extend BaseModule....
+  override private[Chisel] def ports = io.elements.toSeq
+
+  // Do not do reflective naming of internal signals, just name io
+  override private[Chisel] def setRefs(): this.type = {
+    for ((name, port) <- ports) {
+      port.setRef(ModuleIO(this, _namespace.name(name)))
+    }
+    io.setRef("") // don't io parts prepended with io_
+    this
+  }
+
+  // Don't setup clock, reset
+  // Cann't invalide io in one bunch, must invalidate each part separately
+  override private[Chisel] def setupInParent(): this.type = _parent match {
+    case Some(p) => {
+      // Just init instance inputs
+      for((_,port) <- ports) pushCommand(DefInvalid(port.ref))
+      this
+    }
+    case None => this
+  }
+
+  // Using null is horrible but these signals SHOULD NEVER be used:
+  override val clock = null
+  override val reset = null
 }