Merge branch '3.6.x' into 3.6-release

chipsalliance · Apr 5, 2023 · a123880 · a123880
2 parents 9f3bef5 + 05be8a2
commit a123880
Show file tree

Hide file tree

Showing 33 changed files with 305 additions and 167 deletions.
diff --git a/.github/workflows/install-circt/action.yml b/.github/workflows/install-circt/action.yml
@@ -4,7 +4,7 @@ inputs:
   version:
     description: 'version to install'
     required: false
-    default: 'firtool-1.31.0'
+    default: 'firtool-1.37.0'
 
 runs:
   using: composite
@@ -19,7 +19,7 @@ runs:
       if: steps.cache-circt.outputs.cache-hit != 'true'
       run: |
         mkdir circt
-        wget -q -O - https://github.com/llvm/circt/releases/download/${{ inputs.version }}/circt-bin-ubuntu-20.04.tar.gz | tar -zx -C circt/
+        wget -q -O - https://github.com/llvm/circt/releases/download/${{ inputs.version }}/firrtl-bin-ubuntu-20.04.tar.gz | tar -zx -C circt/ --strip-components 1
 
     - shell: bash
       run: echo "$(pwd)/circt/bin" >> $GITHUB_PATH
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -5,6 +5,7 @@ on:
   push:
     branches:
       - master
+      - 3.6.x
       - 3.5.x
       - 3.4.x
       - 3.3.x
@@ -19,7 +20,7 @@ jobs:
         jvm: ["8"]
         scala: ["2.13.10", "2.12.17"]
         espresso: ["2.4"]
-        circt: ["firtool-1.31.0"]
+        circt: ["firtool-1.37.0"]
     runs-on: ${{ matrix.system }}
 
     steps:
@@ -61,7 +62,7 @@ jobs:
       - name: Install CIRCT
         uses: ./.github/workflows/install-circt
       - name: Check Formatting
-        run: sbt scalafmtCheckAll
+        run: sbt fmtCheck
 
   integration:
     name: Integration Tests (w/ chiseltest)
@@ -189,7 +190,7 @@ jobs:
   publish:
     needs: [all_tests_passed]
     runs-on: ubuntu-20.04
-    if: (github.event_name == 'push' && github.ref_name != 'website-test')
+    if: github.event_name == 'push'
 
     steps:
       - name: Checkout
@@ -210,22 +211,3 @@ jobs:
           SONATYPE_PASSWORD: ${{ secrets.SONATYPE_PASSWORD }}
           SONATYPE_USERNAME: ${{ secrets.SONATYPE_USERNAME }}
 
