From 5d278605f2f398b17e7059a70ccd7420aa555cf8 Mon Sep 17 00:00:00 2001 From: chick Date: Wed, 24 Feb 2016 22:52:50 -0800 Subject: Create a test that breaks because of assignment statements in DeqIO and EnqIO bundles --- src/test/scala/chiselTests/VectorPacketIO.scala | 52 +++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 src/test/scala/chiselTests/VectorPacketIO.scala (limited to 'src') diff --git a/src/test/scala/chiselTests/VectorPacketIO.scala b/src/test/scala/chiselTests/VectorPacketIO.scala new file mode 100644 index 00000000..8f68532e --- /dev/null +++ b/src/test/scala/chiselTests/VectorPacketIO.scala @@ -0,0 +1,52 @@ +// See LICENSE for license details. + +package chiselTests + +import Chisel._ +import Chisel.testers.BasicTester + +/** + * This test illustrates the creation of a firrtl file + * with missing declarations, the problem is exposed by + * the creation of the val outs in VectorPacketIO + * NOTE: The problem does not exists if the initialization + * code is removed from DeqIO and EnqIO + * see: Decoupled.scala lines 29 and 38 + * valid := Bool(false) and ready := Bool(false) + * statements inside a bundle + */ +class Packet extends Bundle { + val header = UInt(width = 1) +} + +/** + * The problem occurs with just the ins or the outs + * lines also. + * The problem does not occur if the Vec is taken out + */ +class VectorPacketIO(n: Int) extends Bundle { + val ins = Vec(n, new DeqIO(new Packet())) + val outs = Vec(n, new EnqIO(new Packet())) +} + +/** + * a module uses the vector based IO bundle + * the value of n does not affect the error + */ +class BrokenVectorPacketModule extends Module { + val n = 4 + val io = new VectorPacketIO(n) +} + +class VectorPacketIOUnitTester extends BasicTester { + val device_under_test = Module(new BrokenVectorPacketModule) +} + +class VectorPacketIOUnitTesterSpec extends ChiselFlatSpec { + "a circuit using an io containing a vector of EnqIO wrapped packets" should + "compile and run" in { + assertTesterPasses { + new VectorPacketIOUnitTester + } + } +} -- cgit v1.2.3 From 3c0a67889280803c22fff441462d06bb5081a558 Mon Sep 17 00:00:00 2001 From: chick Date: Wed, 24 Feb 2016 23:05:11 -0800 Subject: Remove the assignment statements in EnqIO and DeqIO Bundle constructors. Make the corresponding test run faster by giving it a Counter. --- src/main/scala/Chisel/util/Decoupled.scala | 22 +++++++++++++++------- src/test/scala/chiselTests/VectorPacketIO.scala | 17 +++++++++++------ 2 files changed, 26 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/main/scala/Chisel/util/Decoupled.scala b/src/main/scala/Chisel/util/Decoupled.scala index 23a08d52..e06e899a 100644 --- a/src/main/scala/Chisel/util/Decoupled.scala +++ b/src/main/scala/Chisel/util/Decoupled.scala @@ -22,21 +22,29 @@ object Decoupled { def apply[T <: Data](gen: T): DecoupledIO[T] = new DecoupledIO(gen) } -/** An I/O bundle for enqueuing data with valid/ready handshaking */ +/** An I/O bundle for enqueuing data with valid/ready handshaking + * initialization must be handled, if necessary, by the parent circuit + */ class EnqIO[T <: Data](gen: T) extends DecoupledIO(gen) { def enq(dat: T): T = { valid := Bool(true); bits := dat; dat } - valid := Bool(false) - for (io <- bits.flatten) - io := UInt(0) + def init(): Unit = { + valid := Bool(false) + for (io <- bits.flatten) + io := UInt(0) + } override def cloneType: this.type = { new EnqIO(gen).asInstanceOf[this.type]; } } -/** An I/O bundle for dequeuing data with valid/ready handshaking */ +/** An I/O bundle for dequeuing data with valid/ready handshaking + * initialization must be handled, if necessary, by the parent circuit + */ class DeqIO[T <: Data](gen: T) extends DecoupledIO(gen) with Flipped { - ready := Bool(false) def deq(b: Boolean = false): T = { ready := Bool(true); bits } + def init(): Unit = { + ready := Bool(false) + } override def cloneType: this.type = { new DeqIO(gen).asInstanceOf[this.type]; } } @@ -54,7 +62,7 @@ class DecoupledIOC[+T <: Data](gen: T) extends Bundle class QueueIO[T <: Data](gen: T, entries: Int) extends Bundle { /** I/O to enqueue data, is [[Chisel.DecoupledIO]] flipped */ - val enq = Decoupled(gen.cloneType).flip + val enq = Decoupled(gen.cloneType).flip() /** I/O to enqueue data, is [[Chisel.DecoupledIO]]*/ val deq = Decoupled(gen.cloneType) /** The current amount of data in the queue */ diff --git a/src/test/scala/chiselTests/VectorPacketIO.scala b/src/test/scala/chiselTests/VectorPacketIO.scala index 8f68532e..5fff6236 100644 --- a/src/test/scala/chiselTests/VectorPacketIO.scala +++ b/src/test/scala/chiselTests/VectorPacketIO.scala @@ -6,14 +6,13 @@ import Chisel._ import Chisel.testers.BasicTester /** - * This test illustrates the creation of a firrtl file + * This test used to fail when assignment statements were + * contained in DeqIO and EnqIO constructors. + * The symptom is creation of a firrtl file * with missing declarations, the problem is exposed by * the creation of the val outs in VectorPacketIO - * NOTE: The problem does not exists if the initialization - * code is removed from DeqIO and EnqIO - * see: Decoupled.scala lines 29 and 38 - * valid := Bool(false) and ready := Bool(false) - * statements inside a bundle + * NOTE: The problem does not exist now because the initialization + * code has been removed from DeqIO and EnqIO */ class Packet extends Bundle { val header = UInt(width = 1) @@ -40,6 +39,12 @@ class BrokenVectorPacketModule extends Module { class VectorPacketIOUnitTester extends BasicTester { val device_under_test = Module(new BrokenVectorPacketModule) + + // This counter just makes the test end quicker + val c = Counter(1) + when(c.inc()) { + stop() + } } class VectorPacketIOUnitTesterSpec extends ChiselFlatSpec { -- cgit v1.2.3 From 2e4d7869400f121bdae692f5c5b7976b1cb58438 Mon Sep 17 00:00:00 2001 From: chick Date: Thu, 25 Feb 2016 11:03:15 -0800 Subject: Fixed comment punctuation and made it clearer that using an init() method for DeqIO and EnqIO initialization is likely to change. --- src/main/scala/Chisel/util/Decoupled.scala | 25 ++++++++++++++++++++++--- src/test/scala/chiselTests/VectorPacketIO.scala | 6 ++++++ 2 files changed, 28 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/main/scala/Chisel/util/Decoupled.scala b/src/main/scala/Chisel/util/Decoupled.scala index e06e899a..ba33e6c7 100644 --- a/src/main/scala/Chisel/util/Decoupled.scala +++ b/src/main/scala/Chisel/util/Decoupled.scala @@ -23,11 +23,20 @@ object Decoupled { } /** An I/O bundle for enqueuing data with valid/ready handshaking - * initialization must be handled, if necessary, by the parent circuit + * Initialization must be handled, if necessary, by the parent circuit */ class EnqIO[T <: Data](gen: T) extends DecoupledIO(gen) { + /** push dat onto the output bits of this interface to let the consumer know it has happened. + * @param dat the values to assign to bits. + * @return dat. + */ def enq(dat: T): T = { valid := Bool(true); bits := dat; dat } + + /** Initialize this Bundle. Valid is set to false, and all bits are set to zero. + * NOTE: This method of initialization is still being discussed and could change in the + * future. + */ def init(): Unit = { valid := Bool(false) for (io <- bits.flatten) @@ -36,12 +45,22 @@ class EnqIO[T <: Data](gen: T) extends DecoupledIO(gen) override def cloneType: this.type = { new EnqIO(gen).asInstanceOf[this.type]; } } -/** An I/O bundle for dequeuing data with valid/ready handshaking - * initialization must be handled, if necessary, by the parent circuit +/** An I/O bundle for dequeuing data with valid/ready handshaking. + * Initialization must be handled, if necessary, by the parent circuit */ class DeqIO[T <: Data](gen: T) extends DecoupledIO(gen) with Flipped { + /** Assert ready on this port and return the associated data bits. + * This is typically used when valid has been asserted by the producer side. + * @param b ignored + * @return the data for this device, + */ def deq(b: Boolean = false): T = { ready := Bool(true); bits } + + /** Initialize this Bundle. + * NOTE: This method of initialization is still being discussed and could change in the + * future. + */ def init(): Unit = { ready := Bool(false) } diff --git a/src/test/scala/chiselTests/VectorPacketIO.scala b/src/test/scala/chiselTests/VectorPacketIO.scala index 5fff6236..99ec66a6 100644 --- a/src/test/scala/chiselTests/VectorPacketIO.scala +++ b/src/test/scala/chiselTests/VectorPacketIO.scala @@ -11,8 +11,11 @@ import Chisel.testers.BasicTester * The symptom is creation of a firrtl file * with missing declarations, the problem is exposed by * the creation of the val outs in VectorPacketIO + * * NOTE: The problem does not exist now because the initialization * code has been removed from DeqIO and EnqIO + * + * IMPORTANT: The canonical way to initialize a decoupled inteface is still being debated. */ class Packet extends Bundle { val header = UInt(width = 1) @@ -35,6 +38,9 @@ class VectorPacketIO(n: Int) extends Bundle { class BrokenVectorPacketModule extends Module { val n = 4 val io = new VectorPacketIO(n) + + /* the following method of initializing the circuit may change in the future */ + io.outs.foreach(_.init()) } class VectorPacketIOUnitTester extends BasicTester { -- cgit v1.2.3