Skip to content

Commit

Permalink
Improve SyncReadMem.read, .readWrite (#3221) (#3233)
Browse files Browse the repository at this point in the history 
* Mark generated wires for read and readWrite as temporary
* Remove semicolons
* Remove intermediate wire for read/readWrite port address
* Convert usages of UnlocatableSourceInfo into macro applications

(cherry picked from commit e078c0f)

Co-authored-by: Jared Barocsi <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
@mergify @jared-barocsi
mergify[bot] and jared-barocsi authored May 2, 2023
1 parent 262a0ec commit 8d0935f
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 52 deletions.
106 changes: 72 additions & 34 deletions core/src/main/scala/chisel3/Mem.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import chisel3.internal._
import chisel3.internal.Builder.pushCommand
import chisel3.internal.firrtl._
import chisel3.internal.sourceinfo.{MemTransform, SourceInfoTransform}
import chisel3.experimental.{SourceInfo, SourceLine, UnlocatableSourceInfo}
import chisel3.experimental.{SourceInfo, SourceLine}

object Mem {

Expand Down Expand Up @@ -142,7 +142,10 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
* @param idx memory element index to write into
* @param data new data to write
*/
def write(idx: UInt, data: T): Unit =
def write(idx: UInt, data: T): Unit = macro SourceInfoTransform.idxDataArg

/** @group SourceInfoTransformMacro */
def do_write(idx: UInt, data: T)(implicit sourceInfo: SourceInfo): Unit =
write_impl(idx, data, Builder.forcedClock, true)

/** Creates a write accessor into the memory with a clock
Expand All @@ -152,20 +155,24 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
* @param data new data to write
* @param clock clock to bind to this accessor
*/
def write(idx: UInt, data: T, clock: Clock): Unit =
def write(idx: UInt, data: T, clock: Clock): Unit = macro SourceInfoTransform.idxDataClockArg

/** @group SourceInfoTransformMacro */
def do_write(idx: UInt, data: T, clock: Clock)(implicit sourceInfo: SourceInfo): Unit =
write_impl(idx, data, clock, false)

private def write_impl(
idx: UInt,
data: T,
clock: Clock,
warn: Boolean
)(
implicit sourceInfo: SourceInfo
): Unit = {
if (warn && clockInst.isDefined && clock != clockInst.get) {
clockWarning(None, MemPortDirection.WRITE)
}
implicit val sourceInfo = UnlocatableSourceInfo
makePort(UnlocatableSourceInfo, idx, MemPortDirection.WRITE, clock) := data
makePort(sourceInfo, idx, MemPortDirection.WRITE, clock) := data
}

/** Creates a masked write accessor into the memory.
Expand All @@ -178,11 +185,20 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
* @note this is only allowed if the memory's element data type is a Vec
*/
def write(
idx: UInt,
writeData: T,
mask: Seq[Bool]
)(
implicit evidence: T <:< Vec[_]
): Unit = macro SourceInfoTransform.idxDataMaskArg

def do_write(
idx: UInt,
data: T,
mask: Seq[Bool]
)(
implicit evidence: T <:< Vec[_]
implicit evidence: T <:< Vec[_],
sourceInfo: SourceInfo
): Unit =
masked_write_impl(idx, data, mask, Builder.forcedClock, true)

Expand All @@ -198,12 +214,22 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
* @note this is only allowed if the memory's element data type is a Vec
*/
def write(
idx: UInt,
writeData: T,
mask: Seq[Bool],
clock: Clock
)(
implicit evidence: T <:< Vec[_]
): Unit = macro SourceInfoTransform.idxDataMaskClockArg

def do_write(
idx: UInt,
data: T,
mask: Seq[Bool],
clock: Clock
)(
implicit evidence: T <:< Vec[_]
implicit evidence: T <:< Vec[_],
sourceInfo: SourceInfo
): Unit =
masked_write_impl(idx, data, mask, clock, false)

Expand All @@ -214,9 +240,9 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
clock: Clock,
warn: Boolean
)(
implicit evidence: T <:< Vec[_]
implicit evidence: T <:< Vec[_],
sourceInfo: SourceInfo
): Unit = {
implicit val sourceInfo = UnlocatableSourceInfo
if (warn && clockInst.isDefined && clock != clockInst.get) {
clockWarning(None, MemPortDirection.WRITE)
}
Expand Down Expand Up @@ -373,14 +399,11 @@ sealed class SyncReadMem[T <: Data] private[chisel3] (t: T, n: BigInt, val readU
)(
implicit sourceInfo: SourceInfo
): T = {
val a = Wire(UInt())
a := DontCare
var port: Option[T] = None
var _port: Option[T] = None
when(enable) {
a := addr
port = Some(super.do_apply_impl(a, clock, MemPortDirection.READ, warn))
_port = Some(super.do_apply_impl(addr, clock, MemPortDirection.READ, warn))
}
port.get
_port.get
}
// 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)`
Expand Down Expand Up @@ -462,19 +485,15 @@ sealed class SyncReadMem[T <: Data] private[chisel3] (t: T, n: BigInt, val readU
)(
implicit sourceInfo: SourceInfo
): T = {
val a = Wire(UInt())
a := DontCare

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

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

/** Generates an explicit read-write port for this SyncReadMem, with a bytemask for
Expand Down Expand Up @@ -520,6 +539,17 @@ sealed class SyncReadMem[T <: Data] private[chisel3] (t: T, n: BigInt, val readU
isWrite: Bool
)(
implicit evidence: T <:< Vec[_]
): T = macro SourceInfoTransform.idxDataMaskEnIswArg

def do_readWrite(
idx: UInt,
writeData: T,
mask: Seq[Bool],
en: Bool,
isWrite: Bool
)(
implicit evidence: T <:< Vec[_],
sourceInfo: SourceInfo
): 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
Expand Down Expand Up @@ -549,7 +579,19 @@ sealed class SyncReadMem[T <: Data] private[chisel3] (t: T, n: BigInt, val readU
clock: Clock
)(
implicit evidence: T <:< Vec[_]
): T = masked_readWrite_impl(idx, writeData, mask, en, isWrite, clock, true)
): T = macro SourceInfoTransform.idxDataMaskEnIswClockArg

def do_readWrite(
idx: UInt,
writeData: T,
mask: Seq[Bool],
en: Bool,
isWrite: Bool,
clock: Clock
)(
implicit evidence: T <:< Vec[_],
sourceInfo: SourceInfo
) = masked_readWrite_impl(idx, writeData, mask, en, isWrite, clock, true)

private def masked_readWrite_impl(
addr: UInt,
Expand All @@ -560,17 +602,13 @@ sealed class SyncReadMem[T <: Data] private[chisel3] (t: T, n: BigInt, val readU
clock: Clock,
warn: Boolean
)(
implicit evidence: T <:< Vec[_]
implicit evidence: T <:< Vec[_],
sourceInfo: SourceInfo
): T = {
implicit val sourceInfo = UnlocatableSourceInfo
val a = Wire(UInt())
a := DontCare

var port: Option[T] = None
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]]
_port = Some(super.do_apply_impl(addr, clock, MemPortDirection.RDWR, warn))
val accessor = _port.get.asInstanceOf[Vec[Data]]

when(isWrite) {
val dataVec = data.asInstanceOf[Vec[Data]]
Expand All @@ -585,6 +623,6 @@ sealed class SyncReadMem[T <: Data] private[chisel3] (t: T, n: BigInt, val readU
when(cond) { p := datum }
}
}
port.get
_port.get
}
}
39 changes: 39 additions & 0 deletions macros/src/main/scala/chisel3/internal/sourceinfo/SourceInfoTransform.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,14 @@ class SourceInfoTransform(val c: Context) extends AutoSourceTransform {
q"$thisObj.$doFuncTerm($n, $x)($implicitSourceInfo)"
}

def idxDataArg(idx: c.Tree, data: c.Tree): c.Tree = {
q"$thisObj.$doFuncTerm($idx, $data)($implicitSourceInfo)"
}

def idxDataClockArg(idx: c.Tree, data: c.Tree, clock: c.Tree): c.Tree = {
q"$thisObj.$doFuncTerm($idx, $data, $clock)($implicitSourceInfo)"
}

def idxEnClockArg(idx: c.Tree, en: c.Tree, clock: c.Tree): c.Tree = {
q"$thisObj.$doFuncTerm($idx, $en, $clock)($implicitSourceInfo)"
}
Expand All @@ -227,10 +235,41 @@ class SourceInfoTransform(val c: Context) extends AutoSourceTransform {
q"$thisObj.$doFuncTerm($idx, $writeData, $en, $isWrite)($implicitSourceInfo)"
}

def idxDataMaskArg(idx: c.Tree, writeData: c.Tree, mask: c.Tree)(evidence: c.Tree): c.Tree = {
q"$thisObj.$doFuncTerm($idx, $writeData, $mask)($evidence, $implicitSourceInfo)"
}

def idxDataMaskClockArg(idx: c.Tree, writeData: c.Tree, mask: c.Tree, clock: c.Tree)(evidence: c.Tree): c.Tree = {
q"$thisObj.$doFuncTerm($idx, $writeData, $mask, $clock)($evidence, $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 idxDataMaskEnIswArg(
idx: c.Tree,
writeData: c.Tree,
mask: c.Tree,
en: c.Tree,
isWrite: c.Tree
)(evidence: c.Tree
): c.Tree = {
q"$thisObj.$doFuncTerm($idx, $writeData, $mask, $en, $isWrite)($evidence, $implicitSourceInfo)"
}

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

def xEnArg(x: c.Tree, en: c.Tree): c.Tree = {
q"$thisObj.$doFuncTerm($x, $en)($implicitSourceInfo)"
}
Expand Down
36 changes: 18 additions & 18 deletions src/test/scala/chiselTests/Mem.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,14 @@ class MemReadWriteTester extends BasicTester {
wdata := 2.U
}
is(2.U) { // Cycle 3: Read from address 0 (data returned next cycle)
address := 0.U;
enable := true.B;
isWrite := false.B;
address := 0.U
enable := true.B
isWrite := false.B
}
is(3.U) { // Cycle 4: Expect RDWR port to contain 3.U, then read from address 1
address := 1.U;
enable := true.B;
isWrite := false.B;
address := 1.U
enable := true.B
isWrite := false.B
assert(rdata === 3.U)
}
is(4.U) { // Cycle 5: Expect rdata to contain 2.U
Expand Down Expand Up @@ -285,16 +285,16 @@ class MemMaskedReadWriteTester extends BasicTester {
wdata := VecInit(5.U, 6.U, 7.U, 8.U)
}
is(2.U) { // Cycle 3: Read from address 0 (data returned next cycle)
address := 0.U;
enable := true.B;
isWrite := false.B;
address := 0.U
enable := true.B
isWrite := false.B
}
is(3.U) { // Cycle 4: Expect RDWR port to contain (1.U, 2.U, 3.U, 4.U), then read from address 1
assert(rdata === VecInit(1.U, 2.U, 3.U, 4.U))

address := 1.U;
enable := true.B;
isWrite := false.B;
address := 1.U
enable := true.B
isWrite := false.B
}
is(4.U) { // Cycle 5: Expect rdata to contain (5.U, 6.U, 7.U, 8.U)
assert(rdata === VecInit(5.U, 6.U, 7.U, 8.U))
Expand All @@ -316,17 +316,17 @@ class MemMaskedReadWriteTester extends BasicTester {
wdata := VecInit(100.U, 0.U, 0.U, 100.U)
}
is(7.U) { // Cycle 8: Read from address 0 (data returned next cycle)
address := 0.U;
enable := true.B;
isWrite := false.B;
address := 0.U
enable := true.B
isWrite := false.B
}
is(8.U) { // Cycle 9: Expect RDWR port to contain (0.U, 2.U, 3.U, 0.U), then read from address 1
// NOT (0.U, 100.U, 100.U, 0.U)
assert(rdata === VecInit(0.U, 2.U, 3.U, 0.U))

address := 1.U;
enable := true.B;
isWrite := false.B;
address := 1.U
enable := true.B
isWrite := false.B
}
is(9.U) { // Cycle 10: Expect rdata to contain (5.U, 0.U, 0.U, 8.U)
// NOT (100.U, 0.U, 0.U, 100.U)
Expand Down

0 comments on commit 8d0935f

Please sign in to comment.