From b169f6db95f9778cf8968cc1042b7f810f9d8123 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 15 Nov 2022 05:28:10 +0000 Subject: fullModulePorts + Opaque Types Fix and Test (#2845) (#2846) (cherry picked from commit 49feb083c69066988ca0666ea4249a86570e2589) Co-authored-by: Megan Wachs --- core/src/main/scala/chisel3/Data.scala | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 50093333..dddc0d5d 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -256,8 +256,13 @@ package experimental { def fullModulePorts(target: BaseModule): Seq[(String, Data)] = { def getPortNames(name: String, data: Data): Seq[(String, Data)] = Seq(name -> data) ++ (data match { case _: Element => Seq() - case r: Record => r.elements.toSeq.flatMap { case (eltName, elt) => getPortNames(s"${name}_${eltName}", elt) } - case v: Vec[_] => v.zipWithIndex.flatMap { case (elt, index) => getPortNames(s"${name}_${index}", elt) } + case r: Record => + r.elements.toSeq.flatMap { + case (eltName, elt) => + if (r._isOpaqueType) { getPortNames(s"${name}", elt) } + else { getPortNames(s"${name}_${eltName}", elt) } + } + case v: Vec[_] => v.zipWithIndex.flatMap { case (elt, index) => getPortNames(s"${name}_${index}", elt) } }) modulePorts(target).flatMap { case (name, data) => -- cgit v1.2.3 From c21f31d09f2511497cea5cb03bd6ddba440c55fe Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 29 Nov 2022 17:24:38 +0000 Subject: Implement compressed Namespace (backport #2856) (#2860) * Implement compressed Namespace (#2856) The namespace disambiguates requests for the same name with _. Rather than storing every disambiguated name in the underlying HashMap, it now only stores the base along with the "next available" index. This makes the logic for checking if a name is already contained in the namespace slightly more sophisticated because users can name things in a way that will collide with disambiguated names from a common substring. For example, in naming the sequence "foo", "foo", "foo_1", the 2nd "foo" takes the name "foo_1" so the following "foo_1" gets disambiguated to "foo_1_1". But since we compressed that original "foo_1" into the same HashMap entry as just "foo", we have to do a form of "prefix checking" whenever naming something that ends in "_". In practice, the saved memory allocations more than make up for the more complicated logic to disambiguate names because the common case is still fast. (cherry picked from commit 1654d87a02ca799bf12805a611a91e7524d49843) # Conflicts: # core/src/main/scala/chisel3/internal/Builder.scala * Resolve backport conflicts Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 63 +++++++++++++++++++--- 1 file changed, 55 insertions(+), 8 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index e3dfff09..d06b7992 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -17,18 +17,29 @@ import chisel3.internal.Builder.Prefix import logger.LazyLogging import scala.collection.mutable +import scala.annotation.tailrec private[chisel3] class Namespace(keywords: Set[String]) { + // This HashMap is compressed, not every name in the namespace is present here. + // If the same name is requested multiple times, it only takes 1 entry in the HashMap and the + // value is incremented for each time the name is requested. + // Names can be requested that collide with compressed sets of names, thus the algorithm for + // checking if a name is present in the Namespace is more complex than just checking the HashMap, + // see getIndex below. private val names = collection.mutable.HashMap[String, Long]() def copyTo(other: Namespace): Unit = names.foreach { case (s: String, l: Long) => other.names(s) = l } for (keyword <- keywords) names(keyword) = 1 - private def rename(n: String): String = { - val index = names(n) + @tailrec + private def rename(n: String, index: Long): String = { val tryName = s"${n}_${index}" - names(n) = index + 1 - if (this contains tryName) rename(n) else tryName + if (names.contains(tryName)) { + rename(n, index + 1) + } else { + names(n) = index + 1 + tryName + } } private def sanitize(s: String, leadingDigitOk: Boolean = false): String = { @@ -40,14 +51,50 @@ private[chisel3] class Namespace(keywords: Set[String]) { if (headOk) res else s"_$res" } - def contains(elem: String): Boolean = names.contains(elem) + /** Checks if `n` ends in `_\d+` and returns the substring before `_` if so, null otherwise */ + // TODO can and should this be folded in to sanitize? Same iteration as the forall? + private def prefix(n: String): Int = { + // This is micro-optimized because it runs on every single name + var i = n.size - 1 + while (i > 0 && n(i).isDigit) { + i -= 1 + } + // Will get i == 0 for all digits or _\d+ with empty prefix, those have no prefix so returning 0 is correct + if (i == n.size) 0 // no digits + else if (n(i) != '_') 0 // no _ + else i + } + + // Gets the current index for this name, None means it is not contained in the Namespace + private def getIndex(elem: String): Option[Long] = + names.get(elem).orElse { + // This exact name isn't contained, but if we end in _, we need to check our prefix + val maybePrefix = prefix(elem) + if (maybePrefix == 0) None + else { + // If we get a prefix collision and our index is taken, we start disambiguating with __1 + names + .get(elem.take(maybePrefix)) + .filter { prefixIdx => + val ourIdx = elem.drop(maybePrefix + 1).toInt + // The namespace starts disambiguating at _1 so _0 is a false collision case + ourIdx != 0 && prefixIdx > ourIdx + } + .map(_ => 1) + } + } + + def contains(elem: String): Boolean = getIndex(elem).isDefined // leadingDigitOk is for use in fields of Records def name(elem: String, leadingDigitOk: Boolean = false): String = { val sanitized = sanitize(elem, leadingDigitOk) - val result = if (this.contains(sanitized)) rename(sanitized) else sanitized - names(result) = 1 - result + getIndex(sanitized) match { + case Some(idx) => rename(sanitized, idx) + case None => + names(sanitized) = 1 + sanitized + } } } -- cgit v1.2.3 From 41d0d4cd075130cb6b4e41a7c7b6183830b5b9bc Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 7 Dec 2022 00:41:55 +0000 Subject: Make PriorityMux stack safe (backport #2854) (#2855) * Make PriorityMux stack safe (#2854) It used to be implemented with recursion, now it's implemented with a stack safe reverse and foldLeft. Also there were no tests for PriorityMux so I added one which helps prove the change is functionally correct. (cherry picked from commit 269ce472e9aa0c242fc028871a1fd5b045c82f83) # Conflicts: # src/test/scala/chiselTests/util/PipeSpec.scala * Resolve backport conflicts Co-authored-by: Jack Koenig Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>--- core/src/main/scala/chisel3/SeqUtils.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/SeqUtils.scala b/core/src/main/scala/chisel3/SeqUtils.scala index b1136120..9d975349 100644 --- a/core/src/main/scala/chisel3/SeqUtils.scala +++ b/core/src/main/scala/chisel3/SeqUtils.scala @@ -64,7 +64,10 @@ private[chisel3] object SeqUtils { if (in.size == 1) { in.head._2 } else { - Mux(in.head._1, in.head._2, priorityMux(in.tail)) + val r = in.view.reverse + r.tail.foldLeft(r.head._2) { + case (alt, (sel, elt)) => Mux(sel, elt, alt) + } } } -- cgit v1.2.3 From 294bf10510b2dc55312be2e87f9bed556c68afc5 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 8 Dec 2022 22:17:33 +0000 Subject: Replay changes on 3.5.x (#2865) Co-authored-by: Aditya Naik --- core/src/main/scala/chisel3/IO.scala | 41 ++++++++++++++++++++++ core/src/main/scala/chisel3/Module.scala | 38 ++------------------ .../main/scala/chisel3/experimental/package.scala | 2 +- 3 files changed, 45 insertions(+), 36 deletions(-) create mode 100644 core/src/main/scala/chisel3/IO.scala (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/IO.scala b/core/src/main/scala/chisel3/IO.scala new file mode 100644 index 00000000..1a28db1e --- /dev/null +++ b/core/src/main/scala/chisel3/IO.scala @@ -0,0 +1,41 @@ +package chisel3 + +import chisel3.internal.requireIsChiselType // Fix ambiguous import +import chisel3.internal.Builder +import chisel3.internal.sourceinfo.SourceInfo + +object IO { + + /** Constructs a port for the current Module + * + * This must wrap the datatype used to set the io field of any Module. + * i.e. All concrete modules must have defined io in this form: + * [lazy] val io[: io type] = IO(...[: io type]) + * + * Items in [] are optional. + * + * The granted iodef must be a chisel type and not be bound to hardware. + * + * Also registers a Data as a port, also performing bindings. Cannot be called once ports are + * requested (so that all calls to ports will return the same information). + * Internal API. + */ + def apply[T <: Data](iodef: T): T = { + val module = Module.currentModule.get // Impossible to fail + require(!module.isClosed, "Can't add more ports after module close") + requireIsChiselType(iodef, "io type") + + // Clone the IO so we preserve immutability of data types + val iodefClone = + try { + iodef.cloneTypeFull + } catch { + // For now this is going to be just a deprecation so we don't suddenly break everyone's code + case e: AutoClonetypeException => + Builder.deprecated(e.getMessage, Some(s"${iodef.getClass}")) + iodef + } + module.bindIoInPlace(iodefClone) + iodefClone + } +} diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index e5c2a848..a2d5cec6 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -200,42 +200,10 @@ abstract class Module(implicit moduleCompileOptions: CompileOptions) extends Raw } package experimental { - - import chisel3.internal.requireIsChiselType // Fix ambiguous import - object IO { - - /** Constructs a port for the current Module - * - * This must wrap the datatype used to set the io field of any Module. - * i.e. All concrete modules must have defined io in this form: - * [lazy] val io[: io type] = IO(...[: io type]) - * - * Items in [] are optional. - * - * The granted iodef must be a chisel type and not be bound to hardware. - * - * Also registers a Data as a port, also performing bindings. Cannot be called once ports are - * requested (so that all calls to ports will return the same information). - * Internal API. - */ + @deprecated("chisel3.experimental.IO is deprecated, use chisel3.IO instead", "Chisel 3.5") def apply[T <: Data](iodef: T): T = { - val module = Module.currentModule.get // Impossible to fail - require(!module.isClosed, "Can't add more ports after module close") - requireIsChiselType(iodef, "io type") - - // Clone the IO so we preserve immutability of data types - val iodefClone = - try { - iodef.cloneTypeFull - } catch { - // For now this is going to be just a deprecation so we don't suddenly break everyone's code - case e: AutoClonetypeException => - Builder.deprecated(e.getMessage, Some(s"${iodef.getClass}")) - iodef - } - module.bindIoInPlace(iodefClone) - iodefClone + chisel3.IO.apply(iodef) } } } @@ -753,7 +721,7 @@ package experimental { * TODO(twigg): Specifically walk the Data definition to call out which nodes * are problematic. */ - protected def IO[T <: Data](iodef: T): T = chisel3.experimental.IO.apply(iodef) + protected def IO[T <: Data](iodef: T): T = chisel3.IO.apply(iodef) // // Internal Functions diff --git a/core/src/main/scala/chisel3/experimental/package.scala b/core/src/main/scala/chisel3/experimental/package.scala index 39131943..42ec9666 100644 --- a/core/src/main/scala/chisel3/experimental/package.scala +++ b/core/src/main/scala/chisel3/experimental/package.scala @@ -72,7 +72,7 @@ package object experimental { val ports: Seq[Data] = gen.elements.toSeq.reverse.map { case (name, data) => - val p = IO(coerceDirection(chiselTypeClone(data).asInstanceOf[Data])) + val p = chisel3.IO(coerceDirection(chiselTypeClone(data).asInstanceOf[Data])) p.suggestName(name) p -- cgit v1.2.3