Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement read-write memory accessors for SyncReadMem #3190

Merged
merged 16 commits into from
Apr 25, 2023
Merged
268 changes: 268 additions & 0 deletions core/src/main/scala/chisel3/Mem.scala
Original file line number Diff line number Diff line change
Expand Up @@ -392,4 +392,272 @@ sealed class SyncReadMem[T <: Data] private[chisel3] (
}
// note: we implement do_read(addr) for SyncReadMem in terms of do_read(addr, en) in order to ensure that
// `mem.read(addr)` will always behave the same as `mem.read(addr, true.B)`

/** Generates an explicit read-write port for this SyncReadMem. Note that this does not infer
* port directionality based on connection semantics and the `when` context unlike SyncReadMem.apply(),
* so the behavior of the port must be controlled by changing the values of the input parameters.
*
* @param idx memory element index to write into
* @param writeData new data to write
* @param enable enables access to the memory
* @param isWrite performs a write instead of a read when enable is true; the return
* value becomes undefined when this parameter is true
*
* @return The value of the memory at idx when enable is true and isWrite is false,
* or an undefined value otherwise.
*
* @example Controlling a read/write port with IO signals
* {{{
* class MyMemWrapper extends Module {
* val width = 2
*
* val io = IO(new Bundle {
* val address = Input(UInt())
* val wdata = Input(UInt(width.W))
* val enable = Input(Bool())
* val isWrite = Input(Bool())
* val rdata = Output(UInt(width.W))
* })
*
* val mem = SyncReadMem(2, UInt(width.W))
* val rwPort = mem.readWrite(io.address, io.wdata, io.enable, io.isWrite)
* io.rdata := rwPort
* }
*
* class Top extends Module {
* val mem = Module(new MyMemWrapper)
* mem.io := DontCare
*
* val rdata = Wire(UInt(2.W))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code would not quite compile

* readData := DontCare
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* val rdata = Wire(UInt(2.W))
* readData := DontCare
* val rdata = Wire(UInt(2.W))
* rdata := DontCare

*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should default mem.io.enable to false.B

* // Read from the memory port
* when(doRead) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is doRead? an Input?

* mem.io.enable := true.B
* mem.io.isWrite := false.B
* mem.io.address := someAddress
* }
* // Read data will show up on the next cycle
* when(timeToRead) {
* readData := mem.io.rdata
Copy link
Contributor

@mwachs5 mwachs5 Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a little confusing, the user is going to have to understand that the rddata is available the following cycle. Let's just omit this part

* }
*
* // Write to the memory
* when(doWrite) {
* mem.io.enable := true.B
* mem.io.isWrite := true.B
* mem.io.address := anotherAddress
* mem.io.wdata := 3.U
* }
* }
* }}}
*/
def readWrite(idx: UInt, writeData: T, en: Bool, isWrite: Bool): T = macro SourceInfoTransform.idxDataEnIswArg

/** @group SourceInfoTransformMacro */
def do_readWrite(idx: UInt, writeData: T, en: Bool, isWrite: Bool)(implicit sourceInfo: SourceInfo): T =
_readWrite_impl(idx, writeData, en, isWrite, Builder.forcedClock, true)

/** Generates an explicit read-write port for this SyncReadMem, using a clock that may be
* different from the implicit clock.
*
* @param idx memory element index to write into
* @param writeData new data to write
* @param enable enables access to the memory
* @param isWrite performs a write instead of a read when enable is true; the return
* value becomes undefined when this parameter is true
* @param clock clock to bind to this read-write port
*
* @return The value of the memory at idx when enable is true and isWrite is false,
* or an undefined value otherwise.
*/
jared-barocsi marked this conversation as resolved.
Show resolved Hide resolved
def readWrite(idx: UInt, writeData: T, en: Bool, isWrite: Bool, clock: Clock): T =
macro SourceInfoTransform.idxDataEnIswClockArg

/** @group SourceInfoTransformMacro */
def do_readWrite(
idx: UInt,
data: T,
en: Bool,
isWrite: Bool,
clock: Clock
)(
implicit sourceInfo: SourceInfo
): T =
_readWrite_impl(idx, data, en, isWrite, clock, true)

