From a737281f670aa34152ce971b57f926ecc9307a8c Mon Sep 17 00:00:00 2001
From: mergify[bot]
Date: Thu, 20 Jan 2022 19:44:41 +0000
Subject: Fix Compatibility Module io wrapping (#2355) (#2358)

The new reflection based IO autowrapping for compatibility mode Modules
would previously throw a NullPointerExceptions if any hardware were
constructed in the Module before "val io" was initialized. The logic is
now more robust for this case.

Co-authored-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
(cherry picked from commit 50e6099fbecc041973564514e55f67ffe069459b)

Co-authored-by: Jack Koenig <koenig@sifive.com>---
 core/src/main/scala/chisel3/BlackBox.scala  | 21 +++++++++-------
 core/src/main/scala/chisel3/RawModule.scala | 39 +++++++++++++++++------------
 2 files changed, 35 insertions(+), 25 deletions(-)

(limited to 'core/src')

diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala
index 89c4ccd3..2f04fb2e 100644
--- a/core/src/main/scala/chisel3/BlackBox.scala
+++ b/core/src/main/scala/chisel3/BlackBox.scala
@@ -145,27 +145,30 @@ abstract class BlackBox(
     extends BaseBlackBox {
 
   // Find a Record port named "io" for purposes of stripping the prefix
-  private[chisel3] lazy val _io: Record =
+  private[chisel3] lazy val _io: Option[Record] =
     this
       .findPort("io")
       .collect { case r: Record => r } // Must be a Record
-      .getOrElse(null) // null handling occurs in generateComponent
 
   // Allow access to bindings from the compatibility package
-  protected def _compatIoPortBound() = portsContains(_io)
+  protected def _compatIoPortBound() = _io.exists(portsContains(_))
 
   private[chisel3] override def generateComponent(): Option[Component] = {
     _compatAutoWrapPorts() // pre-IO(...) compatibility hack
 
     // Restrict IO to just io, clock, and reset
-    require(_io != null, "BlackBox must have a port named 'io' of type Record!")
-    require(portsContains(_io), "BlackBox must have io wrapped in IO(...)")
+    if (!_io.forall(portsContains)) {
+      throwException(s"BlackBox '$this' must have a port named 'io' of type Record wrapped in IO(...)!")
+    }
+
     require(portsSize == 1, "BlackBox must only have one IO, called `io`")
 
     require(!_closed, "Can't generate module more than once")
     _closed = true
 
-    val namedPorts = _io.elements.toSeq.reverse // ListMaps are stored in reverse order
+    val io = _io.get
+
+    val namedPorts = io.elements.toSeq.reverse // ListMaps are stored in reverse order
 
     // There is a risk of user improperly attempting to connect directly with io
     // Long term solution will be to define BlackBox IO differently as part of
@@ -181,18 +184,18 @@ abstract class BlackBox(
     // of the io bundle, but NOT on the io bundle itself.
     // Doing so would cause the wrong names to be assigned, since their parent
     // is now the module itself instead of the io bundle.
-    for (id <- getIds; if id ne _io) {
+    for (id <- getIds; if id ne io) {
       id._onModuleClose
     }
 
     val firrtlPorts = namedPorts.map { namedPort => Port(namedPort._2, namedPort._2.specifiedDirection) }
-    val component = DefBlackBox(this, name, firrtlPorts, _io.specifiedDirection, params)
+    val component = DefBlackBox(this, name, firrtlPorts, io.specifiedDirection, params)
     _component = Some(component)
     _component
   }
 
   private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = {
-    for ((_, port) <- _io.elements) {
+    for ((_, port) <- _io.map(_.elements).getOrElse(Nil)) {
       pushCommand(DefInvalid(UnlocatableSourceInfo, port.ref))
     }
   }
diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala
index b81372d8..12b6a76a 100644
--- a/core/src/main/scala/chisel3/RawModule.scala
+++ b/core/src/main/scala/chisel3/RawModule.scala
@@ -178,11 +178,12 @@ package object internal {
   // Private reflective version of "val io" to maintain Chisel.Module semantics without having
   // io as a virtual method. See https://github.com/freechipsproject/chisel3/pull/1550 for more
   // information about the removal of "val io"
-  private def reflectivelyFindValIO(self: BaseModule): Record = {
+  private def reflectivelyFindValIO(self: BaseModule): Option[Record] = {
     // Java reflection is faster and works for the common case
     def tryJavaReflect: Option[Record] = Try {
       self.getClass.getMethod("io").invoke(self).asInstanceOf[Record]
     }.toOption
+      .filter(_ != null)
     // Anonymous subclasses don't work with Java reflection, so try slower, Scala reflection
     def tryScalaReflect: Option[Record] = {
       val ru = scala.reflect.runtime.universe
@@ -199,6 +200,7 @@ package object internal {
         Try {
           im.reflectField(term).get.asInstanceOf[Record]
         }.toOption
+          .filter(_ != null)
       }
     }
 
@@ -209,13 +211,6 @@ package object internal {
         // Fallback if reflection fails, user can wrap in IO(...)
         self.findPort("io").collect { case r: Record => r }
       }
-      .getOrElse(
-        throwException(
-          s"Compatibility mode Module '$this' must have a 'val io' Bundle. " +
-            "If there is such a field and you still see this error, autowrapping has failed (sorry!). " +
-            "Please wrap the Bundle declaration in IO(...)."
-        )
-      )
   }
 
   /** Legacy Module class that restricts IOs to just io, clock, and reset, and provides a constructor
@@ -243,17 +238,29 @@ package object internal {
     def this(_clock: Clock, _reset: Bool)(implicit moduleCompileOptions: CompileOptions) =
       this(Option(_clock), Option(_reset))(moduleCompileOptions)
 
-    private lazy val _io: Record = reflectivelyFindValIO(this)
+    // Sort of a DIY lazy val because if the user tries to construct hardware before val io is
+    // constructed, _compatAutoWrapPorts will try to access it but it will be null
+    // In that case, we basically need to delay setting this var until later
+    private var _ioValue: Option[Record] = None
+    private def _io: Option[Record] = _ioValue.orElse {
+      _ioValue = reflectivelyFindValIO(this)
+      _ioValue
+    }
 
     // Allow access to bindings from the compatibility package
-    protected def _compatIoPortBound() = portsContains(_io)
+    protected def _compatIoPortBound() = _io.exists(portsContains(_))
 
     private[chisel3] override def generateComponent(): Option[Component] = {
       _compatAutoWrapPorts() // pre-IO(...) compatibility hack
 
       // Restrict IO to just io, clock, and reset
-      require(_io != null, "Module must have io")
-      require(portsContains(_io), "Module must have io wrapped in IO(...)")
+      if (_io.isEmpty || !_compatIoPortBound) {
+        throwException(
+          s"Compatibility mode Module '$this' must have a 'val io' Bundle. " +
+            "If there is such a field and you still see this error, autowrapping has failed (sorry!). " +
+            "Please wrap the Bundle declaration in IO(...)."
+        )
+      }
       require(
         (portsContains(clock)) && (portsContains(reset)),
         "Internal error, module did not have clock or reset as IO"
@@ -264,8 +271,8 @@ package object internal {
     }
 
     override def _compatAutoWrapPorts(): Unit = {
-      if (!_compatIoPortBound() && _io != null) {
-        _bindIoInPlace(_io)
+      if (!_compatIoPortBound()) {
+        _io.foreach(_bindIoInPlace(_))
       }
     }
   }
@@ -283,13 +290,13 @@ package object internal {
     implicit moduleCompileOptions: CompileOptions)
       extends chisel3.BlackBox(params) {
 
-    override private[chisel3] lazy val _io: Record = reflectivelyFindValIO(this)
+    override private[chisel3] lazy val _io: Option[Record] = reflectivelyFindValIO(this)
 
     // This class auto-wraps the BlackBox with IO(...), allowing legacy code (where IO(...) wasn't
     // required) to build.
     override def _compatAutoWrapPorts(): Unit = {
       if (!_compatIoPortBound()) {
-        _bindIoInPlace(_io)
+        _io.foreach(_bindIoInPlace(_))
       }
     }
   }
-- 
cgit v1.2.3


From ea1ced34b5c9e42412cc0ac3e7431cd3194ccbc3 Mon Sep 17 00:00:00 2001
From: mergify[bot]
Date: Tue, 1 Feb 2022 19:56:13 +0000
Subject: Chisel plugin bundle elements handler (#2306) (#2380)

Adds generation of `Bundle.elements` method to the chores done by the compiler plugin
For each `Bundle` find the relevant visible Chisel field members and construct a
hard-coded list of the elements and their names implemented as `_elementsImpl`

For more details: See plugins/README.md

- Should be no change in API
- Handles inheritance and mixins
- Handles Seq[Data]
- Tests in BundleElementSpec

Co-authored-by: chick <chick.markley@sifive.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>
(cherry picked from commit 237200a420581519f29149cbae9b3e968c0d01fc)

Co-authored-by: Chick Markley <chick@qrhino.com>---
 core/src/main/scala/chisel3/Aggregate.scala | 50 +++++++++++++++++++++++++++--
 core/src/main/scala/chisel3/Data.scala      |  2 +-
 2 files changed, 49 insertions(+), 3 deletions(-)

(limited to 'core/src')

diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala
index 15cdf428..75966a16 100644
--- a/core/src/main/scala/chisel3/Aggregate.scala
+++ b/core/src/main/scala/chisel3/Aggregate.scala
@@ -1208,12 +1208,58 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
     * }}}
     */
   final lazy val elements: SeqMap[String, Data] = {
+    val hardwareFields = _elementsImpl.flatMap {
+      case (name, data: Data) =>
+        if (data.isSynthesizable) {
+          Some(s"$name: $data")
+        } else {
+          None
+        }
+      case (name, Some(data: Data)) =>
+        if (data.isSynthesizable) {
+          Some(s"$name: $data")
+        } else {
+          None
+        }
+      case (name, s: scala.collection.Seq[Any]) if s.nonEmpty =>
+        s.head match {
+          // Ignore empty Seq()
+          case d: Data =>
+            throwException(
+              "Public Seq members cannot be used to define Bundle elements " +
+                s"(found public Seq member '${name}'). " +
+                "Either use a Vec if all elements are of the same type, or MixedVec if the elements " +
+                "are of different types. If this Seq member is not intended to construct RTL, mix in the trait " +
+                "IgnoreSeqInBundle."
+            )
+          case _ => // don't care about non-Data Seq
+        }
+        None
+
+      case _ => None
+    }
+    if (hardwareFields.nonEmpty) {
+      throw ExpectedChiselTypeException(s"Bundle: $this contains hardware fields: " + hardwareFields.mkString(","))
+    }
+    VectorMap(_elementsImpl.toSeq.flatMap {
+      case (name, data: Data) =>
+        Some(name -> data)
+      case (name, Some(data: Data)) =>
+        Some(name -> data)
+      case _ => None
+    }.sortWith {
+      case ((an, a), (bn, b)) => (a._id > b._id) || ((a eq b) && (an > bn))
+    }: _*)
+  }
+  /*
+   * This method will be overwritten by the Chisel-Plugin
+   */
+  protected def _elementsImpl: SeqMap[String, Any] = {
     val nameMap = LinkedHashMap[String, Data]()
     for (m <- getPublicFields(classOf[Bundle])) {
       getBundleField(m) match {
         case Some(d: Data) =>
-          requireIsChiselType(d)
-
+          // Checking for chiselType is done in elements method
           if (nameMap contains m.getName) {
             require(nameMap(m.getName) eq d)
           } else {
diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala
index 9e9f5dbb..ef43a3b0 100644
--- a/core/src/main/scala/chisel3/Data.scala
+++ b/core/src/main/scala/chisel3/Data.scala
@@ -480,7 +480,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
   protected[chisel3] def binding: Option[Binding] = _binding
   protected def binding_=(target: Binding) {
     if (_binding.isDefined) {
-      throw RebindingException(s"Attempted reassignment of binding to $this")
+      throw RebindingException(s"Attempted reassignment of binding to $this, from: ${target}")
     }
     _binding = Some(target)
   }
-- 
cgit v1.2.3


From cb77b061e835044b4f3a2b718fb7ce3971b5d06e Mon Sep 17 00:00:00 2001
From: mergify[bot]
Date: Tue, 1 Feb 2022 20:55:35 +0000
Subject: 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>---
 core/src/main/scala/chisel3/Mem.scala | 161 ++++++++++++++++++++++++++++++----
 1 file changed, 142 insertions(+), 19 deletions(-)

(limited to 'core/src')

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)`
 }
-- 
cgit v1.2.3


From 0eba2eb109910c1e4b980ccfc7f3ae85a8d0f223 Mon Sep 17 00:00:00 2001
From: mergify[bot]
Date: Tue, 1 Feb 2022 21:37:08 +0000
Subject: Improve error reporting (backport #2376) (#2379)

* Improve error reporting (#2376)

* Do not trim stack traces of exceptions with no stack trace

This prevents us from accidentally giving stack traces to exceptions
that don't have them and giving misleading messages telling users to use
--full-stacktrace when it won't actually do anything.

Also deprecate ChiselException.chiselStackTrace which is no longer being
used anywhere in this codebase.

* Add exception class for multiple-errors reported

New chisel3.internal.Errors replaces old anonymous class that would show
up as chisel3.internal.ErrorLog$$anon$1 in error messages.

* Add new option --throw-on-first-error

This tells Chisel not to aggregate recoverable errors but instead to
throw an exception on the first one. This gives a stack trace for users
who need it for debugging.

(cherry picked from commit ff2e9c92247b3848659fa09fdd53ddde2120036a)

* Waive MiMa false positives

The waived change is to a package private constructor.

Co-authored-by: Jack Koenig <koenig@sifive.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>---
 .../experimental/hierarchy/Definition.scala        |  2 +-
 core/src/main/scala/chisel3/internal/Builder.scala |  5 +-
 core/src/main/scala/chisel3/internal/Error.scala   | 72 +++++++++++++---------
 3 files changed, 47 insertions(+), 32 deletions(-)

(limited to 'core/src')

diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala b/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala
index b498daf0..59b4c692 100644
--- a/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala
+++ b/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala
@@ -100,7 +100,7 @@ object Definition extends SourceInfoDoc {
     implicit sourceInfo: SourceInfo,
     compileOptions:      CompileOptions
   ): Definition[T] = {
-    val dynamicContext = new DynamicContext(Nil)
+    val dynamicContext = new DynamicContext(Nil, Builder.captureContext().throwOnFirstError)
     Builder.globalNamespace.copyTo(dynamicContext.globalNamespace)
     dynamicContext.inDefinition = true
     val (ir, module) = Builder.build(Module(proto), dynamicContext, false)
diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index fb6ebcc7..a00505a8 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -361,7 +361,7 @@ private[chisel3] class ChiselContext() {
   val viewNamespace = Namespace.empty
 }
 
-private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq) {
+private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) {
   val globalNamespace = Namespace.empty
   val components = ArrayBuffer[Component]()
   val annotations = ArrayBuffer[ChiselAnnotation]()
@@ -653,7 +653,8 @@ private[chisel3] object Builder extends LazyLogging {
 
   def errors: ErrorLog = dynamicContext.errors
   def error(m: => String): Unit = {
-    if (dynamicContextVar.value.isDefined) {
+    // If --throw-on-first-error is requested, throw an exception instead of aggregating errors
+    if (dynamicContextVar.value.isDefined && !dynamicContextVar.value.get.throwOnFirstError) {
       errors.error(m)
     } else {
       throwException(m)
diff --git a/core/src/main/scala/chisel3/internal/Error.scala b/core/src/main/scala/chisel3/internal/Error.scala
index c42f39ed..62086870 100644
--- a/core/src/main/scala/chisel3/internal/Error.scala
+++ b/core/src/main/scala/chisel3/internal/Error.scala
@@ -4,6 +4,7 @@ package chisel3.internal
 
 import scala.annotation.tailrec
 import scala.collection.mutable.{ArrayBuffer, LinkedHashMap}
+import scala.util.control.NoStackTrace
 import _root_.logger.Logger
 
 object ExceptionHelpers {
@@ -46,31 +47,36 @@ object ExceptionHelpers {
       }
 
       // Step 1: Remove elements from the top in the package trimlist
-      ((a: Array[StackTraceElement]) => a.dropWhile(inTrimlist))
-        // Step 2: Optionally remove elements from the bottom until the anchor
-        .andThen(_.reverse)
-        .andThen(a =>
-          anchor match {
-            case Some(b) => a.dropWhile(ste => !ste.getClassName.startsWith(b))
-            case None    => a
-          }
-        )
-        // Step 3: Remove elements from the bottom in the package trimlist
-        .andThen(_.dropWhile(inTrimlist))
-        // Step 4: Reverse back to the original order
-        .andThen(_.reverse.toArray)
-        // Step 5: Add ellipsis stack trace elements and "--full-stacktrace" info
-        .andThen(a =>
-          ellipsis() +:
-            a :+
-            ellipsis() :+
-            ellipsis(
-              Some("Stack trace trimmed to user code only. Rerun with --full-stacktrace to see the full stack trace")
-            )
-        )
-        // Step 5: Mutate the stack trace in this exception
-        .andThen(throwable.setStackTrace(_))
-        .apply(throwable.getStackTrace)
+      val trimStackTrace =
+        ((a: Array[StackTraceElement]) => a.dropWhile(inTrimlist))
+          // Step 2: Optionally remove elements from the bottom until the anchor
+          .andThen(_.reverse)
+          .andThen(a =>
+            anchor match {
+              case Some(b) => a.dropWhile(ste => !ste.getClassName.startsWith(b))
+              case None    => a
+            }
+          )
+          // Step 3: Remove elements from the bottom in the package trimlist
+          .andThen(_.dropWhile(inTrimlist))
+          // Step 4: Reverse back to the original order
+          .andThen(_.reverse.toArray)
+          // Step 5: Add ellipsis stack trace elements and "--full-stacktrace" info
+          .andThen(a =>
+            ellipsis() +:
+              a :+
+              ellipsis() :+
+              ellipsis(
+                Some("Stack trace trimmed to user code only. Rerun with --full-stacktrace to see the full stack trace")
+              )
+          )
+          // Step 5: Mutate the stack trace in this exception
+          .andThen(throwable.setStackTrace(_))
+
+      val stackTrace = throwable.getStackTrace
+      if (stackTrace.nonEmpty) {
+        trimStackTrace(stackTrace)
+      }
     }
 
   }
@@ -80,11 +86,11 @@ object ExceptionHelpers {
 class ChiselException(message: String, cause: Throwable = null) extends Exception(message, cause, true, true) {
 
   /** Package names whose stack trace elements should be trimmed when generating a trimmed stack trace */
-  @deprecated("Use ExceptionHelpers.packageTrimlist. This will be removed in Chisel 3.6", "3.5")
+  @deprecated("Use ExceptionHelpers.packageTrimlist. This will be removed in Chisel 3.6", "Chisel 3.5")
   val blacklistPackages: Set[String] = Set("chisel3", "scala", "java", "sun", "sbt")
 
   /** The object name of Chisel's internal `Builder`. Everything stack trace element after this will be trimmed. */
-  @deprecated("Use ExceptionHelpers.builderName. This will be removed in Chisel 3.6", "3.5")
+  @deprecated("Use ExceptionHelpers.builderName. This will be removed in Chisel 3.6", "Chisel 3.5")
   val builderName: String = chisel3.internal.Builder.getClass.getName
 
   /** Examine a [[Throwable]], to extract all its causes. Innermost cause is first.
@@ -138,6 +144,11 @@ class ChiselException(message: String, cause: Throwable = null) extends Exceptio
     trimmedReverse.toIndexedSeq.reverse.toArray
   }
 
+  @deprecated(
+    "Use extension method trimStackTraceToUserCode defined in ExceptionHelpers.ThrowableHelpers. " +
+      "This will be removed in Chisel 3.6",
+    "Chisel 3.5.0"
+  )
   def chiselStackTrace: String = {
     val trimmed = trimmedStackTrace(likelyCause)
 
@@ -151,6 +162,7 @@ class ChiselException(message: String, cause: Throwable = null) extends Exceptio
     sw.toString
   }
 }
+private[chisel3] class Errors(message: String) extends ChiselException(message) with NoStackTrace
 
 private[chisel3] object throwException {
   def apply(s: String, t: Throwable = null): Nothing =
@@ -234,8 +246,10 @@ private[chisel3] class ErrorLog {
     }
 
     if (!allErrors.isEmpty) {
-      throw new ChiselException("Fatal errors during hardware elaboration. Look above for error list.")
-        with scala.util.control.NoStackTrace
+      throw new Errors(
+        "Fatal errors during hardware elaboration. Look above for error list. " +
+          "Rerun with --throw-on-first-error if you wish to see a stack trace."
+      )
     } else {
       // No fatal errors, clear accumulated warnings since they've been reported
       errors.clear()
-- 
cgit v1.2.3


From 6048c973f0c1c6e80a7a9e8ef6cec71ef0695e68 Mon Sep 17 00:00:00 2001
From: mergify[bot]
Date: Thu, 3 Feb 2022 02:42:14 +0000
Subject: Tweak new mem port clock warnings (#2389) (#2391)

Use Builder.deprecated instead of Builder.warning so that the warnings
are aggregated by source locator to prevent spamming the screen with
duplicated warnings.

(cherry picked from commit 538e223ae81c8b66a4123303f6dab61c874aaa1e)

Co-authored-by: Jack Koenig <koenig@sifive.com>---
 core/src/main/scala/chisel3/Mem.scala | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

(limited to 'core/src')

diff --git a/core/src/main/scala/chisel3/Mem.scala b/core/src/main/scala/chisel3/Mem.scala
index 4b7c0815..3f37308c 100644
--- a/core/src/main/scala/chisel3/Mem.scala
+++ b/core/src/main/scala/chisel3/Mem.scala
@@ -9,7 +9,7 @@ import firrtl.{ir => fir}
 import chisel3.internal._
 import chisel3.internal.Builder.pushCommand
 import chisel3.internal.firrtl._
-import chisel3.internal.sourceinfo.{MemTransform, SourceInfo, SourceInfoTransform, UnlocatableSourceInfo}
+import chisel3.internal.sourceinfo.{MemTransform, SourceInfo, SourceInfoTransform, SourceLine, UnlocatableSourceInfo}
 
 object Mem {
 
@@ -57,6 +57,16 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
   _parent.foreach(_.addId(this))
 
   private val clockInst: Clock = Builder.forcedClock
+
+  protected def clockWarning(sourceInfo: Option[SourceInfo]): Unit = {
+    // Turn into pretty String if possible, if not, Builder.deprecated will find one via stack trace
+    val infoStr = sourceInfo.collect { case SourceLine(file, line, col) => s"$file:$line:$col" }
+    Builder.deprecated(
+      "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",
+      infoStr
+    )
+  }
   // REVIEW TODO: make accessors (static/dynamic, read/write) combinations consistent.
 
   /** Creates a read accessor into the memory with static addressing. See the
@@ -123,10 +133,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
     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"
-      )
+      clockWarning(Some(sourceInfo))
     }
     makePort(sourceInfo, idx, dir, clock)
   }
@@ -158,10 +165,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
     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"
-      )
+      clockWarning(None)
     }
     implicit val sourceInfo = UnlocatableSourceInfo
     makePort(UnlocatableSourceInfo, idx, MemPortDirection.WRITE, clock) := data
@@ -220,10 +224,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt)
   ): Unit = {
     implicit val sourceInfo = UnlocatableSourceInfo
     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"
-      )
+      clockWarning(None)
     }
     val accessor = makePort(sourceInfo, idx, MemPortDirection.WRITE, clock).asInstanceOf[Vec[Data]]
     val dataVec = data.asInstanceOf[Vec[Data]]
-- 
cgit v1.2.3


From 8776e58ff91cd88562b957d7a09322ec16610b81 Mon Sep 17 00:00:00 2001
From: mergify[bot]
Date: Thu, 3 Feb 2022 04:23:48 +0000
Subject: Tweak Bundle._elementsImpl (#2390) (#2392)

* Change type of Bundle._elementsImpl to Iterable

It was previously SeqMap (ListMap on Scala 2.12). This change gives us
more freedom to optimize the implementation without breaking binary
compatibility. It is scala.collection.Iterable because it is perfectly
fine to return mutable collections (like Arrays) since the only use is
to Iterate on them.

* Disallow users implementing Bundle._elementsImpl

Currently, it would result in a runtime linkage error. This turns it
into a compile-time error.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 1b05a14ad6d5784f3b91ab510dc1095423c23ea8)

Co-authored-by: Jack Koenig <koenig@sifive.com>---
 core/src/main/scala/chisel3/Aggregate.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'core/src')

diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala
index 75966a16..ec64f28b 100644
--- a/core/src/main/scala/chisel3/Aggregate.scala
+++ b/core/src/main/scala/chisel3/Aggregate.scala
@@ -1254,7 +1254,7 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
   /*
    * This method will be overwritten by the Chisel-Plugin
    */
-  protected def _elementsImpl: SeqMap[String, Any] = {
+  protected def _elementsImpl: Iterable[(String, Any)] = {
     val nameMap = LinkedHashMap[String, Data]()
     for (m <- getPublicFields(classOf[Bundle])) {
       getBundleField(m) match {
-- 
cgit v1.2.3


From dd5c643ae86c234041092efee4386cc82e59331d Mon Sep 17 00:00:00 2001
From: mergify[bot]
Date: Fri, 4 Feb 2022 02:26:06 +0000
Subject: Fix bundle elements performance regression (#2396) (#2398)

* Only call _elementsImpl once in Bundle.elements

* Distinguish compiler plugin and reflective _elementsImpl

Bundle.elements now will only do post-processing if the user is using
plugin-generated _elementsImpl. This improves performance for the case
where the user does not opt-in to using the plugin to generate
_elementsImpl.

(cherry picked from commit 5fead89ee0132355e551bcb6b87cc2a6db679bee)

Co-authored-by: Jack Koenig <koenig@sifive.com>---
 core/src/main/scala/chisel3/Aggregate.scala | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

(limited to 'core/src')

diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala
index ec64f28b..06ae36f3 100644
--- a/core/src/main/scala/chisel3/Aggregate.scala
+++ b/core/src/main/scala/chisel3/Aggregate.scala
@@ -1207,8 +1207,19 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
     *   assert(uint === "h12345678".U) // This will pass
     * }}}
     */
-  final lazy val elements: SeqMap[String, Data] = {
-    val hardwareFields = _elementsImpl.flatMap {
+  final lazy val elements: SeqMap[String, Data] =
+    // _elementsImpl is a method, only call it once
+    _elementsImpl match {
+      // Those not using plugin-generated _elementsImpl use the old reflective implementation
+      case oldElements: VectorMap[_, _] => oldElements.asInstanceOf[VectorMap[String, Data]]
+      // Plugin-generated _elementsImpl are incomplete and need some processing
+      case rawElements => _processRawElements(rawElements)
+    }
+
+  // The compiler plugin is imperfect at picking out elements statically so we process at runtime
+  // checking for errors and filtering out mistakes
+  private def _processRawElements(rawElements: Iterable[(String, Any)]): SeqMap[String, Data] = {
+    val hardwareFields = rawElements.flatMap {
       case (name, data: Data) =>
         if (data.isSynthesizable) {
           Some(s"$name: $data")
@@ -1241,7 +1252,7 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
     if (hardwareFields.nonEmpty) {
       throw ExpectedChiselTypeException(s"Bundle: $this contains hardware fields: " + hardwareFields.mkString(","))
     }
-    VectorMap(_elementsImpl.toSeq.flatMap {
+    VectorMap(rawElements.toSeq.flatMap {
       case (name, data: Data) =>
         Some(name -> data)
       case (name, Some(data: Data)) =>
@@ -1251,15 +1262,17 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
       case ((an, a), (bn, b)) => (a._id > b._id) || ((a eq b) && (an > bn))
     }: _*)
   }
-  /*
-   * This method will be overwritten by the Chisel-Plugin
+
+  /* The old, reflective implementation of Bundle.elements
+   * This method is optionally overwritten by the compiler plugin for much better performance
    */
   protected def _elementsImpl: Iterable[(String, Any)] = {
     val nameMap = LinkedHashMap[String, Data]()
     for (m <- getPublicFields(classOf[Bundle])) {
       getBundleField(m) match {
         case Some(d: Data) =>
-          // Checking for chiselType is done in elements method
+          requireIsChiselType(d)
+
           if (nameMap contains m.getName) {
             require(nameMap(m.getName) eq d)
           } else {
-- 
cgit v1.2.3


From 9d1e2082df4ecb2942a28b7039eb2ff36953380c Mon Sep 17 00:00:00 2001
From: mergify[bot]
Date: Fri, 4 Feb 2022 04:38:30 +0000
Subject: Fix variable-name typo (#2397) (#2400)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 015755749caa8a05f3809d446b023df80c7419d1)

Co-authored-by: Tynan McAuley <tynan@galois.com>---
 core/src/main/scala/chisel3/internal/Builder.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'core/src')

diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index a00505a8..247be57a 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -504,7 +504,7 @@ private[chisel3] object Builder extends LazyLogging {
   def getPrefix: Prefix = chiselContext.get().prefixStack
 
   def currentModule: Option[BaseModule] = dynamicContextVar.value match {
-    case Some(dyanmicContext) => dynamicContext.currentModule
+    case Some(dynamicContext) => dynamicContext.currentModule
     case _                    => None
   }
   def currentModule_=(target: Option[BaseModule]): Unit = {
-- 
cgit v1.2.3