-
-  deploy_website:
-    name: Deploy Website
-    runs-on: ubuntu-latest
-    needs: [all_tests_passed]
-    if: (github.event_name == 'push') && (github.ref_type == 'branch') && ((github.ref_name == 'website-test') || (github.ref_name == 'master'))
-    steps:
-      - name: Download built website
-        uses: actions/download-artifact@v3
-        with:
-          name: website
-      - name: Untar built website
-        run: tar zxf website.tar.gz
-      - name: Deploy Website to GitHub Pages (From Master Branch)
-        uses: JamesIves/github-pages-deploy-action@3.7.1
-        with:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          BRANCH: gh-pages
-          FOLDER: website/docs/target/site
diff --git a/README.md b/README.md
@@ -55,7 +55,7 @@ Chisel is powered by [FIRRTL (Flexible Intermediate Representation for RTL)](htt
 [![Join the chat at https://gitter.im/freechipsproject/chisel3](https://badges.gitter.im/chipsalliance/chisel3.svg)](https://gitter.im/freechipsproject/chisel3?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
 ![CI](https://github.com/chipsalliance/chisel3/actions/workflows/test.yml/badge.svg)
 [![GitHub tag (latest SemVer)](https://img.shields.io/github/v/tag/chipsalliance/chisel3.svg?include_prereleases&sort=semver)](https://github.com/chipsalliance/chisel3/releases/latest)
-[![Scala version support](https://index.scala-lang.org/chipsalliance/chisel3/chisel3/latest-by-scala-version.svg?color=blue)](https://index.scala-lang.org/chipsalliance/chisel3/chisel3)
+[![chisel3 Scala version support](https://index.scala-lang.org/chipsalliance/chisel/chisel3/latest-by-scala-version.svg?platform=jvm)](https://index.scala-lang.org/chipsalliance/chisel/chisel3)
 [![Sonatype Snapshots](https://img.shields.io/nexus/s/edu.berkeley.cs/chisel3_2.13?server=https%3A%2F%2Foss.sonatype.org)](https://oss.sonatype.org/content/repositories/snapshots/edu/berkeley/cs/)
 [![Scaladoc](https://www.javadoc.io/badge/edu.berkeley.cs/chisel3_2.13.svg?color=blue&label=Scaladoc)](https://javadoc.io/doc/edu.berkeley.cs/chisel3_2.13/latest)
 
@@ -392,4 +392,4 @@ You are encouraged to do your development against the latest SNAPSHOT, but note
 
 ### Roadmap
 
-See (Roadmap)[https://github.com/chipsalliance/chisel3/blob/master/README.md].
+See [Roadmap](https://github.com/chipsalliance/chisel3/blob/master/ROADMAP.md).
diff --git a/build.sbt b/build.sbt
@@ -2,6 +2,9 @@
 
 enablePlugins(SiteScaladocPlugin)
 
+addCommandAlias("fmt", "; scalafmtAll ; scalafmtSbt")
+addCommandAlias("fmtCheck", "; scalafmtCheckAll ; scalafmtSbtCheck")
+
 val defaultVersions = Map(
   "firrtl" -> "edu.berkeley.cs" %% "firrtl" % "1.6.0-RC3",
   "treadle" -> "edu.berkeley.cs" %% "treadle" % "1.6.0-RC3",
@@ -40,13 +43,13 @@ lazy val fatalWarningsSettings = Seq(
   scalacOptions ++= {
     CrossVersion.partialVersion(scalaVersion.value) match {
       case Some((2, n)) if n >= 13 =>
-          if (sys.props.contains("disableFatalWarnings")) {
-            Nil
-          } else {
-            "-Werror" :: Nil
-          }
+        if (sys.props.contains("disableFatalWarnings")) {
+          Nil
+        } else {
+          "-Werror" :: Nil
+        }
 
-      case _                       => Nil
+      case _ => Nil
     }
   }
 )
@@ -56,7 +59,8 @@ lazy val warningSuppression = Seq(
     "msg=APIs in chisel3.internal:s",
     "msg=Importing from firrtl:s",
     "msg=migration to the MLIR:s",
-    "msg=method hasDefiniteSize in trait IterableOnceOps is deprecated:s",  // replacement `knownSize` is not in 2.12
+    "cat=deprecation&origin=chisel3\\.ImplicitInvalidate:s",
+    "msg=method hasDefiniteSize in trait IterableOnceOps is deprecated:s", // replacement `knownSize` is not in 2.12
     "msg=object JavaConverters in package collection is deprecated:s",
     "msg=undefined in comment for method cf in class PrintableHelper:s"
   ).mkString(",")

diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala
@@ -25,19 +25,17 @@ class AliasedAggregateFieldException(message: String) extends chisel3.ChiselExce
   */
 sealed abstract class Aggregate extends Data {
 
-  /** Return an Aggregate's literal value if it is a literal, None otherwise.
-    * If any element of the aggregate is not a literal with a defined width, the result isn't a literal.
-    *
-    * @return an Aggregate's literal value if it is a literal.
-    */
-  override def litOption: Option[BigInt] = {
+  private def checkingLitOption(checkForDontCares: Boolean): Option[BigInt] = {
     // Shift the accumulated value by our width and add in our component, masked by our width.
     def shiftAdd(accumulator: Option[BigInt], elt: Data): Option[BigInt] = {
       (accumulator, elt.litOption) match {
         case (Some(accumulator), Some(eltLit)) =>
           val width = elt.width.get
           val masked = ((BigInt(1) << width) - 1) & eltLit // also handles the negative case with two's complement
           Some((accumulator << width) + masked)
+        case (Some(accumulator), None) if checkForDontCares =>
+          Builder.error(s"Called litValue on aggregate $this contains DontCare")(UnlocatableSourceInfo)
+          None
         case (None, _) => None
         case (_, None) => None
       }
@@ -51,6 +49,19 @@ sealed abstract class Aggregate extends Data {
     }
   }
 
+  /** Return an Aggregate's literal value if it is a literal, None otherwise.
+    * If any element of the aggregate is not a literal with a defined width, the result isn't a literal.
+    *
+    * @return an Aggregate's literal value if it is a literal.
+    */
+  override def litOption: Option[BigInt] = {
+    checkingLitOption(checkForDontCares = false)
+  }
+
+  override def litValue: BigInt = {
+    checkingLitOption(checkForDontCares = true).get
+  }
+
   /** Returns a Seq of the immediate contents of this Aggregate, in order.
     */
   def getElements: Seq[Data]
@@ -941,8 +952,8 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
   }
 
   private def checkClone(clone: Record): Unit = {
-    for ((name, field) <- elements) {
-      if (clone.elements(name) eq field) {
+    for ((name, field) <- _elements) {
+      if (clone._elements(name) eq field) {
         throw new AutoClonetypeException(
           s"The bundle plugin was unable to clone $clone that has field '$name' aliased with base $this." +
             "This likely happened because you tried nesting Data arguments inside of other data structures." +
@@ -969,10 +980,10 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
     // which can cause collisions
     val _namespace = Namespace.empty
     require(
-      !opaqueType || (elements.size == 1 && elements.head._1 == ""),
-      s"Opaque types must have exactly one element with an empty name, not ${elements.size}: ${elements.keys.mkString(", ")}"
+      !opaqueType || (_elements.size == 1 && _elements.head._1 == ""),
+      s"Opaque types must have exactly one element with an empty name, not ${_elements.size}: ${elements.keys.mkString(", ")}"
     )
-    for ((name, elt) <- elements) {
+    for ((name, elt) <- _elements) {
       elt.setRef(this, _namespace.name(name, leadingDigitOk = true), opaque = opaqueType)
     }
   }
@@ -996,7 +1007,7 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
       val dupNames = duplicates.toSeq
         .sortBy(_._id)
         .map { duplicate =>
-          this.elements.collect { case x if x._2._id == duplicate._id => x }.toSeq
+          this._elements.collect { case x if x._2._id == duplicate._id => x }.toSeq
             .sortBy(_._2._id)
             .map(_._1)
             .reverse
@@ -1017,7 +1028,12 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
 
     checkForAndReportDuplicates()
 
-    for ((child, sameChild) <- this.elementsIterator.zip(this.elementsIterator)) {
+    // This check is for making sure that elements always returns the
+    // same object, which will not be the case if the user makes it a
+    // def inside the Record. Checking elementsIterator against itself
+    // is not useful for this check because it's a lazy val which will
+    // always return the same thing.
+    for (((_, child), sameChild) <- this.elements.iterator.zip(this.elementsIterator)) {
       if (child != sameChild) {
         throwException(
           s"${this.className} does not return the same objects when calling .elements multiple times. Did you make it a def by mistake?"
@@ -1177,7 +1193,7 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
   override def toString: String = {
     topBindingOpt match {
       case Some(BundleLitBinding(_)) =>
-        val contents = elements.toList.reverse.map {
+        val contents = _elements.toList.reverse.map {
           case (name, data) =>
             s"$name=$data"
         }.mkString(", ")
@@ -1188,6 +1204,22 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
 
   def elements: SeqMap[String, Data]
 
+  // Internal representation of Record elements. _elements makes it
+  // possible to check for rebinding issues with Record elements
+  // without having to recurse over all elements after the Record is
+  // constructed. Laziness of _elements means that this check will
+  // occur (only) at the first instance _elements is referenced.
+  private[chisel3] lazy val _elements: SeqMap[String, Data] = {
+    for ((name, field) <- elements) {
+      if (field.binding.isDefined) {
+        throw RebindingException(
+          s"Cannot create Record ${this.className}; element ${field} of Record must be a Chisel type, not hardware."
+        )
+      }
+    }
+    elements
+  }
+
   /** Name for Pretty Printing */
   def className: String = try {
     this.getClass.getSimpleName
@@ -1199,11 +1231,11 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
   private[chisel3] override def typeEquivalent(that: Data): Boolean = that match {
     case that: Record =>
       this.getClass == that.getClass &&
-        this.elements.size == that.elements.size &&
-        this.elements.forall {
+        this._elements.size == that._elements.size &&
+        this._elements.forall {
           case (name, model) =>
-            that.elements.contains(name) &&
-              (that.elements(name).typeEquivalent(model))
+            that._elements.contains(name) &&
+              (that._elements(name).typeEquivalent(model))
         }
     case _ => false
   }
@@ -1212,7 +1244,7 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
 
   override def getElements: Seq[Data] = elementsIterator.toIndexedSeq
 
-  final override private[chisel3] def elementsIterator: Iterator[Data] = elements.iterator.map(_._2)
+  final override private[chisel3] def elementsIterator: Iterator[Data] = _elements.iterator.map(_._2)
 
   // Helper because Bundle elements are reversed before printing
   private[chisel3] def toPrintableHelper(elts: Seq[(String, Data)]): Printable = {
@@ -1230,7 +1262,7 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
     * Analogous to printing a Map
     * Results in "`\$className(elt0.name -> elt0.value, ...)`"
     */
-  def toPrintable: Printable = toPrintableHelper(elements.toList)
+  def toPrintable: Printable = toPrintableHelper(_elements.toList)
 
   /** Implementation of cloneType that is [optionally for Record] overridden by the compiler plugin
     *
@@ -1434,5 +1466,5 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
     * @note The order is reversed from the order of elements in order to print
     *   the fields in the order they were defined
     */
-  override def toPrintable: Printable = toPrintableHelper(elements.toList.reverse)
+  override def toPrintable: Printable = toPrintableHelper(_elements.toList.reverse)
 }
diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala
@@ -219,7 +219,7 @@ private[chisel3] object cloneSupertype {
 private[chisel3] object getRecursiveFields {
   def noPath(data: Data): Seq[Data] = data match {
     case data: Record =>
-      data.elements.map {
+      data._elements.map {
         case (_, fieldData) =>
           getRecursiveFields.noPath(fieldData)
       }.fold(Seq(data)) {
@@ -236,7 +236,7 @@ private[chisel3] object getRecursiveFields {
   }
   def apply(data: Data, path: String): Seq[(Data, String)] = data match {
     case data: Record =>
-      data.elements.map {
+      data._elements.map {
         case (fieldName, fieldData) =>
           getRecursiveFields(fieldData, s"$path.$fieldName")
       }.fold(Seq(data -> path)) {
@@ -255,7 +255,7 @@ private[chisel3] object getRecursiveFields {
   def lazily(data: Data, path: String): Seq[(Data, String)] = data match {
     case data: Record =>
       LazyList(data -> path) ++
-        data.elements.view.flatMap {
+        data._elements.view.flatMap {
           case (fieldName, fieldData) =>
             getRecursiveFields(fieldData, s"$path.$fieldName")
         }
@@ -270,7 +270,7 @@ private[chisel3] object getRecursiveFields {
   def lazilyNoPath(data: Data): Seq[Data] = data match {
     case data: Record =>
       LazyList(data) ++
-        data.elements.view.flatMap {
+        data._elements.view.flatMap {
           case (fieldName, fieldData) =>
             getRecursiveFields.lazilyNoPath(fieldData)
         }
@@ -292,13 +292,13 @@ private[chisel3] object getMatchedFields {
       require(x.typeEquivalent(y))
       Seq(x -> y)
     case (x: Record, y: Record) =>
-      (x.elements
-        .zip(y.elements))
+      (x._elements
+        .zip(y._elements))
         .map {
           case ((xName, xElt), (yName, yElt)) =>
             require(
               xName == yName,
-              s"$xName != $yName, ${x.elements}, ${y.elements}, $x, $y"
+              s"$xName != $yName, ${x._elements}, ${y._elements}, $x, $y"
             ) // assume fields returned in same, deterministic order
             getMatchedFields(xElt, yElt)
         }
@@ -839,8 +839,8 @@ object Data {
     }
     implicit class RecordOptGet(rOpt: Option[Record]) {
       // Like .get, but its already defined on Option
-      def grab(k: String): Option[Data] = rOpt.flatMap { _.elements.get(k) }
-      def keys: Iterable[String] = rOpt.map { r => r.elements.map(_._1) }.getOrElse(Seq.empty[String])
+      def grab(k: String): Option[Data] = rOpt.flatMap { _._elements.get(k) }
+      def keys: Iterable[String] = rOpt.map { r => r._elements.map(_._1) }.getOrElse(Seq.empty[String])
     }
     //TODO(azidar): Rewrite this to be more clear, probably not the cleanest way to express this
     private def isDifferent(l: Option[Data], r: Option[Data]): Boolean =
@@ -917,17 +917,17 @@ object Data {
               .reduce(_ && _)
           }
         case (thiz: Record, that: Record) =>
-          if (thiz.elements.size != that.elements.size) {
+          if (thiz._elements.size != that._elements.size) {
             throwException(s"Cannot compare Bundles $thiz and $that: Bundle types differ")
           } else {
-            thiz.elements.map {
+            thiz._elements.map {
               case (thisName, thisData) =>
-                if (!that.elements.contains(thisName))
+                if (!that._elements.contains(thisName))
                   throwException(
                     s"Cannot compare Bundles $thiz and $that: field $thisName (from $thiz) was not found in $that"
                   )
 
-                val thatData = that.elements(thisName)
+                val thatData = that._elements(thisName)
 
                 try {
                   thisData === thatData