/** @group SourceInfoTransformMacro */
private def _readWrite_impl(
addr: UInt,
data: T,
enable: Bool,
isWrite: Bool,
clock: Clock,
warn: Boolean
)(
implicit sourceInfo: SourceInfo
): T = {
val a = Wire(UInt())
a := DontCare
Comment on lines +473 to +474
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using val _a indicates better that this is a temporary. Same for port. Basically, I would use _ for any hardware component created in a library/utility.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nits/golf:

Suggested change
val a = Wire(UInt())
a := DontCare
val _a = WireDefault(chiselTypeOf(addr), DontCare)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments for the masked version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the declaration of a separate from the connection of addr to it required for the enable to infer correctly @jared-barocsi? Or why else is is split?

Also are we sure returning the port works? Such value capturing is often problematic for lexical scoping, what happens if someone uses the return value of this function? Perhaps we should be assigning the read data to a wire? The integration test is passing so that suggests the lexical scoping is okay or at least supported by firtool but I'd be curious to see what the FIRRTL actually looks like.


var port: Option[T] = None
when(enable) {
a := addr
port = Some(super.do_apply_impl(a, clock, MemPortDirection.RDWR, warn))

when(isWrite) {
port.get := data
}
}
port.get
}

/** Generates an explicit read-write port for this SyncReadMem, with a bytemask for
* performing partial writes to a Vec element.
*
* @param idx memory element index to write into
* @param writeData new data to write
* @param mask the write mask as a Seq of Bool: a write to the Vec element in
* memory is only performed if the corresponding mask index is true.
* @param enable enables access to the memory
* @param isWrite performs a write instead of a read when enable is true; the return
* value becomes undefined when this parameter is true
*
* @return The read Vec value of the memory at idx when enable is true and isWrite is false,
* or an undefined value otherwise.
*
* @example Controlling a read/masked write port with IO signals
* {{{
* class MyMaskedMemWrapper extends Module {
* val width = 2
*
* val io = IO(new Bundle {
* val address = Input(UInt())
* val wdata = Input(UInt(width.W))
* val mask = Input(Vec(2, Bool()))
* val enable = Input(Bool())
* val isWrite = Input(Bool())
* val rdata = Output(UInt(width.W))
* })
*
* val mem = SyncReadMem(2, Vec(2, UInt(width.W)))
* val rwPort = mem.readWrite(io.address, io.wdata, io.mask, io.enable, io.isWrite)
* io.rdata := rwPort
* }
*
* class Top extends Module {
* val mem = Module(new MyMemWrapper)
* mem.io := DontCare
*
* val rdata = Wire(UInt(2.W))
* readData := DontCare
*
* // Read from the memory port
* when(doRead) {
* mem.io.enable := true.B
* mem.io.isWrite := false.B
* mem.io.address := someAddress
* }
* // Read data will show up on the next cycle
* when(timeToRead) {
jared-barocsi marked this conversation as resolved.
Show resolved Hide resolved
* readData := mem.io.rdata
* }
*
* // Write to all Vec entries at a specific address in the memory
* when(doNormalWrite) {
* mem.io.enable := true.B
* mem.io.isWrite := true.B
* mem.io.mask := VecInit(true.B, true.B)
* mem.io.address := anotherAddress
* mem.io.wdata := VecInit(2.U, 3.U)
* }
*
* // Write to the memory with a partial bytemask
* when(doMaskedWrite) {
* mem.io.enable := true.B
* mem.io.isWrite := true.B
* mem.io.mask := VecInit(true.B, false.B) // Write only the first element of the vec at `anotherAddress`
* mem.io.address := anotherAddress
* mem.io.wdata := VecInit(0.U, 0.U)
* }
* }
* }}}
*
* @note this is only allowed if the memory's element data type is a Vec
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least in FIRRTL this is allowed so long as the type of the mask is the same type as the underlying data type with the leaf types as all 1-bit UInts. I.e., this is supported on any aggregate including bundles.

This is a relatively niche feature and pretty rare. I am fine to start this as only working for Vec (which makes sense).

*/
def readWrite(
idx: UInt,
writeData: T,
mask: Seq[Bool],
en: Bool,
isWrite: Bool
)(
implicit evidence: T <:< Vec[_]
): T = masked_readWrite_impl(idx, writeData, mask, en, isWrite, Builder.forcedClock, true)

