Refactor tests to use stop() and assert() instead of io.error/io.done

Gate assert, printf, stop by reset
Fix testbenches that never worked
Change simulation prints to display cycle at which test was signaled to end, not when simulator stops
Better documentation for Counter

18 files changed, 155 insertions, 99 deletions

diff --git a/src/main/resources/top.cpp b/src/main/resources/top.cpp
index 756b5531..075d7085 100644
--- a/src/main/resources/top.cpp
+++ b/src/main/resources/top.cpp
@@ -23,7 +23,6 @@ double sc_time_stamp () { // Called by $time in Verilog
 const long timeout = 100000000L;
 
 int main(int argc, char** argv) {
-  vluint32_t done = 0;
   Verilated::commandArgs(argc, argv);   // Remember args
   top = new TOP_TYPE;
 
@@ -36,11 +35,11 @@ int main(int argc, char** argv) {
 #endif
 
 
-  top->reset = 1; 
+  top->reset = 1;
 
   cout << "Starting simulation!\n";
 
-  while (!Verilated::gotFinish() && !done && main_time < timeout) {
+  while (!Verilated::gotFinish() && main_time < timeout) {
     if (main_time > 10) {
       top->reset = 0;   // Deassert reset
     }
@@ -54,10 +53,18 @@ int main(int argc, char** argv) {
 #if VM_TRACE
 	if (tfp) tfp->dump (main_time);	// Create waveform trace for this timestamp
 #endif
-    done = top->io__024done;
     main_time++;               // Time passes...
   }
 
+  if (main_time >= timeout) {
+      cout << "Simulation terminated by timeout at time " << main_time <<
+              " (cycle " << main_time / 10 << ")"<< endl;
+      return -1;
+  } else {
+      cout << "Simulation completed at time " << main_time <<
+              " (cycle " << main_time / 10 << ")"<< endl;
+  }
+
   // Run for 10 more clocks
   vluint64_t end_time = main_time + 100;
   while (main_time < end_time) {
@@ -77,15 +84,5 @@ int main(int argc, char** argv) {
 #if VM_TRACE
     if (tfp) tfp->close();
 #endif
-
-  if (main_time >= timeout) {
-    cout << "Simulation terminated by timeout at cycle " << main_time << endl;
-    return -1;
-  } else {
-    cout << "Simulation completed at cycle " << main_time << endl;
-    int error = top->io__024error;
-    cout << "Simulation return value: " << error << endl;
-    return error;
-  }
 }
 
diff --git a/src/main/scala/Chisel/CoreUtil.scala b/src/main/scala/Chisel/CoreUtil.scala
index 41266cae..aaca404b 100644
--- a/src/main/scala/Chisel/CoreUtil.scala
+++ b/src/main/scala/Chisel/CoreUtil.scala
@@ -7,21 +7,52 @@ import internal.Builder.pushCommand
 import firrtl._
 
 object assert {
+  /** Checks for a condition to be valid in the circuit at all times. If the
+    * condition evaluates to false, the circuit simulation stops with an error.
+    *
+    * Does not fire when in reset (defined as the encapsulating Module's
+    * reset). If your definition of reset is not the encapsulating Module's
+    * reset, you will need to gate this externally.
+    *
+    * May be called outside of a Module (like defined in a function), so
+    * functions using assert make the standard Module assumptions (single clock
+    * and single reset).
+    *
+    * @param cond condition, assertion fires (simulation fails) when false
+    * @param message optional message to print when the assertion fires
+    */
   def apply(cond: Bool, message: String="") {
-    when(!cond) {
-      if (message.isEmpty()) {
-        printf(s"Assertion failed: (TODO: code / lineno)")
-      } else {
-        printf(s"Assertion failed: (TODO: code / lineno): $message")
+    when (!Builder.dynamicContext.currentModule.get.reset) {
+      when(!cond) {
+        if (message.isEmpty()) {
+          printf(s"Assertion failed: (TODO: code / lineno)")
+        } else {
+          printf(s"Assertion failed: (TODO: code / lineno): $message")
+        }
+        pushCommand(Stop(Node(Builder.dynamicContext.currentModule.get.clock), 1))
       }
-      pushCommand(Stop(Node(Builder.dynamicContext.currentModule.get.clock), 1))
     }
   }
 }
 
 object printf {
+  /** Prints a message in simulation.
+    *
+    * Does not fire when in reset (defined as the encapsulating Module's
+    * reset). If your definition of reset is not the encapsulating Module's
+    * reset, you will need to gate this externally.
+    *
+    * May be called outside of a Module (like defined in a function), so
+    * functions using printf make the standard Module assumptions (single clock
+    * and single reset).
+    *
+    * @param fmt printf format string
+    * @param data format string varargs containing data to print
+    */
   def apply(fmt: String, data: Bits*) {
-    pushCommand(Printf(Node(Builder.dynamicContext.currentModule.get.clock),
-        fmt, data.map(Node(_))))
+    when (!Builder.dynamicContext.currentModule.get.reset) {
+      pushCommand(Printf(Node(Builder.dynamicContext.currentModule.get.clock),
+          fmt, data.map((d: Bits) => d.ref)))
+    }
   }
 }
diff --git a/src/main/scala/Chisel/testers/BasicTester.scala b/src/main/scala/Chisel/testers/BasicTester.scala
index fecbe2a9..1079727c 100644
--- a/src/main/scala/Chisel/testers/BasicTester.scala
+++ b/src/main/scala/Chisel/testers/BasicTester.scala
@@ -8,18 +8,20 @@ import internal.Builder.pushCommand
 import firrtl._
 
 class BasicTester extends Module {
-  val io = new Bundle {
-    val done = Bool()
-    val error = UInt(width = 4)
-  }
-  io.done := Bool(false)
-  io.error := UInt(0)
+  // The testbench has no IOs, rather it should communicate using printf, assert, and stop.
+  val io = new Bundle()
 
   def popCount(n: Long): Int = n.toBinaryString.count(_=='1')
 
-  /** Ends the test, reporting success.
+  /** Ends the test reporting success.
+    *
+    * Does not fire when in reset (defined as the encapsulating Module's
+    * reset). If your definition of reset is not the encapsulating Module's
+    * reset, you will need to gate this externally.
     */
   def stop() {
-    pushCommand(Stop(Node(clock), 0))
+    when (!reset) {
+      pushCommand(Stop(Node(clock), 0))
+    }
   }
 }
diff --git a/src/main/scala/Chisel/util/Counter.scala b/src/main/scala/Chisel/util/Counter.scala
index e4a8e95b..356cedc8 100644
--- a/src/main/scala/Chisel/util/Counter.scala
+++ b/src/main/scala/Chisel/util/Counter.scala
@@ -3,10 +3,14 @@
 package Chisel
 
 /** A counter module
-  * @param n The maximum value of the counter, does not have to be power of 2
+  * @param n number of counts before the counter resets (or one more than the
+  * maximum output value of the counter), need not be a power of two
   */
 class Counter(val n: Int) {
   val value = if (n == 1) UInt(0) else Reg(init=UInt(0, log2Up(n)))
+  /** Increment the counter this cycle. Returns whether the counter is at its
+    * maximum (and will wrap around on the next inc() call).
+    */
   def inc(): Bool = {
     if (n == 1) {
       Bool(true)
diff --git a/src/test/scala/chiselTests/Assert.scala b/src/test/scala/chiselTests/Assert.scala
index 31e53f36..86494855 100644
--- a/src/test/scala/chiselTests/Assert.scala
+++ b/src/test/scala/chiselTests/Assert.scala
@@ -8,14 +8,12 @@ import Chisel.testers.BasicTester
 
 class FailingAssertTester() extends BasicTester {
   assert(Bool(false))
-  io.done := Bool(true)
-  io.error := Bool(false)
+  stop()
 }
 
 class SucceedingAssertTester() extends BasicTester {
   assert(Bool(true))
-  io.done := Bool(true)
-  io.error := Bool(false)
+  stop()
 }
 
 class AssertSpec extends ChiselFlatSpec {
diff --git a/src/test/scala/chiselTests/BitwiseOps.scala b/src/test/scala/chiselTests/BitwiseOps.scala
index d180c11e..31feaada 100644
--- a/src/test/scala/chiselTests/BitwiseOps.scala
+++ b/src/test/scala/chiselTests/BitwiseOps.scala
@@ -8,18 +8,17 @@ import org.scalatest.prop._
 import Chisel.testers.BasicTester
 
 class BitwiseOpsTester(w: Int, _a: Int, _b: Int) extends BasicTester {
-  io.done := Bool(true)
   val mask = (1 << w) - 1
-  val a = UInt(_a)
-  val b = UInt(_b)
-  when(~a != UInt(mask & ~_a)) { io.error := UInt(1) }
-  when((a & b) != UInt(mask & (_a & _b))) { io.error := UInt(2) }
-  when((a | b)  != UInt(mask & (_a | _b))) { io.error := UInt(3) }
-  when((a ^ b) != UInt(mask & (_a ^ _b))) { io.error := UInt(4) }
+  val a = UInt(_a, w)
+  val b = UInt(_b, w)
+  assert(~a === UInt(mask & ~_a))
+  assert((a & b) === UInt(_a & _b))
+  assert((a | b) === UInt(_a | _b))
+  assert((a ^ b) === UInt(_a ^ _b))
+  stop()
 }
 
 class BitwiseOpsSpec extends ChiselPropSpec {
-
   property("All bit-wise ops should return the correct result") {
     forAll(safeUIntPair) { case(w: Int, a: Int, b: Int) =>
       assert(execute{ new BitwiseOpsTester(w, a, b) })
diff --git a/src/test/scala/chiselTests/BundleWire.scala b/src/test/scala/chiselTests/BundleWire.scala
index 5beed039..128b2c5f 100644
--- a/src/test/scala/chiselTests/BundleWire.scala
+++ b/src/test/scala/chiselTests/BundleWire.scala
@@ -25,10 +25,13 @@ class BundleWire(n: Int) extends Module {
 
 class BundleWireTester(n: Int, x: Int, y: Int) extends BasicTester {
   val dut = Module(new BundleWire(n))
-  io.done := Bool(true)
   dut.io.in.x := UInt(x)
   dut.io.in.y := UInt(y)
-  io.error := dut.io.outs.map(o => o.x != UInt(x) || o.y != UInt(y)).foldLeft(UInt(0))(_##_)
+  for (elt <- dut.io.outs) {
+    assert(elt.x === UInt(x))
+    assert(elt.y === UInt(y))
+  }
+  stop()
 }
 
 class BundleWireSpec extends ChiselPropSpec {
diff --git a/src/test/scala/chiselTests/ComplexAssign.scala b/src/test/scala/chiselTests/ComplexAssign.scala
index bbd3d6c2..02f0e1ac 100644
--- a/src/test/scala/chiselTests/ComplexAssign.scala
+++ b/src/test/scala/chiselTests/ComplexAssign.scala
@@ -37,13 +37,13 @@ class ComplexAssignTester(enList: List[Boolean], re: Int, im: Int) extends Basic
   dut.io.e := Vec(enList.map(Bool(_)))(cnt)
   val re_correct = dut.io.out.re === Mux(dut.io.e, dut.io.in.re, UInt(0))
   val im_correct = dut.io.out.im === Mux(dut.io.e, dut.io.in.im, UInt(0))
-  when(!re_correct || !im_correct) {
-    io.done := Bool(true); io.error := cnt
-  } .elsewhen(wrap) { io.done := Bool(true) }
+  assert(re_correct && im_correct)
+  when(wrap) {
+    stop()
+  }
 }
 
 class ComplexAssignSpec extends ChiselPropSpec {
-
   property("All complex assignments should return the correct result") {
     forAll(enSequence(2), safeUInts, safeUInts) { (en: List[Boolean], re: Int, im: Int) =>
       assert(execute{ new ComplexAssignTester(en, re, im) })
diff --git a/src/test/scala/chiselTests/Counter.scala b/src/test/scala/chiselTests/Counter.scala
index 1aa2aaba..aa21423f 100644
--- a/src/test/scala/chiselTests/Counter.scala
+++ b/src/test/scala/chiselTests/Counter.scala
@@ -9,27 +9,33 @@ import Chisel.testers.BasicTester
 class CountTester(max: Int) extends BasicTester {
   val cnt = Counter(max)
   when(Bool(true)) { cnt.inc() }
-  when(cnt.value === UInt(max-1)) { io.done := Bool(true) }
+  when(cnt.value === UInt(max-1)) {
+    stop()
+  }
 }
 
 class EnableTester(seed: Int) extends BasicTester {
   val ens = Reg(init = UInt(seed))
   ens := ens >> 1
-  val (cntEn, cntWrap) = Counter(ens(0), 32)
-  val cnt = Counter(Bool(true), 32)._1
-  when(cnt === UInt(31)) {
-    io.done := Bool(true)
-    io.error := cnt != UInt(popCount(seed))
+
+  val (cntEnVal, _) = Counter(ens(0), 32)
+  val (_, done) = Counter(Bool(true), 33)
+
+  when(done) {
+    assert(cntEnVal === UInt(popCount(seed)))
+    stop()
   }
 }
 
 class WrapTester(max: Int) extends BasicTester {
   val (cnt, wrap) = Counter(Bool(true), max)
-  when(wrap) { io.done := Bool(true); io.error := cnt != UInt(max) }
+  when(wrap) {
+    assert(cnt === UInt(max - 1))
+    stop()
+  }
 }
 
 class CounterSpec extends ChiselPropSpec {
-
   property("Counter should count up") {
     forAll(smallPosInts) { (max: Int) => assert(execute{ new CountTester(max) }) }
   }
diff --git a/src/test/scala/chiselTests/Decoder.scala b/src/test/scala/chiselTests/Decoder.scala
index 7751804b..8a9c5a5b 100644
--- a/src/test/scala/chiselTests/Decoder.scala
+++ b/src/test/scala/chiselTests/Decoder.scala
@@ -20,8 +20,13 @@ class DecoderTester(pairs: List[(String, String)]) extends BasicTester {
   val (cnt, wrap) = Counter(Bool(true), pairs.size)
   val dut = Module(new Decoder(bitpats))
   dut.io.inst := Vec(insts.map(UInt(_)))(cnt)
-  when(!dut.io.matched) { io.done := Bool(true); io.error := cnt }
-  when(wrap) { io.done := Bool(true) }
+  when(!dut.io.matched) {
+    assert(cnt === UInt(0))
+    stop()
+  }
+  when(wrap) {
+    stop()
+  }
 }
 
 class DecoderSpec extends ChiselPropSpec {
diff --git a/src/test/scala/chiselTests/GCD.scala b/src/test/scala/chiselTests/GCD.scala
index 529f9964..8fdf8db0 100644
--- a/src/test/scala/chiselTests/GCD.scala
+++ b/src/test/scala/chiselTests/GCD.scala
@@ -32,8 +32,8 @@ class GCDTester(a: Int, b: Int, z: Int) extends BasicTester {
   dut.io.e := first
   when(first) { first := Bool(false) }
   when(dut.io.v) {
-    io.done := Bool(true)
-    io.error := (dut.io.z != UInt(z)).toUInt
+    assert(dut.io.z === UInt(z))
+    stop()
   }
 }
 
@@ -46,7 +46,7 @@ class GCDSpec extends ChiselPropSpec {
     ("a", "b", "z"),  // First tuple defines column names
     ( 64,  48,  16),  // Subsequent tuples define the data
     ( 12,   9,   3),
-    ( 48,  64,  12))
+    ( 48,  64,  16))
 
   property("GCD should elaborate") {
     elaborate { new GCD }
diff --git a/src/test/scala/chiselTests/MulLookup.scala b/src/test/scala/chiselTests/MulLookup.scala
index 93917a4e..caaeac2c 100644
--- a/src/test/scala/chiselTests/MulLookup.scala
+++ b/src/test/scala/chiselTests/MulLookup.scala
@@ -26,8 +26,8 @@ class MulLookupTester(w: Int, x: Int, y: Int) extends BasicTester {
   val dut = Module(new MulLookup(w))
   dut.io.x := UInt(x)
   dut.io.y := UInt(y)
-  io.done := Bool(true)
-  io.error := dut.io.z != UInt(x * y)
+  assert(dut.io.z === UInt(x * y))
+  stop()
 }
 
 class MulLookupSpec extends ChiselPropSpec {
diff --git a/src/test/scala/chiselTests/OptionBundle.scala b/src/test/scala/chiselTests/OptionBundle.scala
index 1b7be1ec..b23c1f6b 100644
--- a/src/test/scala/chiselTests/OptionBundle.scala
+++ b/src/test/scala/chiselTests/OptionBundle.scala
@@ -27,21 +27,21 @@ class OptionBundleModule(hasIn: Boolean) extends Module {
 class SomeOptionBundleTester(expected: Boolean) extends BasicTester {
   val mod = Module(new OptionBundleModule(true))
   mod.io.in.get := Bool(expected)
-  io.error := mod.io.out != Bool(expected)
-  io.done := Bool(true)
+  assert(mod.io.out === Bool(expected))
+  stop()
 }
 
 class NoneOptionBundleTester() extends BasicTester {
   val mod = Module(new OptionBundleModule(true))
-  io.error := mod.io.out != Bool(false)
-  io.done := Bool(true)
+  assert(mod.io.out === Bool(false))
+  stop()
 }
 
 class InvalidOptionBundleTester() extends BasicTester {
   val mod = Module(new OptionBundleModule(false))
   mod.io.in.get := Bool(true)
-  io.error := UInt(1)
-  io.done := Bool(true)
+  assert(Bool(false))
+  stop()
 }
 
 class OptionBundleSpec extends ChiselFlatSpec {
diff --git a/src/test/scala/chiselTests/Printf.scala b/src/test/scala/chiselTests/Printf.scala
index e6f5d222..2e26dfbf 100644
--- a/src/test/scala/chiselTests/Printf.scala
+++ b/src/test/scala/chiselTests/Printf.scala
@@ -7,15 +7,16 @@ import Chisel._
 import Chisel.testers.BasicTester
 
 class SinglePrintfTester() extends BasicTester {
-  printf("done=%x", io.done)
-  io.done := Bool(true)
-  io.error := Bool(false)
+  val x = UInt(254)
+  printf("x=%x", x)
+  stop()
 }
 
 class MultiPrintfTester() extends BasicTester {
-  printf("done=%x error=%x", io.done, io.error)
-  io.done := Bool(true)
-  io.error := Bool(false)
+  val x = UInt(254)
+  val y = UInt(255)
+  printf("x=%x y=%x", x, y)
+  stop()
 }
 
 class PrintfSpec extends ChiselFlatSpec {
diff --git a/src/test/scala/chiselTests/Stop.scala b/src/test/scala/chiselTests/Stop.scala
index 7ce9461b..4c1db1d6 100644
--- a/src/test/scala/chiselTests/Stop.scala
+++ b/src/test/scala/chiselTests/Stop.scala
@@ -7,7 +7,6 @@ import Chisel._
 import Chisel.testers.BasicTester
 
 class StopTester() extends BasicTester {
-  io.done := Bool(false)
   stop()
 }
 
diff --git a/src/test/scala/chiselTests/Tbl.scala b/src/test/scala/chiselTests/Tbl.scala
index a3b1feb0..003554c8 100644
--- a/src/test/scala/chiselTests/Tbl.scala
+++ b/src/test/scala/chiselTests/Tbl.scala
@@ -19,7 +19,9 @@ class Tbl(w: Int, n: Int) extends Module {
   io.o := m(io.ri)
   when (io.we) {
     m(io.wi) := io.d
-    when(io.ri === io.wi) { io.o := io.d }
+    when(io.ri === io.wi) {
+      io.o := io.d
+    }
   }
 }
 
@@ -34,18 +36,24 @@ class TblTester(w: Int, n: Int, idxs: List[Int], values: List[Int]) extends Basi
   dut.io.ri := prev_idx
   dut.io.we := Bool(true) //TODO enSequence
   dut.io.d := vvalues(cnt)
-  when(cnt > UInt(0) && dut.io.o != prev_value) { io.done := Bool(true); io.error := prev_idx }
-  when(wrap) { io.done := Bool(true) }
+  when (cnt > UInt(0)) {
+    when (prev_idx === vidxs(cnt)) {
+      assert(dut.io.o === vvalues(cnt))
+    } .otherwise {
+      assert(dut.io.o === prev_value)
+    }
+  }
+  when(wrap) {
+    stop()
+  }
 }
 
 class TblSpec extends ChiselPropSpec {
-
   property("All table reads should return the previous write") {
     forAll(safeUIntPairN(8)) { case(w: Int, pairs: List[(Int, Int)]) =>
-      whenever(w > 0) {
-        val (idxs, values) = pairs.unzip
-        assert(execute{ new TblTester(w, 1 << w, idxs, values) })
-      }
+      require(w > 0)
+      val (idxs, values) = pairs.unzip
+      assert(execute{ new TblTester(w, 1 << w, idxs, values) })
     }
   }
 }
diff --git a/src/test/scala/chiselTests/Vec.scala b/src/test/scala/chiselTests/Vec.scala
index 6d16ec08..0c3d046e 100644
--- a/src/test/scala/chiselTests/Vec.scala
+++ b/src/test/scala/chiselTests/Vec.scala
@@ -8,36 +8,40 @@ import org.scalatest.prop._
 import Chisel.testers.BasicTester
 
 class ValueTester(w: Int, values: List[Int]) extends BasicTester {
-  io.done := Bool(true)
   val v = Vec(values.map(UInt(_, width = w))) // TODO: does this need a Wire? Why no error?
-  io.error := v.zip(values).map { case(a,b) =>
-    a != UInt(b)
-  }.foldLeft(UInt(0))(_##_)
+  for ((a,b) <- v.zip(values)) {
+    assert(a === UInt(b))
+  }
+  stop()
 }
 
 class TabulateTester(n: Int) extends BasicTester {
-  io.done := Bool(true)
   val v = Vec(Range(0, n).map(i => UInt(i * 2)))
   val x = Vec(Array.tabulate(n){ i => UInt(i * 2) })
   val u = Vec.tabulate(n)(i => UInt(i*2))
-  when(v.toBits != x.toBits) { io.error := UInt(1) }
-  when(v.toBits != u.toBits) { io.error := UInt(2) }
-  when(x.toBits != u.toBits) { io.error := UInt(3) }
+
+  assert(v.toBits === x.toBits)
+  assert(v.toBits === u.toBits)
+  assert(x.toBits === u.toBits)
+
+  stop()
 }
 
 class ShiftRegisterTester(n: Int) extends BasicTester {
   val (cnt, wrap) = Counter(Bool(true), n*2)
-  when(wrap) { io.done := Bool(true) }
-
   val shifter = Reg(Vec(UInt(width = log2Up(n)), n))
   (shifter, shifter drop 1).zipped.foreach(_ := _)
   shifter(n-1) := cnt
-  val expected = cnt - UInt(n)
-  when(cnt >= UInt(n) && expected != shifter(0)) { io.done := Bool(true); io.error := expected }
+  when (cnt >= UInt(n)) {
+    val expected = cnt - UInt(n)
+    assert(shifter(0) === expected)
+  }
+  when (wrap) {
+    stop()
+  }
 }
 
 class VecSpec extends ChiselPropSpec {
-
   property("Vecs should be assignable") {
     forAll(safeUIntN(8)) { case(w: Int, v: List[Int]) =>
       assert(execute{ new ValueTester(w, v) })
diff --git a/src/test/scala/chiselTests/VendingMachine.scala b/src/test/scala/chiselTests/VendingMachine.scala
index 8a21cb2e..012fc493 100644
--- a/src/test/scala/chiselTests/VendingMachine.scala
+++ b/src/test/scala/chiselTests/VendingMachine.scala
@@ -58,7 +58,6 @@ class VendingMachineTester(c: VendingMachine) extends Tester(c) {
 */
 
 class VendingMachineSpec extends ChiselPropSpec {
-  
   property("VendingMachine should elaborate") {
     elaborate { new VendingMachine }
   }