From c313e137d4e562ef20195312501840ceab8cbc6a Mon Sep 17 00:00:00 2001 From: Adam Izraelevitz Date: Thu, 26 Oct 2017 12:39:42 -0700 Subject: Invalidateapi (#645) * Require explicit connection to DontCare to generate "is invalid". * Add tests for RefNotInitializedException. Currently, we fail the when ... otherwise ... * Disable ScalaTest shrinking on error in ComplexAssignSpec. * fix broken merge; still some binding issues * cleanup DontCare connection checks; add missing directions to test module IOs * Have library code inherit compileOptions from the enclosing Module (if it exists). * work around current firrtl uninitialized references with Strict compile options and explicitInvalidate * more CompileOptions cleanup; move test-specific defines to package object * minimize differences with master * set default CompileOptions.explicitInvalidate to false until we fix the FIRRTL when issue * ignore the StrictCompiler property checks (until CompileOptions.explicitInvalidate is defaulted to true) * Revert "more CompileOptions cleanup; move test-specific defines to package object" This reverts commit e4486edcba990d150e76e08a2fc6abca033556e0. * Revert "work around current firrtl uninitialized references with Strict compile options and explicitInvalidate" This reverts commit 426faa430a62c3dac2dbdf33044d3386d4243157. * remove unused code * Convert to binding-based DontCare implementation * comment cleanup to minimize differences with master * Tentatively remove possibly redundant DefInvalid on module ports. * Respond to code review change request. - backout build.sbt change - correct indentation - handle bulk of DontCare semantics in elemConnect() - have DontCare extend Element, not Data (eliminate most Object specific methods - add comments indicating reason for explicit DontCare connections * Initialize test elements without requiring a DontCare. * Respond to review change requests. - DontCare should work on left or right side in BiDirectional connections - call bind() to set DontCare binding instead of messing with internal variables - DontCares are only equivalent with DontCares - clean up processWhens() definition * Eliminate DontCare connection to inputs in MonoConnect(). * Pull aggregates apart for the purpose of DontCare connections. * Restore the explicit (conditionally executed) ports DefInvalidin ImplicitModule() * Don't add DontCare's to the module list of _ids. * Add missing DefInvalid() to LegacyModule(). * Respond to review requests: add DontCare BiConnect Vec, remove null parent hack to avoid addId(), initialize singletons early in Builder * Move DontCare out of chisel3.experimental. --- src/test/scala/chiselTests/ComplexAssign.scala | 4 + src/test/scala/chiselTests/DontTouchSpec.scala | 1 + src/test/scala/chiselTests/FixedPointSpec.scala | 2 + src/test/scala/chiselTests/InvalidateAPISpec.scala | 164 +++++++++++++++++++++ src/test/scala/chiselTests/Vec.scala | 3 + src/test/scala/chiselTests/VectorPacketIO.scala | 5 + src/test/scala/chiselTests/When.scala | 3 +- 7 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 src/test/scala/chiselTests/InvalidateAPISpec.scala (limited to 'src/test/scala/chiselTests') diff --git a/src/test/scala/chiselTests/ComplexAssign.scala b/src/test/scala/chiselTests/ComplexAssign.scala index a13ec959..e101e7c0 100644 --- a/src/test/scala/chiselTests/ComplexAssign.scala +++ b/src/test/scala/chiselTests/ComplexAssign.scala @@ -8,6 +8,7 @@ import org.scalatest.prop._ import chisel3._ import chisel3.testers.BasicTester import chisel3.util._ +import org.scalacheck.Shrink class Complex[T <: Data](val re: T, val im: T) extends Bundle { override def cloneType: this.type = @@ -47,6 +48,9 @@ class ComplexAssignTester(enList: List[Boolean], re: Int, im: Int) extends Basic class ComplexAssignSpec extends ChiselPropSpec { property("All complex assignments should return the correct result") { + // Disable shrinking on error. + implicit val noShrinkListVal = Shrink[List[Boolean]](_ => Stream.empty) + implicit val noShrinkInt = Shrink[Int](_ => Stream.empty) forAll(enSequence(2), safeUInts, safeUInts) { (en: List[Boolean], re: Int, im: Int) => assertTesterPasses{ new ComplexAssignTester(en, re, im) } } diff --git a/src/test/scala/chiselTests/DontTouchSpec.scala b/src/test/scala/chiselTests/DontTouchSpec.scala index 9a4e6660..7aa4d2e2 100644 --- a/src/test/scala/chiselTests/DontTouchSpec.scala +++ b/src/test/scala/chiselTests/DontTouchSpec.scala @@ -13,6 +13,7 @@ class HasDeadCodeChild(withDontTouch: Boolean) extends Module { val c = Output(Vec(2, UInt(32.W))) }) io.b := io.a + io.c := DontCare if (withDontTouch) { dontTouch(io.c) } diff --git a/src/test/scala/chiselTests/FixedPointSpec.scala b/src/test/scala/chiselTests/FixedPointSpec.scala index 28c3aa55..8caa7f1e 100644 --- a/src/test/scala/chiselTests/FixedPointSpec.scala +++ b/src/test/scala/chiselTests/FixedPointSpec.scala @@ -108,6 +108,8 @@ class SBPTester extends BasicTester { assert(dut.io.out === 3.0.F(0.BP)) val test = Wire(FixedPoint(10.W, 5.BP)) + // Initialize test, avoiding a "Reference test is not fully initialized" error from firrtl. + test := 0.0.F(5.BP) val q = test.setBinaryPoint(18) assert(q.getWidth.U === 23.U) diff --git a/src/test/scala/chiselTests/InvalidateAPISpec.scala b/src/test/scala/chiselTests/InvalidateAPISpec.scala new file mode 100644 index 00000000..7176c654 --- /dev/null +++ b/src/test/scala/chiselTests/InvalidateAPISpec.scala @@ -0,0 +1,164 @@ +// See LICENSE for license details. + +package chiselTests + +import chisel3._ +import chisel3.core.BiConnect.BiConnectException +import chisel3.util.Counter +import firrtl.passes.CheckInitialization.RefNotInitializedException +import org.scalatest._ + +class InvalidateAPISpec extends ChiselPropSpec with Matchers { + + def myGenerateFirrtl(t: => Module): String = Driver.emit(() => t) + def compileFirrtl(t: => Module): Unit = { + Driver.execute(Array[String]("--compiler", "verilog"), () => t) + } + class TrivialInterface extends Bundle { + val in = Input(Bool()) + val out = Output(Bool()) + } + + property("an output connected to DontCare should emit a Firrtl \"is invalid\" with Strict CompileOptions") { + import chisel3.core.ExplicitCompileOptions.Strict + class ModuleWithDontCare extends Module { + val io = IO(new TrivialInterface) + io.out := DontCare + io.out := io.in + } + val firrtlOutput = myGenerateFirrtl(new ModuleWithDontCare) + firrtlOutput should include("io.out is invalid") + } + + property("an output without a DontCare should NOT emit a Firrtl \"is invalid\" with Strict CompileOptions") { + import chisel3.core.ExplicitCompileOptions.Strict + class ModuleWithoutDontCare extends Module { + val io = IO(new TrivialInterface) + io.out := io.in + } + val firrtlOutput = myGenerateFirrtl(new ModuleWithoutDontCare) + firrtlOutput should not include("is invalid") + } + + property("an output without a DontCare should emit a Firrtl \"is invalid\" with NotStrict CompileOptions") { + import chisel3.core.ExplicitCompileOptions.NotStrict + class ModuleWithoutDontCare extends Module { + val io = IO(new TrivialInterface) + io.out := io.in + } + val firrtlOutput = myGenerateFirrtl(new ModuleWithoutDontCare) + firrtlOutput should include("io is invalid") + } + + property("a bundle with a DontCare should emit a Firrtl \"is invalid\" with Strict CompileOptions") { + import chisel3.core.ExplicitCompileOptions.Strict + class ModuleWithoutDontCare extends Module { + val io = IO(new TrivialInterface) + io <> DontCare + } + val firrtlOutput = myGenerateFirrtl(new ModuleWithoutDontCare) + firrtlOutput should include("io.out is invalid") + firrtlOutput should include("io.in is invalid") + } + + property("a Vec with a DontCare should emit a Firrtl \"is invalid\" with Strict CompileOptions and bulk connect") { + import chisel3.core.ExplicitCompileOptions.Strict + val nElements = 5 + class ModuleWithoutDontCare extends Module { + val io = IO(new Bundle { + val ins = Input(Vec(nElements, Bool())) + }) + io.ins <> DontCare + } + val firrtlOutput = myGenerateFirrtl(new ModuleWithoutDontCare) + for (i <- 0 until nElements) + firrtlOutput should include(s"io.ins[$i] is invalid") + } + + property("a Vec with a DontCare should emit a Firrtl \"is invalid\" with Strict CompileOptions and mono connect") { + import chisel3.core.ExplicitCompileOptions.Strict + val nElements = 5 + class ModuleWithoutDontCare extends Module { + val io = IO(new Bundle { + val ins = Input(Vec(nElements, Bool())) + }) + io.ins := DontCare + } + val firrtlOutput = myGenerateFirrtl(new ModuleWithoutDontCare) + for (i <- 0 until nElements) + firrtlOutput should include(s"io.ins[$i] is invalid") + } + + property("a DontCare cannot be a connection sink (LHS) for := ") { + import chisel3.core.ExplicitCompileOptions.Strict + class ModuleWithDontCareSink extends Module { + val io = IO(new TrivialInterface) + DontCare := io.in + } + val exception = intercept[ChiselException] { + elaborate(new ModuleWithDontCareSink) + } + exception.getMessage should include("DontCare cannot be a connection sink (LHS)") + } + + property("a DontCare cannot be a connection sink (LHS) for <>") { + import chisel3.core.ExplicitCompileOptions.Strict + class ModuleWithDontCareSink extends Module { + val io = IO(new TrivialInterface) + DontCare <> io.in + } + val exception = intercept[BiConnectException] { + elaborate(new ModuleWithDontCareSink) + } + exception.getMessage should include("DontCare cannot be a connection sink (LHS)") + } + + property("FIRRTL should complain about partial initialization with Strict CompileOptions and conditional connect") { + import chisel3.core.ExplicitCompileOptions.Strict + class ModuleWithIncompleteAssignment extends Module { + val io = IO(new Bundle { + val out = Output(Bool()) + }) + val counter = Counter(8) + when (counter.inc()) { + io.out := true.B + } + } + val exception = intercept[RefNotInitializedException] { + compileFirrtl(new ModuleWithIncompleteAssignment) + } + exception.getMessage should include("is not fully initialized") + } + + property("FIRRTL should not complain about partial initialization with Strict CompileOptions and conditional connect after unconditional connect") { + import chisel3.core.ExplicitCompileOptions.Strict + class ModuleWithUnconditionalAssignment extends Module { + val io = IO(new Bundle { + val out = Output(Bool()) + }) + val counter = Counter(8) + io.out := false.B + when (counter.inc()) { + io.out := true.B + } + } + compileFirrtl(new ModuleWithUnconditionalAssignment) + } + + property("FIRRTL should not complain about partial initialization with Strict CompileOptions and conditional connect with otherwise clause") { + import chisel3.core.ExplicitCompileOptions.Strict + class ModuleWithConditionalAndOtherwiseAssignment extends Module { + val io = IO(new Bundle { + val out = Output(Bool()) + }) + val counter = Counter(8) + when (counter.inc()) { + io.out := true.B + } otherwise { + io.out := false.B + } + } + + compileFirrtl(new ModuleWithConditionalAndOtherwiseAssignment) + } +} diff --git a/src/test/scala/chiselTests/Vec.scala b/src/test/scala/chiselTests/Vec.scala index 6c62ab26..1c5157b5 100644 --- a/src/test/scala/chiselTests/Vec.scala +++ b/src/test/scala/chiselTests/Vec.scala @@ -146,6 +146,7 @@ class ZeroEntryVecTester extends BasicTester { val m = Module(new Module { val io = IO(Output(bundleWithZeroEntryVec)) + io.foo := false.B }) WireInit(m.io.bar) @@ -179,6 +180,8 @@ class ModuleIODynamicIndexTester(n: Int) extends BasicTester { when (cycle =/= i.U) { m.in := 0.U // default assert(m.out === 0.U) + } .otherwise { + m.in := DontCare } } // only connect one dut per cycle diff --git a/src/test/scala/chiselTests/VectorPacketIO.scala b/src/test/scala/chiselTests/VectorPacketIO.scala index bcf59e03..ba3664a3 100644 --- a/src/test/scala/chiselTests/VectorPacketIO.scala +++ b/src/test/scala/chiselTests/VectorPacketIO.scala @@ -40,6 +40,11 @@ class BrokenVectorPacketModule extends Module { val n = 4 val io = IO(new VectorPacketIO(n)) + // Avoid a "Reference io is not fully initialized" error from firrtl. + for (i <- 0 until n) { + io.outs(i) <> io.ins(i) + } + /* the following method of initializing the circuit may change in the future */ io.ins.foreach(_.nodeq()) io.outs.foreach(_.noenq()) diff --git a/src/test/scala/chiselTests/When.scala b/src/test/scala/chiselTests/When.scala index d2646df7..3bd63831 100644 --- a/src/test/scala/chiselTests/When.scala +++ b/src/test/scala/chiselTests/When.scala @@ -7,7 +7,6 @@ import org.scalatest._ import chisel3._ import chisel3.testers.BasicTester import chisel3.util._ -//import chisel3.core.ExplicitCompileOptions.Strict class WhenTester() extends BasicTester { val cnt = Counter(4) @@ -66,6 +65,8 @@ class NoOtherwiseOverlappedWhenTester() extends BasicTester { out := 3.U } .elsewhen (cnt.value <= 3.U) { out := 0.U + } .otherwise { + out := DontCare } assert(out === cnt.value + 1.U) -- cgit v1.2.3