/** Generates an explicit read-write port for this SyncReadMem, with a bytemask for
* performing partial writes to a Vec element and a clock that may be different from
* the implicit clock.
*
* @param idx memory element index to write into
* @param writeData new data to write
* @param mask the write mask as a Seq of Bool: a write to the Vec element in
* memory is only performed if the corresponding mask index is true.
* @param enable enables access to the memory
* @param isWrite performs a write instead of a read when enable is true; the return
* value becomes undefined when this parameter is true
* @param clock clock to bind to this read-write port
*
* @return The read Vec value of the memory at idx when enable is true and isWrite is false,
* or an undefined value otherwise.
*
* @note this is only allowed if the memory's element data type is a Vec
jared-barocsi marked this conversation as resolved.
Show resolved Hide resolved
*/
def readWrite(
idx: UInt,
writeData: T,
mask: Seq[Bool],
en: Bool,
isWrite: Bool,
clock: Clock
)(
implicit evidence: T <:< Vec[_]
): T = masked_readWrite_impl(idx, writeData, mask, en, isWrite, clock, true)

private def masked_readWrite_impl(
addr: UInt,
data: T,
mask: Seq[Bool],
enable: Bool,
isWrite: Bool,
clock: Clock,
warn: Boolean
)(
implicit evidence: T <:< Vec[_]
): T = {
implicit val sourceInfo = UnlocatableSourceInfo
val a = Wire(UInt())
Comment on lines +558 to +574
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would also be good to have a source locator here, since we're already taking an implicit argument we can just take the sourceInfo argument. Users won't be able to immediately call .apply on the return value, but due to the existing implicit argument, they already can't.

a := DontCare

var port: Option[T] = None
when(enable) {
a := addr
port = Some(super.do_apply_impl(a, clock, MemPortDirection.RDWR, warn))
val accessor = port.get.asInstanceOf[Vec[Data]]

when(isWrite) {
val dataVec = data.asInstanceOf[Vec[Data]]
if (accessor.length != dataVec.length) {
Builder.error(s"Mem write data must contain ${accessor.length} elements (found ${dataVec.length})")
}
if (accessor.length != mask.length) {
Builder.error(s"Mem write mask must contain ${accessor.length} elements (found ${mask.length})")
}

for (((cond, p), datum) <- mask.zip(accessor).zip(dataVec))
when(cond) { p := datum }
}
}
port.get
}
}
22 changes: 22 additions & 0 deletions docs/src/explanations/memories.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,28 @@ Here is an example single read/write port waveform, with [masks](#masks) (again,

![read/write ports example waveform](https://svg.wavedrom.com/github/freechipsproject/www.chisel-lang.org/master/docs/src/main/resources/json/smem_rw.json)

Single-ported SRAMs can also be explicitly generated by using the `readWrite` call, which yields a single read/write accessor like so:

```scala mdoc:silent
class RDWR_Smem extends Module {
val width: Int = 32
val io = IO(new Bundle {
val enable = Input(Bool())
val write = Input(Bool())
val addr = Input(UInt(10.W))
val dataIn = Input(UInt(width.W))
val dataOut = Output(UInt(width.W))
})

val mem = SyncReadMem(1024, UInt(width.W))
val rwPort = mem.readWrite(io.addr, io.dataIn, io.enable, io.write)
jared-barocsi marked this conversation as resolved.
Show resolved Hide resolved

when(io.enable && !io.write) {
io.dataOut := rwPort
jared-barocsi marked this conversation as resolved.
Show resolved Hide resolved
}
}
```

### `Mem`: combinational/asynchronous-read, sequential/synchronous-write

Chisel supports random-access memories via the `Mem` construct. Writes to `Mem`s are combinational/asynchronous-read, sequential/synchronous-write. These `Mem`s will likely be synthesized to register banks, since most SRAMs in modern technologies (FPGA, ASIC) tend to no longer support combinational (asynchronous) reads.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,14 @@ class SourceInfoTransform(val c: Context) extends AutoSourceTransform {
q"$thisObj.$doFuncTerm($idx, $en, $clock)($implicitSourceInfo)"
}

def idxDataEnIswArg(idx: c.Tree, writeData: c.Tree, en: c.Tree, isWrite: c.Tree): c.Tree = {
q"$thisObj.$doFuncTerm($idx, $writeData, $en, $isWrite)($implicitSourceInfo)"
}

def idxDataEnIswClockArg(idx: c.Tree, writeData: c.Tree, en: c.Tree, isWrite: c.Tree, clock: c.Tree): c.Tree = {
q"$thisObj.$doFuncTerm($idx, $writeData, $en, $isWrite, $clock)($implicitSourceInfo)"
}

def xEnArg(x: c.Tree, en: c.Tree): c.Tree = {
q"$thisObj.$doFuncTerm($x, $en)($implicitSourceInfo)"
}
Expand Down
Loading