From 2e06898dc0216e69d9ecc4b44c93007d4b7b70b6 Mon Sep 17 00:00:00 2001 From: Hansung Kim Date: Mon, 13 Mar 2023 16:22:22 -0700 Subject: [PATCH] Handle enqueue and lookup at the same cycle This fixes the inflight table filling up to full after some time in the memtrace simulation. --- src/main/scala/tilelink/Coalescing.scala | 29 ++++++++---- src/test/scala/CoalescingUnitTest.scala | 60 ++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/src/main/scala/tilelink/Coalescing.scala b/src/main/scala/tilelink/Coalescing.scala index a226c0e..958443b 100644 --- a/src/main/scala/tilelink/Coalescing.scala +++ b/src/main/scala/tilelink/Coalescing.scala @@ -98,9 +98,9 @@ class CoalescingUnit(numLanes: Int = 1)(implicit p: Parameters) // Debug only val inflightCounter = RegInit(UInt(32.W), 0.U) - when (tlOut.a.valid) { + when(tlOut.a.valid) { // don't inc/dec on simultaneous req/resp - when (!tlOut.d.valid) { + when(!tlOut.d.valid) { inflightCounter := inflightCounter + 1.U } }.elsewhen(tlOut.d.valid) { @@ -231,6 +231,9 @@ class InflightCoalReqTable( .map { i => table(i).valid } .reduce { (v0, v1) => v0 && v1 } + val enqFire = io.enq.ready && io.enq.valid + val lookupFire = io.lookup.ready && io.lookup.valid + // Enqueue logic // // Instantiate simple cascade of muxes that indicate what is the current @@ -246,14 +249,8 @@ class InflightCoalReqTable( dontTouch(chosenEmptyIndex) dontTouch(full) - val enqFire = io.enq.ready && io.enq.valid - when(enqFire) { - val entry = table(chosenEmptyIndex) - entry.valid := true.B - entry.bits := io.enq.bits - } - io.enq.ready := !full + // actual write will happen down below // Currently, we assume coalescer never blocks generating coalesced requests. // If this ever happens, it means the table is insufficiently large to keep @@ -281,11 +278,23 @@ class InflightCoalReqTable( // TODO: return something actually useful io.lookup.bits := table(matchIndex).bits.respSourceId - val lookupFire = io.lookup.ready && io.lookup.valid when(lookupFire) { // As soon as a lookup returns a match, dequeue that entry table(matchIndex).valid := false.B } + + when(enqFire) { + // When we're enqueueing and looking up at the same time, we want to reuse + // the current matching entry for writing the new incoming entry + // to prevent having to write to two locations at the same cycle. + // TODO: This might or might not be an issue w.r.t. write ports of the + // registers, double-check + val indexToWrite = Mux(lookupFire, matchIndex, chosenEmptyIndex) + val entryToWrite = table(indexToWrite) + entryToWrite.valid := true.B + entryToWrite.bits := io.enq.bits + } + dontTouch(io.lookup) dontTouch(matchIndex) } diff --git a/src/test/scala/CoalescingUnitTest.scala b/src/test/scala/CoalescingUnitTest.scala index 3596e21..6417753 100644 --- a/src/test/scala/CoalescingUnitTest.scala +++ b/src/test/scala/CoalescingUnitTest.scala @@ -40,16 +40,19 @@ class CoalescingUnitTest extends AnyFlatSpec with ChiselScalatestTester { it should "stop enqueueing when full" in { test(new InflightCoalReqTable(numLanes, sourceWidth, entries)) { c => + // fill up the table for (i <- 0 until entries) { + val sourceId = i c.io.enq.ready.expect(true.B) c.io.enq.valid.poke(true.B) c.io.enq.bits.fromLane.poke(0.U) - c.io.enq.bits.respSourceId.poke(i.U) + c.io.enq.bits.respSourceId.poke(sourceId.U) c.io.enq.bits.reqSourceIds.foreach { id => id.poke(0.U) } - + c.io.lookup.ready.poke(false.B) c.clock.step() } + // now cannot enqueue any more c.io.enq.ready.expect(false.B) c.io.enq.valid.poke(true.B) c.io.enq.bits.fromLane.poke(0.U) @@ -58,11 +61,32 @@ class CoalescingUnitTest extends AnyFlatSpec with ChiselScalatestTester { c.clock.step() c.io.enq.ready.expect(false.B) + + // try to lookup all existing entries + for (i <- 0 until entries) { + val sourceId = i + c.io.enq.valid.poke(false.B) + c.io.lookup.ready.poke(true.B) + c.io.lookupSourceId.poke(sourceId) + c.io.lookup.valid.expect(true.B) + c.io.lookup.bits.expect(sourceId) + c.clock.step() + } + + // now the table should be empty + for (i <- 0 until entries) { + val sourceId = i + c.io.enq.valid.poke(false.B) + c.io.lookup.ready.poke(true.B) + c.io.lookupSourceId.poke(sourceId) + c.io.lookup.valid.expect(false.B) + c.clock.step() + } } } it should "lookup matching entry" in { test(new InflightCoalReqTable(numLanes, sourceWidth, entries)) - .withAnnotations(Seq(VcsBackendAnnotation, WriteFsdbAnnotation)) { c => + .withAnnotations(Seq(WriteVcdAnnotation)) { c => c.reset.poke(true.B) c.clock.step(10) c.reset.poke(false.B) @@ -98,4 +122,34 @@ class CoalescingUnitTest extends AnyFlatSpec with ChiselScalatestTester { c.io.lookup.valid.expect(false.B) } } + it should "handle lookup and enqueue at the same time" in { + test(new InflightCoalReqTable(numLanes, sourceWidth, entries)) { c => + // fill up the table + val targetSourceId = 1.U + c.io.enq.ready.expect(true.B) + c.io.enq.valid.poke(true.B) + c.io.enq.bits.fromLane.poke(0.U) + c.io.enq.bits.respSourceId.poke(0.U) + c.io.enq.bits.reqSourceIds.foreach { id => id.poke(0.U) } + c.clock.step() + c.io.enq.ready.expect(true.B) + c.io.enq.valid.poke(true.B) + c.io.enq.bits.fromLane.poke(0.U) + c.io.enq.bits.respSourceId.poke(targetSourceId) + c.io.enq.bits.reqSourceIds.foreach { id => id.poke(0.U) } + c.clock.step() + + // do both enqueue and lookup at the same cycle + val enqSourceId = 2.U + c.io.enq.ready.expect(true.B) + c.io.enq.valid.poke(true.B) + c.io.enq.bits.fromLane.poke(0.U) + c.io.enq.bits.respSourceId.poke(enqSourceId) + c.io.enq.bits.reqSourceIds.foreach { id => id.poke(0.U) } + c.io.lookup.ready.poke(true.B) + c.io.lookupSourceId.poke(targetSourceId) + + c.clock.step() + } + } }