From 79604f51beb0cc535546c45990f98f6976bf7d51 Mon Sep 17 00:00:00 2001
From: Hansung Kim <hansung_kim@berkeley.edu>
Date: Tue, 23 Jul 2024 15:09:03 -0700
Subject: [PATCH] Fix possible CIRCT bug on SourceGenerator

When migrated to amd3 (possibly wiht newer CIRCT version), a new bug
shows up where storing both meta and valid into a single table doesn't
work, since writing meta writes {1'b0, meta} to the whole row of the
table overwriting the valid bit.  Work this around by creating separate
tables for the meta and valid bits.

While at it, remove use of outdated NewSourceGenerator in VortexBank.
---
 .../scala/radiance/memory/Coalescing.scala    | 25 +++++++++++--------
 .../scala/radiance/memory/VortexCache.scala   |  9 ++++---
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/main/scala/radiance/memory/Coalescing.scala b/src/main/scala/radiance/memory/Coalescing.scala
index 1823c9f..cac5e95 100644
--- a/src/main/scala/radiance/memory/Coalescing.scala
+++ b/src/main/scala/radiance/memory/Coalescing.scala
@@ -332,24 +332,27 @@ class SourceGenerator[T <: Data](
     val meta = getMetadataType
     val valid = Bool()
   }
-  // valid: in use, invalid: available
-  // val occupancyTable = Mem(numSourceId, Valid(UInt(sourceWidth.W)))
-  val occupancyTable = Mem(numSourceId, row)
+  val occupancyTable = Mem(numSourceId, Bool()/* true: in use, false: free */)
+  // Due to a potential chisel/CIRCT bug, storing both meta and valid in a
+  // single table doesn't work; writing meta writes {1'b0, meta} to the whole
+  // row of the table, overwriting the valid bit.  Workaround by creating
+  // separate tables for meta and valid.
+  val metadataTable = Mem(numSourceId, getMetadataType)
   when(reset.asBool) {
-    (0 until numSourceId).foreach { occupancyTable(_).valid := false.B }
+    (0 until numSourceId).foreach { occupancyTable(_) := false.B }
   }
-  val frees = (0 until numSourceId).map(!occupancyTable(_).valid)
+  val frees = (0 until numSourceId).map(!occupancyTable(_))
   val lowestFree = PriorityEncoder(frees)
-  val lowestFreeRow = occupancyTable(lowestFree)
+  val lowestFreeValid = occupancyTable(lowestFree)
 
-  io.id.valid := (if (ignoreInUse) true.B else !lowestFreeRow.valid)
+  io.id.valid := (if (ignoreInUse) true.B else !lowestFreeValid)
   io.id.bits := lowestFree
   when(io.gen && io.id.valid /* fire */ ) {
     // handle reclaim at the same cycle, e.g. for 0-latency D channel response
     when (!io.reclaim.valid || io.reclaim.bits =/= io.id.bits) {
-      occupancyTable(io.id.bits).valid := true.B // mark in use
+      occupancyTable(io.id.bits) := true.B // mark in use
       if (metadata.isDefined) {
-        occupancyTable(io.id.bits).meta := io.meta
+        metadataTable(io.id.bits) := io.meta
       }
     }
   }
@@ -357,10 +360,10 @@ class SourceGenerator[T <: Data](
     // @perf: would this require multiple write ports?
     // NOTE: this does not seem sufficient to handle same-cycle gen-reclaim on
     // its own
-    occupancyTable(io.reclaim.bits).valid := false.B // mark freed
+    occupancyTable(io.reclaim.bits) := false.B // mark freed
   }
   io.peek := {
-    if (metadata.isDefined) occupancyTable(io.reclaim.bits).meta else 0.U
+    if (metadata.isDefined) metadataTable(io.reclaim.bits) else 0.U
   }
 
   when(io.gen && io.id.valid) {
diff --git a/src/main/scala/radiance/memory/VortexCache.scala b/src/main/scala/radiance/memory/VortexCache.scala
index b592afd..2e23186 100644
--- a/src/main/scala/radiance/memory/VortexCache.scala
+++ b/src/main/scala/radiance/memory/VortexCache.scala
@@ -124,8 +124,8 @@ class VortexBankPassThrough(config: VortexL1Config)(implicit p: Parameters)
 
     // Make sure the outgoing edge of this passthrough has enough sourceIds
     // that encompasses the core-side incoming edge's.  This is an unfortunate
-    // hack due to not doing proper param negotiations across disconnected
-    // Diplomacy graphs.
+    // hack due to incomplete param negotiations across disconnected Diplomacy
+    // graphs.
     // println(s"${upstream.params.sourceBits} <= ${downstream.params.sourceBits}")
     require(upstream.params.sourceBits <= downstream.params.sourceBits,
             "mem-side source of L1 cache truncates core-side source! " +
@@ -344,7 +344,7 @@ class VortexBankImp(
   // its MSHR and therefore doesn't allocate a new tag id for write requests.
   // We use a separate source ID allocator to solve this.
   val sourceGen = Module(
-    new NewSourceGenerator(
+    new SourceGenerator(
       log2Ceil(config.memSideSourceIds),
       metadata = Some(UInt(32.W)),
       ignoreInUse = false
@@ -556,7 +556,8 @@ class NewSourceGenerator[T <: Data](
   when(reset.asBool) {
     (0 until numSourceId).foreach { i =>
       occupancyTable(i).id.valid := false.B
-      occupancyTable(i).age := 0.U // Reset age during reset
+      occupancyTable(i).meta := 0.U
+      occupancyTable(i).age := 0.U
     }
   }
   val frees = (0 until numSourceId).map(!occupancyTable(_).id.valid)