chipsalliance / chipsalliance/chisel
Better instantiation and inference of SyncReadMem
- Dominant language
- Scala
- Stars
- 4.8k
- Forks
- 658
- Avg merge
- 18h 59m
- Merged PRs (30d)
- 14
Description
**Type of issue**: feature request
**Impact**: API modification
**Development Phase**: proposal
**Other information**
**If the current behavior is a bug, please provide the steps to reproduce the problem:**
**What is the current behavior?**
For a long time, we have been using `--infer-rw --repl-seq-mem` to infer the SRAMs to replace the SyncReadMem in Chisel. However, we have the following issues when using the generated verilog for SRAMs by FIRRTL transform.
First, SRAMs provided by foundries usually have different features of reading and writing. For example, some SRAMs hold their read data after a read enable until the next read enable. However, we cannot simulate this type of SRAM in the generated verilog since they are generated in the FIRRTL stage instead of Chisel (the current behavior of read is always `ram[raddr]` or assigned with random values if `GARBAGE_ASSIGN` is enabled).
Second, SRAMs typically have non-functional pins that need to be connected to the top or to mbist controllers inside some hierarchy. However, we cannot easily add extra IOs to the SyncReadMem.
There are various alternative ways to address the issues.
We can drop the SyncReadMem and describe the SRAMs in Chisel modules. However, here comes the issue of module names. Due to the dedup, SRAM modules may result in uncontinuous module name prefixes, which is a little bit hard to read. This issue can be addressed by manually maintain the SRAM indexes and pass a `desiredName` to the SRAM modules, as we did [here](https://github.com/OpenXiangShan/HuanCun/blob/southlake/src/main/scala/huancun/utils/SRAMTemplate.scala#L202-L228).
We also think of the Definition and Instance work Chisel has provided. Although it addresses with the naming issue, its usage is not that good. First, it cannot be detected by IDEA. Second, it does not support `@public` for `def`s but we do need the `read` and `write` functions to access the SRAMs. Accessing SRAMs with `read` and `write` are much better than accessin with IOs that differs between different types of SRAMs.
By implementing the SRAMs in Chisel, we also have the issue of losing the SRAM configuration files generated by FIRRTL. We have to manually add some code in Scala to track the SRAMs and generate the configuration file before chisel finishes the elaborate of all modules. This is not elegant for a project since we have to add something like `dumpSRAMConf(pathToConfFile)` everywhere we call ChiselStage. Unless, we have to use a Python script or something like this to parse the verilog and generate the configuration file, which is even worse than the `dumpSRAMConf(pathToConfFile)` since we have to deal with the generated Verilog/SV.
I understand the above issues can be resolved by hacking the FIRRTL transform or other project-dependent coding tricks in Chisel. But I do think a better native SyncReadMem provided by Chisel or chise3.util is what we need.
**What is the expected behavior?**
For SyncReadMem:
- Allow the user to specify the behavior of read data. Specifically, (1) whether to hold the read data until the next read enable or (write/read) enable, (2)whether to bypass write data (RAW) for dual-port (and more-than-one-port if needed) SRAMs. There may be more features requested, but we can start with simpler and more urgent ones.
- Allow the user to add extra pins (and possibly default values of the output pins to tie them off) for the SRAMs.
**Please tell us about your environment:**
**What is the use case for changing the behavior?**
Use Chisel for tape-out in various processes with SRAMs from different foundries.
Contributor guide
Assessment
This issue has not been assessed yet.