From 6757ea1bbd0575644f0e4d399866365a20ef4df5 Mon Sep 17 00:00:00 2001 From: Richard Yan Date: Mon, 1 May 2023 00:51:31 -0700 Subject: [PATCH] shift queue bug fixes + new unit test --- src/main/scala/tilelink/Coalescing.scala | 22 ++- .../scala/coalescing/CoalescingUnitTest.scala | 184 ++++++++++-------- 2 files changed, 116 insertions(+), 90 deletions(-) diff --git a/src/main/scala/tilelink/Coalescing.scala b/src/main/scala/tilelink/Coalescing.scala index 4398bd8..5f0fc56 100644 --- a/src/main/scala/tilelink/Coalescing.scala +++ b/src/main/scala/tilelink/Coalescing.scala @@ -152,8 +152,10 @@ class ReqSourceGen(sourceWidth: Int) extends Module { class CoalShiftQueue[T <: Data](gen: T, entries: Int, config: CoalescerConfig) extends Module { val io = IO(new Bundle { - val enq = Vec(config.numLanes, DeqIO(gen.cloneType)) - val deq = Vec(config.numLanes, EnqIO(gen.cloneType)) + val queue = new Bundle { + val enq = Vec(config.numLanes, DeqIO(gen.cloneType)) + val deq = Vec(config.numLanes, EnqIO(gen.cloneType)) + } val invalidate = Input(Valid(Vec(config.numLanes, UInt(entries.W)))) val coalescable = Input(Vec(config.numLanes, Bool())) val mask = Output(Vec(config.numLanes, UInt(entries.W))) @@ -175,18 +177,18 @@ class CoalShiftQueue[T <: Data](gen: T, entries: Int, config: CoalescerConfig) e })) val shiftHint = !io.coalescable.reduce(_ || _) - val syncedEnqValid = io.enq.map(_.valid).reduce(_ || _) - val syncedDeqValid = io.deq.map(_.valid).reduce(_ || _) + val syncedEnqValid = io.queue.enq.map(_.valid).reduce(_ || _) + val syncedDeqValid = io.queue.deq.map(_.valid).reduce(_ || _) for (i <- 0 until config.numLanes) { - val enq = io.enq(i) - val deq = io.deq(i) + val enq = io.queue.enq(i) + val deq = io.queue.deq(i) val ctrl = controlSignals(i) ctrl.full := writePtr(i) === entries.U ctrl.empty := writePtr(i) === 0.U // shift when no outstanding dequeue, no more coalescable chunks, and not empty - ctrl.shift := syncedDeqValid && shiftHint && !ctrl.empty + ctrl.shift := !syncedDeqValid && shiftHint && !ctrl.empty // dequeue is valid when: // head entry is valid, has not been processed by downstream, and is not coalescable @@ -542,8 +544,8 @@ class CoalescingUnitImp(outer: CoalescingUnit, config: CoalescerConfig) extends // data, at the cost of re-aligning at the outgoing end. req.mask := tlIn.a.bits.mask - val enq = reqQueues.io.enq(lane) - val deq = reqQueues.io.deq(lane) + val enq = reqQueues.io.queue.enq(lane) + val deq = reqQueues.io.queue.deq(lane) enq.valid := tlIn.a.valid enq.bits := req deq.ready := true.B // TODO: deq.ready should respect downstream arbiter @@ -695,7 +697,7 @@ class CoalescingUnitImp(outer: CoalescingUnit, config: CoalescerConfig) extends s"tlCoal param `dataBits` (${tlCoal.params.dataBits}) mismatches coalescer constant" + s" (${(1 << config.dataBusWidth) * 8})" ) - val reqQueueHeads = reqQueues.io.deq.map(_.bits) + val reqQueueHeads = reqQueues.io.queue.deq.map(_.bits) // Do a 2-D copy from every (numLanes * queueDepth) invalidate output of the // coalescer to every (numLanes * queueDepth) entry in the inflight table. (newEntry.lanes zip coalescer.io.invalidate.bits).zipWithIndex diff --git a/src/test/scala/coalescing/CoalescingUnitTest.scala b/src/test/scala/coalescing/CoalescingUnitTest.scala index 9de8d41..bcd657e 100644 --- a/src/test/scala/coalescing/CoalescingUnitTest.scala +++ b/src/test/scala/coalescing/CoalescingUnitTest.scala @@ -213,58 +213,119 @@ class CoalescerUnitTest extends AnyFlatSpec with ChiselScalatestTester { it should "resort to the backup policy when coverage is below average" in {} } -/*class CoalShiftQueueTest extends AnyFlatSpec with ChiselScalatestTester { +class CoalShiftQueueTest extends AnyFlatSpec with ChiselScalatestTester { behavior of "request shift queues" it should "work like normal shiftqueue when no invalidate" in { - test(new CoalShiftQueue(UInt(8.W), 4)) { c => - c.io.queue.deq.ready.poke(false.B) - c.io.allowShift.poke(true.B) - c.io.queue.enq.ready.expect(true.B) - c.io.queue.enq.valid.poke(true.B) - c.io.queue.enq.bits.poke(0x12.U) +// new CoalShiftQueue(0.U,4, testConfig) + def attemptEnqueue(c: CoalShiftQueue[UInt], bits: Seq[UInt], valids: Seq[Bool]): Unit = { + ((c.io.queue.enq zip bits) zip valids).foreach { case ((enq, ent), valid) => + enq.ready.expect(true.B) + enq.valid.poke(valid) + enq.bits.poke(ent) + } c.clock.step() - c.io.queue.enq.ready.expect(true.B) - c.io.queue.enq.valid.poke(true.B) - c.io.queue.enq.bits.poke(0x34.U) - c.clock.step() - c.io.queue.enq.ready.expect(true.B) - c.io.queue.enq.valid.poke(true.B) - c.io.queue.enq.bits.poke(0x56.U) + } + + def expectDequeue(c: CoalShiftQueue[UInt], bits: Seq[UInt], valids: Seq[Bool]): Unit = { + ((c.io.queue.deq zip bits) zip valids).foreach { case ((deq, ent), valid) => + deq.valid.expect(valid) + deq.bits.expect(ent) + } + } + + def pokeVec[T <: Data](vec: Seq[T], value: Seq[T]): Unit = { + (vec zip value).foreach { case (a, b) => a.poke(b) } + } + + test(new CoalShiftQueue(UInt(8.W),4, testConfig)) { c => + c.io.coalescable.foreach(_.poke(true.B)) + c.io.queue.deq.foreach(_.ready.poke(false.B)) + + attemptEnqueue(c, Seq.fill(4)(1.U), Seq.fill(4)(true.B)) + attemptEnqueue(c, Seq.fill(4)(2.U), Seq(true.B, false.B, false.B, false.B)) // should remain synchronous + attemptEnqueue(c, Seq.fill(4)(3.U), Seq.fill(4)(true.B)) + + c.io.queue.enq.foreach(_.valid.poke(false.B)) + c.io.queue.enq.foreach(_.ready.expect(true.B)) + // check if head is the first enqueued item + expectDequeue(c, Seq.fill(4)(1.U), Seq.fill(4)(false.B)) c.clock.step() - c.io.queue.enq.valid.poke(false.B) - - c.io.queue.deq.ready.poke(true.B) - c.io.queue.deq.valid.expect(true.B) - c.io.queue.deq.bits.expect(0x12.U) - c.clock.step() - c.io.queue.deq.ready.poke(true.B) - c.io.queue.deq.valid.expect(true.B) - c.io.queue.deq.bits.expect(0x34.U) - c.clock.step() - // enqueue in the middle - c.io.queue.deq.ready.poke(false.B) - c.io.queue.enq.ready.expect(true.B) - c.io.queue.enq.valid.poke(true.B) - c.io.queue.enq.bits.poke(0x78.U) - c.clock.step() - c.io.queue.enq.valid.poke(false.B) - c.io.queue.deq.ready.poke(true.B) - c.io.queue.deq.valid.expect(true.B) - c.io.queue.deq.bits.expect(0x56.U) - c.clock.step() - c.io.queue.deq.ready.poke(true.B) - c.io.queue.deq.valid.expect(true.B) - c.io.queue.deq.bits.expect(0x78.U) + c.io.queue.deq.foreach(_.ready.poke(true.B)) + // should not dequeue because all are coalescable + expectDequeue(c, Seq.fill(4)(1.U), Seq.fill(4)(false.B)) c.clock.step() - // should be emptied - c.io.queue.deq.valid.expect(false.B) + pokeVec(c.io.coalescable, Seq(false.B, false.B, false.B, true.B)) + // first 3 items should be valid now + expectDequeue(c, Seq.fill(4)(1.U), Seq(true.B, true.B, true.B, false.B)) + // only dequeue first item - 4th item should not be dequeued since not valid + pokeVec(c.io.queue.deq.map(_.ready), Seq(true.B, false.B, false.B, true.B)) + c.clock.step() + + // first item should turn invalid + c.io.coalescable.foreach(_.poke(false.B)) + expectDequeue(c, Seq.fill(4)(1.U), Seq(false.B, true.B, true.B, true.B)) + // now dequeue everything else in the first line + c.io.queue.deq.foreach(_.ready.poke(true.B)) + c.clock.step() + + // all dequeued, none valid this cycle + expectDequeue(c, Seq.fill(4)(1.U), Seq.fill(4)(false.B)) + c.clock.step() + + // shifted last cycle + c.io.coalescable.foreach(_.poke(false.B)) + c.io.queue.deq.foreach(_.ready.poke(true.B)) + expectDequeue(c, Seq.fill(4)(2.U), Seq(true.B, false.B, false.B, false.B)) + c.clock.step() + + expectDequeue(c, Seq.fill(4)(2.U), Seq(false.B, false.B, false.B, false.B)) + c.clock.step() + + pokeVec(c.io.coalescable, Seq(true.B, false.B, true.B, true.B)) + expectDequeue(c, Seq.fill(4)(3.U), Seq(false.B, true.B, false.B, false.B)) + c.clock.step() + + c.io.coalescable.foreach(_.poke(false.B)) + expectDequeue(c, Seq.fill(4)(3.U), Seq(true.B, false.B, true.B, true.B)) + c.clock.step() + + // empty + expectDequeue(c, Seq.fill(4)(3.U), Seq.fill(4)(false.B)) + + // now enqueue back to full & test back pressure + c.io.queue.deq.foreach(_.ready.poke(false.B)) + attemptEnqueue(c, Seq.fill(4)(1.U), Seq.fill(4)(true.B)) + pokeVec(c.io.coalescable, Seq(true.B, true.B, true.B, true.B)) + attemptEnqueue(c, Seq.fill(4)(2.U), Seq.fill(4)(true.B)) + attemptEnqueue(c, Seq.fill(4)(3.U), Seq.fill(4)(true.B)) + attemptEnqueue(c, Seq.fill(4)(4.U), Seq.fill(4)(true.B)) + + // check full + c.io.queue.enq.foreach(_.ready.expect(false.B)) + c.clock.step() + + // now indicate the next cycle will dequeue everything + c.io.queue.deq.foreach(_.ready.poke(true.B)) + c.io.coalescable.foreach(_.poke(false.B)) + c.clock.step() + + // should still be full, but allow enqueue + c.io.coalescable.foreach(_.poke(true.B)) + c.io.queue.enq.foreach(_.ready.expect(false.B)) // check full + c.io.coalescable.foreach(_.poke(false.B)) + attemptEnqueue(c, Seq.fill(4)(5.U), Seq.fill(4)(true.B)) + + expectDequeue(c, Seq.fill(4)(2.U), Seq.fill(4)(true.B)) + c.clock.step() + +// attemptEnqueue(c, Seq.fill(4)(6.U), Seq.fill(4)(true.B)) } } - +/* it should "work when enqueing and dequeueing simultaneously" in { test(new CoalShiftQueue(UInt(8.W), 4)) { c => c.io.invalidate.valid.poke(false.B) @@ -297,43 +358,6 @@ class CoalescerUnitTest extends AnyFlatSpec with ChiselScalatestTester { } } - it should "not shift entries when allowShift is false" in { - test(new CoalShiftQueue(UInt(8.W), 4)) { c => - c.io.invalidate.valid.poke(false.B) - c.io.queue.deq.ready.poke(false.B) - - c.io.allowShift.poke(false.B) - - // prepare - c.io.queue.enq.ready.expect(true.B) - c.io.queue.enq.valid.poke(true.B) - c.io.queue.enq.bits.poke(0x12.U) - c.clock.step() - c.io.queue.enq.ready.expect(true.B) - c.io.queue.enq.valid.poke(true.B) - c.io.queue.enq.bits.poke(0x34.U) - c.clock.step() - c.io.queue.enq.valid.poke(false.B) - - // dequeueing should work normally when allowShift is false... - c.io.queue.deq.ready.poke(true.B) - c.io.queue.deq.valid.expect(true.B) - c.io.queue.deq.bits.expect(0x12.U) - c.clock.step() - // but should stop there and not dequeue the next entry - c.io.queue.deq.ready.poke(true.B) - c.io.queue.deq.valid.expect(false.B) - c.clock.step() - // when allowShift is back one, dequeueing should start working from next - // cycle - c.io.allowShift.poke(true.B) - c.clock.step() - c.io.queue.deq.ready.poke(true.B) - c.io.queue.deq.valid.expect(true.B) - c.io.queue.deq.bits.expect(0x34.U) - } - } - it should "work when enqueing and dequeueing simultaneously to a depth=1 queue" in { test(new CoalShiftQueue(UInt(8.W), 1)) { c => c.io.invalidate.valid.poke(false.B) @@ -526,8 +550,8 @@ class CoalescerUnitTest extends AnyFlatSpec with ChiselScalatestTester { c.io.queue.deq.valid.expect(true.B) c.io.queue.deq.bits.expect(0x34) } - } -}*/ + }*/ +} object uncoalescerTestConfig extends CoalescerConfig( numLanes = 4,