Optional clock param for memory ports (#2333) (#2382)

Warn if clock at memory instantiation differs from clock bound at port
creation and port clock is not manually passed

Co-authored-by: Jack Koenig <koenig@sifive.com>
(cherry picked from commit 465805ec7b2696a985eaa12cf9c6868f11ac2931)

Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com>

1 files changed, 142 insertions, 19 deletions

diff --git a/core/src/main/scala/chisel3/Mem.scala b/core/src/main/scala/chisel3/Mem.scala
index ff5072ad..4b7c0815 100644
--- a/core/src/main/scala/chisel3/Mem.scala
+++ b/core/src/main/scala/chisel3/Mem.scala
@@ -56,6 +56,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
     with SourceInfoDoc {
   _parent.foreach(_.addId(this))
 
+  private val clockInst: Clock = Builder.forcedClock
   // REVIEW TODO: make accessors (static/dynamic, read/write) combinations consistent.
 
   /** Creates a read accessor into the memory with static addressing. See the
@@ -63,17 +64,17 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
     */
   def apply(x: BigInt): T = macro SourceInfoTransform.xArg
 
-  /** Creates a read accessor into the memory with static addressing. See the
-    * class documentation of the memory for more detailed information.
-    */
-  def apply(x: Int): T = macro SourceInfoTransform.xArg
-
   /** @group SourceInfoTransformMacro */
   def do_apply(idx: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T = {
     require(idx >= 0 && idx < length)
-    apply(idx.asUInt)
+    apply(idx.asUInt, Builder.forcedClock)
   }
 
+  /** Creates a read accessor into the memory with static addressing. See the
+    * class documentation of the memory for more detailed information.
+    */
+  def apply(x: Int): T = macro SourceInfoTransform.xArg
+
   /** @group SourceInfoTransformMacro */
   def do_apply(idx: Int)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T =
     do_apply(BigInt(idx))(sourceInfo, compileOptions)
@@ -85,7 +86,12 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
 
   /** @group SourceInfoTransformMacro */
   def do_apply(idx: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T =
-    makePort(sourceInfo, idx, MemPortDirection.INFER)
+    do_apply_impl(idx, Builder.forcedClock, MemPortDirection.INFER, true)
+
+  def apply(x: UInt, y: Clock): T = macro SourceInfoTransform.xyArg
+
+  def do_apply(idx: UInt, clock: Clock)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T =
+    do_apply_impl(idx, clock, MemPortDirection.INFER, false)
 
   /** Creates a read accessor into the memory with dynamic addressing. See the
     * class documentation of the memory for more detailed information.
@@ -94,16 +100,71 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
 
   /** @group SourceInfoTransformMacro */
   def do_read(idx: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T =
-    makePort(sourceInfo, idx, MemPortDirection.READ)
+    do_apply_impl(idx, Builder.forcedClock, MemPortDirection.READ, true)
+
+  /** Creates a read accessor into the memory with dynamic addressing.
+    * Takes a clock parameter to bind a clock that may be different
+    * from the implicit clock. See the class documentation of the memory
+    * for more detailed information.
+    */
+  def read(x: UInt, y: Clock): T = macro SourceInfoTransform.xyArg
+
+  /** @group SourceInfoTransformMacro */
+  def do_read(idx: UInt, clock: Clock)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T =
+    do_apply_impl(idx, clock, MemPortDirection.READ, false)
+
+  protected def do_apply_impl(
+    idx:   UInt,
+    clock: Clock,
+    dir:   MemPortDirection,
+    warn:  Boolean
+  )(
+    implicit sourceInfo: SourceInfo,
+    compileOptions:      CompileOptions
+  ): T = {
+    if (warn && clock != clockInst) {
+      Builder.warning(
+        "The clock used to initialize the memory is different than the one used to initialize the port. " +
+          "If this is intentional, please pass the clock explicitly when creating the port. This behavior will be an error in 3.6.0"
+      )
+    }
+    makePort(sourceInfo, idx, dir, clock)
+  }
 
   /** Creates a write accessor into the memory.
     *
     * @param idx memory element index to write into
     * @param data new data to write
     */
-  def write(idx: UInt, data: T)(implicit compileOptions: CompileOptions): Unit = {
+  def write(idx: UInt, data: T)(implicit compileOptions: CompileOptions): Unit =
+    write_impl(idx, data, Builder.forcedClock, true)
+
+  /** Creates a write accessor into the memory with a clock
+    * that may be different from the implicit clock.
+    *
+    * @param idx memory element index to write into
+    * @param data new data to write
+    * @param clock clock to bind to this accessor
+    */
+  def write(idx: UInt, data: T, clock: Clock)(implicit compileOptions: CompileOptions): Unit =
+    write_impl(idx, data, clock, false)
+
+  private def write_impl(
+    idx:   UInt,
+    data:  T,
+    clock: Clock,
+    warn:  Boolean
+  )(
+    implicit compileOptions: CompileOptions
+  ): Unit = {
+    if (warn && clock != clockInst) {
+      Builder.warning(
+        "The clock used to initialize the memory is different than the one used to initialize the port. " +
+          "If this is intentional, please pass the clock explicitly when creating the port. This behavior will be an error in 3.6.0"
+      )
+    }
     implicit val sourceInfo = UnlocatableSourceInfo
-    makePort(UnlocatableSourceInfo, idx, MemPortDirection.WRITE) := data
+    makePort(UnlocatableSourceInfo, idx, MemPortDirection.WRITE, clock) := data
   }
 
   /** Creates a masked write accessor into the memory.
@@ -122,9 +183,49 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
   )(
     implicit evidence: T <:< Vec[_],
     compileOptions:    CompileOptions
+  ): Unit =
+    masked_write_impl(idx, data, mask, Builder.forcedClock, true)
+
+  /** Creates a masked write accessor into the memory with a clock
+    * that may be different from the implicit clock.
+    *
+    * @param idx memory element index to write into
+    * @param data new data to write
+    * @param mask 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 clock clock to bind to this accessor
+    *
+    * @note this is only allowed if the memory's element data type is a Vec
+    */
+  def write(
+    idx:   UInt,
+    data:  T,
+    mask:  Seq[Bool],
+    clock: Clock
+  )(
+    implicit evidence: T <:< Vec[_],
+    compileOptions:    CompileOptions
+  ): Unit =
+    masked_write_impl(idx, data, mask, clock, false)
+
+  private def masked_write_impl(
+    idx:   UInt,
+    data:  T,
+    mask:  Seq[Bool],
+    clock: Clock,
+    warn:  Boolean
+  )(
+    implicit evidence: T <:< Vec[_],
+    compileOptions:    CompileOptions
   ): Unit = {
     implicit val sourceInfo = UnlocatableSourceInfo
-    val accessor = makePort(sourceInfo, idx, MemPortDirection.WRITE).asInstanceOf[Vec[Data]]
+    if (warn && clock != clockInst) {
+      Builder.warning(
+        "The clock used to initialize the memory is different than the one used to initialize the port. " +
+          "If this is intentional, please pass the clock explicitly when creating the port. This behavior will be an error in 3.6.0"
+      )
+    }
+    val accessor = makePort(sourceInfo, idx, MemPortDirection.WRITE, clock).asInstanceOf[Vec[Data]]
     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})")
@@ -139,7 +240,8 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
   private def makePort(
     sourceInfo: SourceInfo,
     idx:        UInt,
-    dir:        MemPortDirection
+    dir:        MemPortDirection,
+    clock:      Clock
   )(
     implicit compileOptions: CompileOptions
   ): T = {
@@ -147,7 +249,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
     val i = Vec.truncateIndex(idx, length)(sourceInfo, compileOptions)
 
     val port = pushCommand(
-      DefMemPort(sourceInfo, t.cloneTypeFull, Node(this), dir, i.ref, Builder.forcedClock.ref)
+      DefMemPort(sourceInfo, t.cloneTypeFull, Node(this), dir, i.ref, clock.ref)
     ).id
     // Bind each element of port to being a MemoryPort
     port.bind(MemoryPortBinding(Builder.forcedUserModule, Builder.currentWhen))
@@ -244,23 +346,44 @@ object SyncReadMem {
   */
 sealed class SyncReadMem[T <: Data] private (t: T, n: BigInt, val readUnderWrite: SyncReadMem.ReadUnderWrite)
     extends MemBase[T](t, n) {
+
+  override def read(x: UInt): T = macro SourceInfoTransform.xArg
+
+  /** @group SourceInfoTransformMacro */
+  override def do_read(idx: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T =
+    do_read(idx = idx, en = true.B)
+
   def read(x: UInt, en: Bool): T = macro SourceInfoTransform.xEnArg
 
   /** @group SourceInfoTransformMacro */
-  def do_read(addr: UInt, enable: Bool)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T = {
+  def do_read(idx: UInt, en: Bool)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T =
+    _read_impl(idx, en, Builder.forcedClock, true)
+
+  def read(idx: UInt, en: Bool, clock: Clock): T = macro SourceInfoTransform.xyzArg
+
+  /** @group SourceInfoTransformMacro */
+  def do_read(idx: UInt, en: Bool, clock: Clock)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T =
+    _read_impl(idx, en, clock, false)
+
+  /** @group SourceInfoTransformMacro */
+  private def _read_impl(
+    addr:   UInt,
+    enable: Bool,
+    clock:  Clock,
+    warn:   Boolean
+  )(
+    implicit sourceInfo: SourceInfo,
+    compileOptions:      CompileOptions
+  ): T = {
     val a = Wire(UInt())
     a := DontCare
     var port: Option[T] = None
     when(enable) {
       a := addr
-      port = Some(super.do_read(a))
+      port = Some(super.do_apply_impl(a, clock, MemPortDirection.READ, warn))
     }
     port.get
   }
-
-  /** @group SourceInfoTransformMacro */
-  override def do_read(idx: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions) =
-    do_read(addr = idx, enable = true.B)
   // 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)`
 }