[arch/memdomain] refractor: finish the back-end arch unification#20
Conversation
Mikemy666
commented
Dec 18, 2025
- Achieve unified backend architecture.
- Modify the readme reference file.
- Set up the translator IO framework; currently, it supports pass-through functionality.
There was a problem hiding this comment.
Pull request overview
This PR refactors the memory domain backend to achieve a unified architecture by consolidating separate scratchpad and accumulator banks into a single AccMonitor abstraction. The changes eliminate the previously separate AccBank module and simplify the memory controller's connection topology by removing intermediate ToPhysicalLine translation layers, implementing pass-through functionality in the Translator instead.
Key Changes:
- Introduced
AccMonitor.scalawhich wrapsSramBankwith an accumulation pipeline (AccPipe) and read router (AccReadRouter), replacing the formerAccBank.scala - Unified all memory banks in
Scratchpadto useAccMonitor, combiningsp_banksandacc_banksinto a single pool with consistent parameters - Removed
ToPhysicalLinemodules fromMemDomain, connecting translator outputs directly to scratchpad IO
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| AccMonitor.scala | New unified bank monitor with accumulation support; adds metadata fields (rob_id, bank_id, is_acc) to read/write IOs |
| AccBank.scala | Deleted - functionality moved to AccMonitor.scala |
| Scratchpad.scala | Refactored to instantiate AccMonitor for all banks; unified interface using SramReadWithInfo/SramWriteWithInfo; added metadata muxing for write arbitration |
| MemDomain.scala | Removed ToPhysicalLine instantiations; simplified connections by wiring translator directly to scratchpad |
| MemController.scala | Removed redundant self-import of MemController |
| README.md | Updated documentation to reflect unified architecture; expanded module descriptions and interface specifications |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val bank_id = Input(UInt(log2Up(b.sp_banks+b.acc_banks).W)) | ||
| } | ||
|
|
||
| class AccMonitorReadIO(n: Int, w: Int)(implicit b: CustomBuckyballConfig, p: Parameters) extends Bundle{ |
There was a problem hiding this comment.
Missing space before opening brace. Should be 'Bundle {' instead of 'Bundle{'.
| class AccMonitorReadIO(n: Int, w: Int)(implicit b: CustomBuckyballConfig, p: Parameters) extends Bundle{ | |
| class AccMonitorReadIO(n: Int, w: Int)(implicit b: CustomBuckyballConfig, p: Parameters) extends Bundle { |
| } | ||
|
|
||
|
|
||
| class AccReadRouter(val n: Int, val w: Int)(implicit b: CustomBuckyballConfig, p: Parameters) extends Module { |
There was a problem hiding this comment.
Missing space after closing parenthesis before 'extends'. Should be ') extends' instead of ')extends'.
| } | ||
|
|
||
|
|
||
| class AccMonitor(n: Int, w: Int, aligned_to: Int, single_ported: Boolean) (implicit b: CustomBuckyballConfig, p: Parameters) extends Module { |
There was a problem hiding this comment.
Missing space after closing parenthesis before 'extends'. Should be ') extends' instead of ')extends'.
| valid_reg := io.write_in.io.req.valid | ||
| addr_reg := io.write_in.io.req.bits.addr | ||
| data_reg := io.write_in.io.req.bits.data | ||
| mask_reg := io.write_in.io.req.bits.mask |
There was a problem hiding this comment.
The pipeline register valid_reg is updated based on io.write_in.io.req.valid without checking if the transaction actually fires (i.e., both valid and ready are true). This should be valid_reg := io.write_in.io.req.fire to ensure the pipeline only advances when the request is accepted. Similarly, addr_reg, data_reg, and mask_reg should also be conditionally updated using when(io.write_in.io.req.fire).
| valid_reg := io.write_in.io.req.valid | |
| addr_reg := io.write_in.io.req.bits.addr | |
| data_reg := io.write_in.io.req.bits.data | |
| mask_reg := io.write_in.io.req.bits.mask | |
| valid_reg := io.write_in.io.req.fire | |
| when (io.write_in.io.req.fire) { | |
| addr_reg := io.write_in.io.req.bits.addr | |
| data_reg := io.write_in.io.req.bits.data | |
| mask_reg := io.write_in.io.req.bits.mask | |
| } |
| bank.io.write.is_acc := Mux(exec_write_sel, exec_isacc_sel, main_isacc_sel) | ||
| bank.io.write.bank_id := Mux(exec_write_sel, exec_bankid, main_bankid) | ||
| bank.io.write.rob_id := Mux(exec_write_sel, exec_robid, main_robid) |
There was a problem hiding this comment.
Inconsistent alignment in assignment statements. The spacing between field names and assignment operators should be uniform across lines 109-111 for better readability.
| bank.io.write.is_acc := Mux(exec_write_sel, exec_isacc_sel, main_isacc_sel) | |
| bank.io.write.bank_id := Mux(exec_write_sel, exec_bankid, main_bankid) | |
| bank.io.write.rob_id := Mux(exec_write_sel, exec_robid, main_robid) | |
| bank.io.write.is_acc := Mux(exec_write_sel, exec_isacc_sel, main_isacc_sel) | |
| bank.io.write.bank_id := Mux(exec_write_sel, exec_bankid, main_bankid) | |
| bank.io.write.rob_id := Mux(exec_write_sel, exec_robid, main_robid) |
| import framework.memdomain.mem.{SramReadIO, SramWriteIO} | ||
| import framework.balldomain.blink.{SramReadWithInfo, SramWriteWithInfo} | ||
|
|
||
| class AccMonitorWriteIO(n: Int, w: Int, mask_len: Int)(implicit b: CustomBuckyballConfig, p: Parameters)extends Bundle { |
There was a problem hiding this comment.
Missing space after closing parenthesis before 'extends'. Should be ') extends' instead of ')extends'.
| class AccMonitorWriteIO(n: Int, w: Int, mask_len: Int)(implicit b: CustomBuckyballConfig, p: Parameters)extends Bundle { | |
| class AccMonitorWriteIO(n: Int, w: Int, mask_len: Int)(implicit b: CustomBuckyballConfig, p: Parameters) extends Bundle { |