From 41f4eef0d85b65fabd0d786efa8baa099513dcf0 Mon Sep 17 00:00:00 2001 From: Chick Markley Date: Mon, 29 Jul 2019 11:35:09 -0700 Subject: [PATCH] Change the verilator generation code to call dut->eval() instead (#252) of dut->_eval_settle(dut->__VlSymsp) Test in SecondClockDrivesRegisterSpec will only work with this change. GCDSpec has code to do regression work to make sure the above change did not significantly slow the verilator simulation. --- .../chisel3/iotesters/VerilatorBackend.scala | 5 +- src/test/scala/examples/GCDSpec.scala | 69 +++++++++++++------ .../SecondClockDrivesRegisterSpec.scala | 62 +++++++++++++++++ 3 files changed, 113 insertions(+), 23 deletions(-) create mode 100644 src/test/scala/examples/SecondClockDrivesRegisterSpec.scala diff --git a/src/main/scala/chisel3/iotesters/VerilatorBackend.scala b/src/main/scala/chisel3/iotesters/VerilatorBackend.scala index 7b358056..ff7f057e 100644 --- a/src/main/scala/chisel3/iotesters/VerilatorBackend.scala +++ b/src/main/scala/chisel3/iotesters/VerilatorBackend.scala @@ -149,7 +149,10 @@ class $dutApiClassName: public sim_api_t { main_time++; } virtual inline void update() { - dut->_eval_settle(dut->__VlSymsp); + // This seems to force a full eval of circuit, so registers with alternate clocks are update correctly + dut->eval(); + // This was the original call, did not refresh registers when some other clock transitioned + // dut->_eval_settle(dut->__VlSymsp); } }; diff --git a/src/test/scala/examples/GCDSpec.scala b/src/test/scala/examples/GCDSpec.scala index 570a87bc..c872fd2a 100644 --- a/src/test/scala/examples/GCDSpec.scala +++ b/src/test/scala/examples/GCDSpec.scala @@ -8,6 +8,7 @@ import chisel3._ import chisel3.util._ import chisel3.iotesters._ import org.scalatest.{FlatSpec, Matchers} +import treadle.chronometry.Timer object RealGCD2 { val num_width = 16 @@ -54,28 +55,38 @@ class RealGCD2 extends Module { } } -class GCDPeekPokeTester(c: RealGCD2) extends PeekPokeTester(c) { - for { - i <- 1 to 10 - j <- 1 to 10 - } { - val (gcd_value, _) = GCDCalculator.computeGcdResultsAndCycles(i, j) - - poke(c.io.RealGCD2in.bits.a, i) - poke(c.io.RealGCD2in.bits.b, j) - poke(c.io.RealGCD2in.valid, 1) - - var count = 0 - while(peek(c.io.RealGCD2out.valid) == BigInt(0) && count < 20) { - step(1) - count += 1 +class GCDPeekPokeTester(c: RealGCD2, maxX: Int = 10, maxY: Int = 10, showTiming: Boolean = false) + extends PeekPokeTester(c) { + val timer = new Timer + + timer("overall") { + for { + i <- 1 to maxX + j <- 1 to maxY + } { + val (gcd_value, _) = GCDCalculator.computeGcdResultsAndCycles(i, j) + + timer("operation") { + poke(c.io.RealGCD2in.bits.a, i) + poke(c.io.RealGCD2in.bits.b, j) + poke(c.io.RealGCD2in.valid, 1) + + var count = 0 + while (peek(c.io.RealGCD2out.valid) == BigInt(0) && count < 20000) { + step(1) + count += 1 + } + if (count > 30000) { + // println(s"Waited $count cycles on gcd inputs $i, $j, giving up") + System.exit(0) + } + expect(c.io.RealGCD2out.bits, gcd_value) + step(1) + } } - if(count > 30) { - // println(s"Waited $count cycles on gcd inputs $i, $j, giving up") - System.exit(0) - } - expect(c.io.RealGCD2out.bits, gcd_value) - step(1) + } + if(showTiming) { + println(s"\n${timer.report()}") } } @@ -95,7 +106,8 @@ class GCDSpec extends FlatSpec with Matchers { new GCDPeekPokeTester(c) } should be (true) } - it should "run firrtl via command line arguments" in { + + it should "run firrtl-interpreter via command line arguments" in { // val args = Array.empty[String] val args = Array("--backend-name", "firrtl", "--fint-write-vcd") iotesters.Driver.execute(args, () => new RealGCD2) { c => @@ -138,5 +150,18 @@ class GCDSpec extends FlatSpec with Matchers { new File("test_run_dir/gcd_make_vcd/RealGCD2.vcd").exists() should be (true) } + + it should "run verilator with larger input vector to run regressions" in { + // + // Use this test combined with changing the comments on VerilatorBackend.scala lines 153 and 155 to + // measure the consequence of that change, at the time of last using this the cost appeared to be < 3% + // + val args = Array("--backend-name", "verilator") + iotesters.Driver.execute(args, () => new RealGCD2) { c => + new GCDPeekPokeTester(c, 100, 1000, showTiming = true) + } should be (true) + } + + } diff --git a/src/test/scala/examples/SecondClockDrivesRegisterSpec.scala b/src/test/scala/examples/SecondClockDrivesRegisterSpec.scala new file mode 100644 index 00000000..47249070 --- /dev/null +++ b/src/test/scala/examples/SecondClockDrivesRegisterSpec.scala @@ -0,0 +1,62 @@ +package examples + +import chisel3._ +import chisel3.experimental.MultiIOModule +import chisel3.iotesters.PeekPokeTester +import chisel3.util.Counter +import org.scalatest.{FreeSpec, Matchers} + + +class SecondClockDrivesRegisterSpec extends FreeSpec with Matchers { + class SecondClock extends MultiIOModule { + val inClock = IO(Input(Bool())) + val out = IO(Output(UInt(8.W))) + + withClock(inClock.asClock) { + out := Counter(true.B, 8)._1 + } + } + + class SecondClockTester(c: SecondClock) extends PeekPokeTester(c) { + poke(c.inClock, 0) + expect(c.out, 0) + + // Main clock should do nothing + step(1) + expect(c.out, 0) + step(1) + expect(c.out, 0) + + // Output should advance on rising edge, even without main clock edge + poke(c.inClock, 1) + expect(c.out, 1) + + step(1) + expect(c.out, 1) + + // Repeated, 1should do nothing + poke(c.inClock, 1) + expect(c.out, 1) + + // and again + poke(c.inClock, 0) + expect(c.out, 1) + poke(c.inClock, 1) + expect(c.out, 2) + } + + "poking a clock should flip register" - { + + "should work with Treadle" in { + iotesters.Driver.execute(Array(), () => new SecondClock) { c => + new SecondClockTester(c) + } should be(true) + } + + "should work with Verilator" in { + iotesters.Driver.execute(Array("--backend-name", "verilator"), () => new SecondClock) { c => + new SecondClockTester(c) + } should be(true) + } + } +}