collector: Fix per-reg dequeueing of collBanks

Author: hansungk

src/main/scala/radiance/muon/Collector.scala

@@ -52,10 +52,12 @@ object CollectorResponse {
 class CollectorOperandRead(implicit p: Parameters) extends CoreBundle()(p) {
   val collEntryWidth = log2Up(muonParams.numCollectorEntries)
   val hasPReg = !muonParams.useCollector
-  val req = Flipped(Valid(new Bundle {
+  val req = Flipped(Valid(Vec(Isa.maxNumRegs, new Bundle {
+    val enable = Bool()
     val collEntry = UInt(collEntryWidth.W)
-  }))
-  val resp = Decoupled(Vec(Isa.maxNumRegs, new Bundle {
+  })))
+  // same-cycle as `req`
+  val resp = Valid(Vec(Isa.maxNumRegs, new Bundle {
     val enable = Bool()
     val pReg = Option.when(hasPReg)(pRegT)
     val data = Vec(numLanes, regDataT)
@@ -103,7 +105,7 @@ class DuplicatedCollector(implicit p: Parameters) extends CoreModule()(p) {
   }
 
   // collector banks, i.e. flip-flops that stage collected register data until
-  // EX consumes them
+  // EX consumes them.  Separate per-reg, enq/deq drifts need care.
   val collBankEntries = 2
   val collBanks = Seq.fill(Isa.maxNumRegs)(Module(
     new Queue(gen = vecRegDataT, entries = collBankEntries, pipe = true)
@@ -117,22 +119,23 @@ class DuplicatedCollector(implicit p: Parameters) extends CoreModule()(p) {
   val readPRegs = io.readReq.bits.regs.map(_.pReg)
   (readEns lazyZip readPRegs lazyZip rfBanks lazyZip collBanks)
     .foreach { case (en, pReg, banks, collBank) =>
-      val bankEn = io.readReq.fire && en && (pReg =/= 0.U)
+      val bankEn = io.readReq.fire && en
       val bankPorts = VecInit(banks.map(_.readPorts.head))
       val bankId = regBankId(pReg)
       val nextBankId = RegNext(bankId, 0.U)
       val nextEn = RegNext(bankEn, false.B)
       val nextPReg = RegNext(pReg, 0.U)
 
       bankPorts.foreach(_.enable := false.B)
-      bankPorts(bankId).enable := bankEn
+      bankPorts(bankId).enable := bankEn && (pReg =/= 0.U)
       bankPorts.foreach(_.address := regBankAddr(pReg))
 
-      val bankOut = Mux(nextPReg =/= 0.U,
+      val regOut = Mux(nextPReg =/= 0.U,
         VecInit(bankPorts.map(_.data))(nextBankId),
         vecZeros)
+      // TODO: @perf: Skip latching 0.U to collBank
       collBank.io.enq.valid := nextEn
-      collBank.io.enq.bits := bankOut
+      collBank.io.enq.bits := regOut
     }
 
   // read response
@@ -164,13 +167,19 @@ class DuplicatedCollector(implicit p: Parameters) extends CoreModule()(p) {
   io.writeResp.ports.head.bits.collEntry := 0.U // duplicated collector has no collector entry
 
   // operand serve (readData)
+  // currently, collector drives response with valid data regardless of EX req
   io.readData.resp.valid := collBanks.map(_.io.deq.valid).reduce(_ || _)
   (io.readData.resp.bits zip collBanks).foreach { case (opnd, collBank) =>
+    // FIXME: this doesn't work when collBanks drift!!
     opnd.enable := collBank.io.deq.valid
     opnd.data := collBank.io.deq.bits
     assert(opnd.pReg.isEmpty)
-    // dequeue.  Note: should be fire to consider EX back-pressure
-    collBank.io.deq.ready := io.readData.resp.fire
+  }
+  // dequeue collector bank on successful serve
+  (io.readData.req.bits zip collBanks).foreach { case (req, collBank) =>
+    assert(!req.enable || io.readData.req.valid,
+      "collector: reg.enable set when request is invalid?")
+    collBank.io.deq.ready := req.enable
   }
 }
 

src/main/scala/radiance/muon/ReservationStation.scala

@@ -237,9 +237,10 @@ class ReservationStation(implicit p: Parameters) extends CoreModule()(p) {
   // issue
   // -----
 
-  def issueArbBundleT = new Bundle {
+  def issueBundleT = new Bundle {
     val entry = chiselTypeOf(io.issue.bits)
     val entryId = UInt(log2Ceil(numEntries).W)
+    val hasOps = chiselTypeOf(hasOpTable.head)
   }
 
   // check issue eligiblity after collection finished & RAW settled
@@ -270,10 +271,11 @@ class ReservationStation(implicit p: Parameters) extends CoreModule()(p) {
 
     eligibleTable(i) := eligible
 
-    val candidate = Wire(Decoupled(issueArbBundleT))
+    val candidate = Wire(Decoupled(issueBundleT))
     candidate.valid := eligible
     candidate.bits.entry := instTable(i)
     candidate.bits.entryId := i.U
+    candidate.bits.hasOps := hasOps
 
     // deregister upon issue
     when (candidate.fire) {
@@ -292,14 +294,6 @@ class ReservationStation(implicit p: Parameters) extends CoreModule()(p) {
   (issueScheduler.io.in zip eligibles).foreach { case (s, e) => s <> e }
   val issueScheduled = issueScheduler.io.out
 
-  // drive collector's operand serve port upon issue, for use in EX
-  // make sure to line this up with EX fire, so that collector data gets
-  // consumed when it's used
-  io.collector.readData.req.valid := io.issue.valid
-  io.collector.readData.req.bits.collEntry := 0.U // fixed for DuplicatedCollector, TODO
-  io.collector.readData.resp.ready := io.issue.fire
-  assert(useCollector, "FIXME: !useCollector is broken currently")
-
   val issuedId = WireDefault(issueScheduler.io.out.bits.entryId)
   // FIXME: Old bypass-specific code; remove
   //
@@ -338,12 +332,12 @@ class ReservationStation(implicit p: Parameters) extends CoreModule()(p) {
   // latch the issue at the output to cut timing before EX (which currently
   // does not latch its input), and also to stage it when EX is !ready.
   val issueStaged = Module(
-    new Queue(gen = chiselTypeOf(io.issue.bits), entries = 1, pipe = true)
+    new Queue(gen = issueBundleT, entries = 1, pipe = true)
   )
-  issueStaged.io.enq.valid := issueScheduler.io.out.valid
-  issueStaged.io.enq.bits := issueScheduler.io.out.bits.entry
-  issueScheduler.io.out.ready := issueStaged.io.enq.ready
-  io.issue <> issueStaged.io.deq
+  issueStaged.io.enq <> issueScheduler.io.out
+  io.issue.valid := issueStaged.io.deq.valid
+  io.issue.bits := issueStaged.io.deq.bits.entry
+  issueStaged.io.deq.ready := io.issue.ready
 
   if (muonParams.debug) {
     when (io.issue.fire) {
@@ -352,6 +346,16 @@ class ReservationStation(implicit p: Parameters) extends CoreModule()(p) {
     }
   }
 
+  // drive collector's operand serve port upon issue, for use in EX
+  // make sure to line this up with EX fire, so that collector data gets
+  // consumed when it's used
+  io.collector.readData.req.valid := io.issue.valid
+  (io.collector.readData.req.bits zip issueStaged.io.deq.bits.hasOps).foreach { case (req, hasOp) =>
+    req.enable := io.issue.fire && hasOp
+    req.collEntry := 0.U // fixed for DuplicatedCollector, TODO
+  }
+
+  assert(useCollector, "FIXME: !useCollector is broken currently")
 
   // ---------
   // writeback