From 4e8f362c399f85b93bce4175562670242d8411b1 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 25 Mar 2022 17:35:44 +0000 Subject: rm unused/deprecated BlackBoxResourceAnno import (#2458) (#2460) Signed-off-by: Schuyler Eldridge (cherry picked from commit 2c2d72ceaa494b6acc351ff4300dbb40d4a7d863) Co-authored-by: Schuyler Eldridge --- src/main/scala/chisel3/util/BlackBoxUtils.scala | 8 +------- src/main/scala/chisel3/util/ExtModuleUtils.scala | 8 +------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/main/scala/chisel3/util/BlackBoxUtils.scala b/src/main/scala/chisel3/util/BlackBoxUtils.scala index 7c4400f8..579a6307 100644 --- a/src/main/scala/chisel3/util/BlackBoxUtils.scala +++ b/src/main/scala/chisel3/util/BlackBoxUtils.scala @@ -4,13 +4,7 @@ package chisel3.util import chisel3._ import chisel3.experimental.{ChiselAnnotation, RunFirrtlTransform} -import firrtl.transforms.{ - BlackBoxInlineAnno, - BlackBoxNotFoundException, - BlackBoxPathAnno, - BlackBoxResourceAnno, - BlackBoxSourceHelper -} +import firrtl.transforms.{BlackBoxInlineAnno, BlackBoxNotFoundException, BlackBoxPathAnno, BlackBoxSourceHelper} import firrtl.annotations.ModuleName import logger.LazyLogging diff --git a/src/main/scala/chisel3/util/ExtModuleUtils.scala b/src/main/scala/chisel3/util/ExtModuleUtils.scala index 5c9c02ba..8a687d36 100644 --- a/src/main/scala/chisel3/util/ExtModuleUtils.scala +++ b/src/main/scala/chisel3/util/ExtModuleUtils.scala @@ -3,13 +3,7 @@ package chisel3.util import chisel3.experimental.{ChiselAnnotation, ExtModule, RunFirrtlTransform} -import firrtl.transforms.{ - BlackBoxInlineAnno, - BlackBoxNotFoundException, - BlackBoxPathAnno, - BlackBoxResourceAnno, - BlackBoxSourceHelper -} +import firrtl.transforms.{BlackBoxInlineAnno, BlackBoxNotFoundException, BlackBoxPathAnno, BlackBoxSourceHelper} import BlackBoxHelpers._ -- cgit v1.2.3 From 157d5c3e37058286539867ca9a33b8bc1c6e43e3 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 30 Mar 2022 23:58:32 +0000 Subject: Use var List instead of ListBuffer to save memory (#2465) (#2467) This reduces memory use of every HasId by 64 bytes. Every instance of HasId (including all Data) had 2 ListBuffer vals for recording post-naming hooks, yet this feature is almost never used. These are now vars of type List which allows the common case of Nil to add no incremental memory use per instance of HasId. (cherry picked from commit cf410180ac8de854d8d7ecf89f4813ac8541dcdb) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 1ffe54ab..d9ac8cc5 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -115,10 +115,10 @@ private[chisel3] trait HasId extends InstanceId { private var prefix_seed: Prefix = Nil // Post-seed hooks called to carry the suggested seeds to other candidates as needed - private val suggest_postseed_hooks = scala.collection.mutable.ListBuffer.empty[String => Unit] + private var suggest_postseed_hooks: List[String => Unit] = Nil // Post-seed hooks called to carry the auto seeds to other candidates as needed - private val auto_postseed_hooks = scala.collection.mutable.ListBuffer.empty[String => Unit] + private var auto_postseed_hooks: List[String => Unit] = Nil /** Takes the last seed suggested. Multiple calls to this function will take the last given seed, unless * this HasId is a module port (see overridden method in Data.scala). @@ -136,7 +136,7 @@ private[chisel3] trait HasId extends InstanceId { // Bypass the overridden behavior of autoSeed in [[Data]], apply autoSeed even to ports private[chisel3] def forceAutoSeed(seed: String): this.type = { auto_seed = Some(seed) - for (hook <- auto_postseed_hooks) { hook(seed) } + for (hook <- auto_postseed_hooks.reverse) { hook(seed) } prefix_seed = Builder.getPrefix this } @@ -154,7 +154,7 @@ private[chisel3] trait HasId extends InstanceId { def suggestName(seed: => String): this.type = { if (suggested_seed.isEmpty) suggested_seed = Some(seed) prefix_seed = Builder.getPrefix - for (hook <- suggest_postseed_hooks) { hook(seed) } + for (hook <- suggest_postseed_hooks.reverse) { hook(seed) } this } @@ -211,8 +211,8 @@ private[chisel3] trait HasId extends InstanceId { private[chisel3] def hasAutoSeed: Boolean = auto_seed.isDefined - private[chisel3] def addSuggestPostnameHook(hook: String => Unit): Unit = suggest_postseed_hooks += hook - private[chisel3] def addAutoPostnameHook(hook: String => Unit): Unit = auto_postseed_hooks += hook + private[chisel3] def addSuggestPostnameHook(hook: String => Unit): Unit = suggest_postseed_hooks ::= hook + private[chisel3] def addAutoPostnameHook(hook: String => Unit): Unit = auto_postseed_hooks ::= hook // Uses a namespace to convert suggestion into a true name // Will not do any naming if the reference already assigned. -- cgit v1.2.3 From 32430bcfc49b0293748ba3f6e5000a45a3dcce92 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 1 Apr 2022 17:24:56 +0000 Subject: Prevent FIRRTL bulk connects on BlackBox Bundles. (#2468) (#2469) (cherry picked from commit 4da1e89f3a0b79adcb39ea5defb393ed6c00fa2f) Co-authored-by: fzi-hielscher <47524191+fzi-hielscher@users.noreply.github.com>--- core/src/main/scala/chisel3/internal/BiConnect.scala | 8 +++++++- src/test/scala/chiselTests/BulkConnectSpec.scala | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/internal/BiConnect.scala b/core/src/main/scala/chisel3/internal/BiConnect.scala index a8b425f5..2d6c9e4a 100644 --- a/core/src/main/scala/chisel3/internal/BiConnect.scala +++ b/core/src/main/scala/chisel3/internal/BiConnect.scala @@ -285,7 +285,13 @@ private[chisel3] object BiConnect { case _ => true } - typeCheck && contextCheck && bindingCheck && flow_check && sourceNotLiteralCheck + // do not bulk connect the 'io' pseudo-bundle of a BlackBox since it will be decomposed in FIRRTL + def blackBoxCheck = Seq(source, sink).map(_._parent).forall { + case Some(_: BlackBox) => false + case _ => true + } + + typeCheck && contextCheck && bindingCheck && flow_check && sourceNotLiteralCheck && blackBoxCheck } // These functions (finally) issue the connection operation diff --git a/src/test/scala/chiselTests/BulkConnectSpec.scala b/src/test/scala/chiselTests/BulkConnectSpec.scala index 463122bd..281890d4 100644 --- a/src/test/scala/chiselTests/BulkConnectSpec.scala +++ b/src/test/scala/chiselTests/BulkConnectSpec.scala @@ -94,6 +94,26 @@ class BulkConnectSpec extends ChiselPropSpec { chirrtl should include("deq <= enq") } + property("Chisel connects should not emit a FIRRTL bulk connect for BlackBox IO Bundles") { + class MyBundle extends Bundle { + val O: Bool = Output(Bool()) + val I: Bool = Input(Bool()) + } + + val chirrtl = ChiselStage.emitChirrtl(new Module { + val io: MyBundle = IO(Flipped(new MyBundle)) + + val bb = Module(new BlackBox { + val io: MyBundle = IO(Flipped(new MyBundle)) + }) + + io <> bb.io + }) + // There won't be a bb.io Bundle in FIRRTL, so connections have to be done element-wise + chirrtl should include("bb.O <= io.O") + chirrtl should include("io.I <= bb.I") + } + property("MonoConnect should bulk connect undirectioned internal wires") { val chirrtl = ChiselStage.emitChirrtl(new Module { val io = IO(new Bundle {}) -- cgit v1.2.3 From 1de3eb7367dc79f601433d1bbec34f95911eb926 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 5 Apr 2022 04:24:40 +0000 Subject: Micro-optimize String building in _computeName (#2472) (#2473) (cherry picked from commit 3940136bec72fc44e40d454f2c2dcc421fc92d82) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index d9ac8cc5..5d6dc7bc 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -183,7 +183,10 @@ private[chisel3] trait HasId extends InstanceId { */ def buildName(seed: String, prefix: Prefix): String = { val builder = new StringBuilder() - prefix.foreach(builder ++= _ + "_") + prefix.foreach { p => + builder ++= p + builder += '_' + } builder ++= seed builder.toString } -- cgit v1.2.3 From d766e8f7270579406d54abc9015d494cd199c6ce Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 5 Apr 2022 17:06:28 +0000 Subject: Micro-optimize Namespace.name (#2474) (#2475) * During sanitize, only filter the String if needed * Do not recurse on name, saving an unnecessary call to sanitize (cherry picked from commit 559b3df3e5bd6c73588638aa44a6df1244a11a53) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 5d6dc7bc..c0007a5f 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -35,7 +35,7 @@ private[chisel3] class Namespace(keywords: Set[String]) { // TODO what character set does FIRRTL truly support? using ANSI C for now def legalStart(c: Char) = (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_' def legal(c: Char) = legalStart(c) || (c >= '0' && c <= '9') - val res = s.filter(legal) + val res = if (s.forall(legal)) s else s.filter(legal) val headOk = (!res.isEmpty) && (leadingDigitOk || legalStart(res.head)) if (headOk) res else s"_$res" } @@ -45,12 +45,9 @@ private[chisel3] class Namespace(keywords: Set[String]) { // leadingDigitOk is for use in fields of Records def name(elem: String, leadingDigitOk: Boolean = false): String = { val sanitized = sanitize(elem, leadingDigitOk) - if (this contains sanitized) { - name(rename(sanitized)) - } else { - names(sanitized) = 1 - sanitized - } + val result = if (this.contains(sanitized)) rename(sanitized) else sanitized + names(result) = 1 + result } } -- cgit v1.2.3 From 898142ba05b04fb1602b249fd1ae81baa3f47f89 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 12 Apr 2022 00:09:55 +0000 Subject: Enhance views to [sometimes] support dynamic indexing and implement FlatIO (backport #2476) (#2479) * Capture 1:1 mappings of Aggregates inside of views This is implemented by including any corresponding Aggregates from the DataView.mapping in the AggregateViewBinding.childMap (which is now of type Map[Data, Data]). This enables dynamically indexing Vecs that are themselves elements of larger Aggregates in views when the corresponding element of the view is a Vec of the same type. It also increases the number of cases where a single Target can represent part of a view. (cherry picked from commit 1f6b1ca14ccf86918065073c3f6f3626dd83a68e) * Add FlatIO API for creating ports from Bundles without a prefix (cherry picked from commit 772a3a1fe3b9372b7c2d7cd2d424b2adcd633cdb) * [docs] Add FlatIO to the general cookbook (cherry picked from commit b4159641350f238f0f899b69954142ce8ee11544) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Data.scala | 2 +- core/src/main/scala/chisel3/Element.scala | 11 +++-- .../chisel3/experimental/dataview/package.scala | 55 ++++++++++++---------- .../experimental/hierarchy/Lookupable.scala | 16 +++---- .../main/scala/chisel3/experimental/package.scala | 28 +++++++++++ core/src/main/scala/chisel3/internal/Binding.scala | 11 +++-- docs/src/cookbooks/cookbook.md | 45 ++++++++++++++++++ .../scala/chiselTests/experimental/DataView.scala | 30 ++++++++++++ .../experimental/DataViewTargetSpec.scala | 4 +- .../chiselTests/experimental/FlatIOSpec.scala | 51 ++++++++++++++++++++ 10 files changed, 209 insertions(+), 44 deletions(-) create mode 100644 src/test/scala/chiselTests/experimental/FlatIOSpec.scala diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index ef43a3b0..f468335e 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -667,7 +667,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { topBindingOpt match { // DataView case Some(ViewBinding(target)) => reify(target).ref - case Some(AggregateViewBinding(viewMap, _)) => + case Some(AggregateViewBinding(viewMap)) => viewMap.get(this) match { case None => materializeWire() // FIXME FIRRTL doesn't have Aggregate Init expressions // This should not be possible because Element does the lookup in .topBindingOpt diff --git a/core/src/main/scala/chisel3/Element.scala b/core/src/main/scala/chisel3/Element.scala index 401f2bdf..39b7689c 100644 --- a/core/src/main/scala/chisel3/Element.scala +++ b/core/src/main/scala/chisel3/Element.scala @@ -36,10 +36,15 @@ abstract class Element extends Data { case Some(litArg) => Some(ElementLitBinding(litArg)) case _ => Some(DontCareBinding()) } - case Some(b @ AggregateViewBinding(viewMap, _)) => + // TODO Do we even need this? Looking up things in the AggregateViewBinding is fine + case Some(b @ AggregateViewBinding(viewMap)) => viewMap.get(this) match { - case Some(elt) => Some(ViewBinding(elt)) - case _ => throwException(s"Internal Error! $this missing from topBinding $b") + case Some(elt: Element) => Some(ViewBinding(elt)) + // TODO We could generate a reduced AggregateViewBinding, but is there a point? + // Generating the new object would be somewhat slow, it's not clear if we should do this + // matching anyway + case Some(data: Aggregate) => Some(b) + case _ => throwException(s"Internal Error! $this missing from topBinding $b") } case topBindingOpt => topBindingOpt } diff --git a/core/src/main/scala/chisel3/experimental/dataview/package.scala b/core/src/main/scala/chisel3/experimental/dataview/package.scala index 891ecb81..c583c516 100644 --- a/core/src/main/scala/chisel3/experimental/dataview/package.scala +++ b/core/src/main/scala/chisel3/experimental/dataview/package.scala @@ -97,11 +97,16 @@ package object dataview { val targetContains: Data => Boolean = implicitly[DataProduct[T]].dataSet(target) // Resulting bindings for each Element of the View - val childBindings = + // Kept separate from Aggregates for totality checking + val elementBindings = new mutable.HashMap[Data, mutable.ListBuffer[Element]] ++ viewFieldLookup.view.collect { case (elt: Element, _) => elt } .map(_ -> new mutable.ListBuffer[Element]) + // Record any Aggregates that correspond 1:1 for reification + // Using Data instead of Aggregate to avoid unnecessary checks + val aggregateMappings = mutable.ArrayBuffer.empty[(Data, Data)] + def viewFieldName(d: Data): String = viewFieldLookup.get(d).map(_ + " ").getOrElse("") + d.toString @@ -130,7 +135,7 @@ package object dataview { s"View field $fieldName has width ${vwidth} that is incompatible with target value $tex's width ${twidth}" ) } - childBindings(vex) += tex + elementBindings(vex) += tex } mapping.foreach { @@ -145,7 +150,7 @@ package object dataview { } getMatchedFields(aa, ba).foreach { case (aelt: Element, belt: Element) => onElt(aelt, belt) - case _ => // Ignore matching of Aggregates + case (t, v) => aggregateMappings += (v -> t) } } @@ -155,7 +160,7 @@ package object dataview { val targetSeen: Option[mutable.Set[Data]] = if (total) Some(mutable.Set.empty[Data]) else None - val resultBindings = childBindings.map { + val elementResult = elementBindings.map { case (data, targets) => val targetsx = targets match { case collection.Seq(target: Element) => target @@ -183,23 +188,25 @@ package object dataview { } view match { - case elt: Element => view.bind(ViewBinding(resultBindings(elt))) + case elt: Element => view.bind(ViewBinding(elementResult(elt))) case agg: Aggregate => - // We record total Data mappings to provide a better .toTarget - val topt = target match { - case d: Data if total => Some(d) + // Don't forget the potential mapping of the view to the target! + target match { + case d: Data if total => + aggregateMappings += (agg -> d) case _ => - // Record views that don't have the simpler .toTarget for later renaming - Builder.unnamedViews += view - None } - // TODO We must also record children as unnamed, some could be namable but this requires changes to the Binding + + val fullResult = elementResult ++ aggregateMappings + + // We need to record any Aggregates that don't have a 1-1 mapping (including the view + // itself) getRecursiveFields.lazily(view, "_").foreach { - case (agg: Aggregate, _) if agg != view => - Builder.unnamedViews += agg + case (unnamed: Aggregate, _) if !fullResult.contains(unnamed) => + Builder.unnamedViews += unnamed case _ => // Do nothing } - agg.bind(AggregateViewBinding(resultBindings, topt)) + agg.bind(AggregateViewBinding(fullResult)) } } @@ -208,8 +215,8 @@ package object dataview { private def unfoldView(elt: Element): LazyList[Element] = { def rec(e: Element): LazyList[Element] = e.topBindingOpt match { case Some(ViewBinding(target)) => target #:: rec(target) - case Some(AggregateViewBinding(mapping, _)) => - val target = mapping(e) + case Some(avb: AggregateViewBinding) => + val target = avb.lookup(e).get target #:: rec(target) case Some(_) | None => LazyList.empty } @@ -246,15 +253,11 @@ package object dataview { */ private[chisel3] def reifySingleData(data: Data): Option[Data] = { val candidate: Option[Data] = - data.binding.collect { // First check if this is a total mapping of an Aggregate - case AggregateViewBinding(_, Some(t)) => t - }.orElse { // Otherwise look via top binding - data.topBindingOpt match { - case None => None - case Some(ViewBinding(target)) => Some(target) - case Some(AggregateViewBinding(lookup, _)) => lookup.get(data) - case Some(_) => None - } + data.topBindingOpt match { + case None => None + case Some(ViewBinding(target)) => Some(target) + case Some(AggregateViewBinding(lookup)) => lookup.get(data) + case Some(_) => None } candidate.flatMap { d => // Candidate may itself be a view, keep tracing in those cases diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala b/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala index 60290f83..46a38e7c 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala @@ -173,12 +173,12 @@ object Lookupable { // We have to lookup the target(s) of the view since they may need to be underlying into the current context val newBinding = data.topBinding match { case ViewBinding(target) => ViewBinding(lookupData(reify(target))) - case avb @ AggregateViewBinding(map, targetOpt) => + case avb @ AggregateViewBinding(map) => data match { - case _: Element => ViewBinding(lookupData(reify(map(data)))) + case e: Element => ViewBinding(lookupData(reify(avb.lookup(e).get))) case _: Aggregate => // Provide a 1:1 mapping if possible - val singleTargetOpt = targetOpt.filter(_ => avb == data.binding.get).flatMap(reifySingleData) + val singleTargetOpt = map.get(data).filter(_ => avb == data.binding.get).flatMap(reifySingleData) singleTargetOpt match { case Some(singleTarget) => // It is 1:1! // This is a little tricky because the values in newMap need to point to Elements of newTarget @@ -187,15 +187,15 @@ object Lookupable { case (res, from) => (res: Data) -> mapRootAndExtractSubField(map(from), _ => newTarget) }.toMap - AggregateViewBinding(newMap, Some(newTarget)) + AggregateViewBinding(newMap + (result -> newTarget)) case None => // No 1:1 mapping so we have to do a flat binding // Just remap each Element of this aggregate val newMap = coiterate(result, data).map { // Upcast res to Data since Maps are invariant in the Key type parameter - case (res, from) => (res: Data) -> lookupData(reify(map(from))) + case (res, from) => (res: Data) -> lookupData(reify(avb.lookup(from).get)) }.toMap - AggregateViewBinding(newMap, None) + AggregateViewBinding(newMap) } } } @@ -204,8 +204,8 @@ object Lookupable { // We must also mark non-1:1 and child Aggregates in the view for renaming newBinding match { case _: ViewBinding => // Do nothing - case AggregateViewBinding(_, target) => - if (target.isEmpty) { + case AggregateViewBinding(childMap) => + if (!childMap.contains(result)) { Builder.unnamedViews += result } // Binding does not capture 1:1 for child aggregates views diff --git a/core/src/main/scala/chisel3/experimental/package.scala b/core/src/main/scala/chisel3/experimental/package.scala index ce258a25..9b9c83f4 100644 --- a/core/src/main/scala/chisel3/experimental/package.scala +++ b/core/src/main/scala/chisel3/experimental/package.scala @@ -57,6 +57,34 @@ package object experimental { type Direction = ActualDirection val Direction = ActualDirection + /** The same as [[IO]] except there is no prefix for the name of the val */ + def FlatIO[T <: Record](gen: => T)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T = noPrefix { + import dataview._ + def coerceDirection(d: Data) = { + import chisel3.{SpecifiedDirection => SD} + DataMirror.specifiedDirectionOf(gen) match { + case SD.Flip => Flipped(d) + case SD.Input => Input(d) + case SD.Output => Output(d) + case _ => d + } + } + val ports: Seq[Data] = + gen.elements.toSeq.reverse.map { + case (name, data) => + val p = IO(coerceDirection(chiselTypeClone(data).asInstanceOf[Data])) + p.suggestName(name) + p + + } + + implicit val dv: DataView[Seq[Data], T] = DataView.mapping( + _ => chiselTypeClone(gen).asInstanceOf[T], + (seq, rec) => seq.zip(rec.elements.toSeq.reverse).map { case (port, (_, field)) => port -> field } + ) + ports.viewAs[T] + } + implicit class ChiselRange(val sc: StringContext) extends AnyVal { import scala.language.experimental.macros diff --git a/core/src/main/scala/chisel3/internal/Binding.scala b/core/src/main/scala/chisel3/internal/Binding.scala index 6431dd23..bab79bc1 100644 --- a/core/src/main/scala/chisel3/internal/Binding.scala +++ b/core/src/main/scala/chisel3/internal/Binding.scala @@ -141,11 +141,16 @@ case class DontCareBinding() extends UnconstrainedBinding private[chisel3] case class ViewBinding(target: Element) extends UnconstrainedBinding /** Binding for Aggregate Views - * @param childMap Mapping from children of this view to each child's target + * @param childMap Mapping from children of this view to their respective targets * @param target Optional Data this Aggregate views if the view is total and the target is a Data + * @note For any Elements in the childMap, both key and value must be Elements + * @note The types of key and value need not match for the top Data in a total view of type + * Aggregate */ -private[chisel3] case class AggregateViewBinding(childMap: Map[Data, Element], target: Option[Data]) - extends UnconstrainedBinding +private[chisel3] case class AggregateViewBinding(childMap: Map[Data, Data]) extends UnconstrainedBinding { + // Helper lookup function since types of Elements always match + def lookup(key: Element): Option[Element] = childMap.get(key).map(_.asInstanceOf[Element]) +} /** Binding for Data's returned from accessing an Instance/Definition members, if not readable/writable port */ private[chisel3] case object CrossModuleBinding extends TopBinding { diff --git a/docs/src/cookbooks/cookbook.md b/docs/src/cookbooks/cookbook.md index ae7c7bf6..b9e5db38 100644 --- a/docs/src/cookbooks/cookbook.md +++ b/docs/src/cookbooks/cookbook.md @@ -26,6 +26,7 @@ Please note that these examples make use of [Chisel's scala-style printing](../e * [How do I unpack a value ("reverse concatenation") like in Verilog?](#how-do-i-unpack-a-value-reverse-concatenation-like-in-verilog) * [How do I do subword assignment (assign to some bits in a UInt)?](#how-do-i-do-subword-assignment-assign-to-some-bits-in-a-uint) * [How do I create an optional I/O?](#how-do-i-create-an-optional-io) +* [How do I create I/O without a prefix?](#how-do-i-create-io-without-a-prefix) * [How do I minimize the number of bits used in an output vector](#how-do-i-minimize-the-number-of-bits-used-in-an-output-vector) * Predictable Naming * [How do I get Chisel to name signals properly in blocks like when/withClockAndReset?](#how-do-i-get-chisel-to-name-signals-properly-in-blocks-like-whenwithclockandreset) @@ -546,6 +547,50 @@ class ModuleWithOptionalIO(flag: Boolean) extends Module { } ``` +### How do I create I/O without a prefix? + +In most cases, you can simply call `IO` multiple times: + +```scala mdoc:silent:reset +import chisel3._ + +class MyModule extends Module { + val in = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + + out := in +% 1.U +} +``` + +```scala mdoc:verilog +getVerilogString(new MyModule) +``` + +If you have a `Bundle` from which you would like to create ports without the +normal `val` prefix, you can use `FlatIO`: + +```scala mdoc:silent:reset +import chisel3._ +import chisel3.experimental.FlatIO + +class MyBundle extends Bundle { + val foo = Input(UInt(8.W)) + val bar = Output(UInt(8.W)) +} + +class MyModule extends Module { + val io = FlatIO(new MyBundle) + + io.bar := io.foo +% 1.U +} +``` + +Note that `io_` is nowhere to be seen! + +```scala mdoc:verilog +getVerilogString(new MyModule) +``` + ### How do I minimize the number of bits used in an output vector? Use inferred width and a `Seq` instead of a `Vec`: diff --git a/src/test/scala/chiselTests/experimental/DataView.scala b/src/test/scala/chiselTests/experimental/DataView.scala index 0285a524..e7caacfd 100644 --- a/src/test/scala/chiselTests/experimental/DataView.scala +++ b/src/test/scala/chiselTests/experimental/DataView.scala @@ -332,6 +332,36 @@ class DataViewSpec extends ChiselFlatSpec { chirrtl should include("dataOut <= vec[addr]") } + it should "support dynamic indexing for Vecs that correspond 1:1 in a view" in { + class MyBundle extends Bundle { + val foo = Vec(4, UInt(8.W)) + val bar = UInt(2.W) + } + implicit val myView = DataView[(Vec[UInt], UInt), MyBundle]( + _ => new MyBundle, + _._1 -> _.foo, + _._2 -> _.bar + ) + class MyModule extends Module { + val dataIn = IO(Input(UInt(8.W))) + val addr = IO(Input(UInt(2.W))) + val dataOut = IO(Output(UInt(8.W))) + + val vec = RegInit(0.U.asTypeOf(Vec(4, UInt(8.W)))) + val addrReg = Reg(UInt(2.W)) + val view = (vec, addrReg).viewAs[MyBundle] + // Dynamic indexing is more of a "generator" in Chisel3 than an individual node + // This style is not recommended, this is just testing the behavior + val selected = view.foo(view.bar) + view.bar := addr + selected := dataIn + dataOut := selected + } + val chirrtl = ChiselStage.emitChirrtl(new MyModule) + chirrtl should include("vec[addrReg] <= dataIn") + chirrtl should include("dataOut <= vec[addrReg]") + } + it should "error if you try to dynamically index a Vec view that does not correspond to a Vec target" in { class MyModule extends Module { val inA, inB = IO(Input(UInt(8.W))) diff --git a/src/test/scala/chiselTests/experimental/DataViewTargetSpec.scala b/src/test/scala/chiselTests/experimental/DataViewTargetSpec.scala index da27c9c8..ddeeab6e 100644 --- a/src/test/scala/chiselTests/experimental/DataViewTargetSpec.scala +++ b/src/test/scala/chiselTests/experimental/DataViewTargetSpec.scala @@ -125,9 +125,7 @@ class DataViewTargetSpec extends ChiselFlatSpec { val pairs = annos.collect { case DummyAnno(t, idx) => (idx, t.toString) }.sortBy(_._1) val expected = Seq( 0 -> "~MyParent|MyChild>out.foo", - // The child of the view that was itself an Aggregate got split because 1:1 is lacking here - 1 -> "~MyParent|MyChild>out.foo[0]", - 1 -> "~MyParent|MyChild>out.foo[1]", + 1 -> "~MyParent|MyChild>out.foo", 2 -> "~MyParent|MyParent/inst:MyChild>out.foo", 3 -> "~MyParent|MyParent/inst:MyChild>out" ) diff --git a/src/test/scala/chiselTests/experimental/FlatIOSpec.scala b/src/test/scala/chiselTests/experimental/FlatIOSpec.scala new file mode 100644 index 00000000..dfce447f --- /dev/null +++ b/src/test/scala/chiselTests/experimental/FlatIOSpec.scala @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chiselTests.experimental + +import chisel3._ +import chisel3.util.Valid +import chisel3.stage.ChiselStage.emitChirrtl +import chisel3.experimental.FlatIO +import chiselTests.ChiselFlatSpec + +class FlatIOSpec extends ChiselFlatSpec { + behavior.of("FlatIO") + + it should "create ports without a prefix" in { + class MyModule extends RawModule { + val io = FlatIO(new Bundle { + val in = Input(UInt(8.W)) + val out = Output(UInt(8.W)) + }) + io.out := io.in + } + val chirrtl = emitChirrtl(new MyModule) + chirrtl should include("input in : UInt<8>") + chirrtl should include("output out : UInt<8>") + chirrtl should include("out <= in") + } + + it should "support bulk connections between FlatIOs and regular IOs" in { + class MyModule extends RawModule { + val in = FlatIO(Input(Valid(UInt(8.W)))) + val out = IO(Output(Valid(UInt(8.W)))) + out := in + } + val chirrtl = emitChirrtl(new MyModule) + chirrtl should include("out.bits <= bits") + chirrtl should include("out.valid <= valid") + } + + it should "dynamically indexing Vecs inside of FlatIOs" in { + class MyModule extends RawModule { + val io = FlatIO(new Bundle { + val addr = Input(UInt(2.W)) + val in = Input(Vec(4, UInt(8.W))) + val out = Output(Vec(4, UInt(8.W))) + }) + io.out(io.addr) := io.in(io.addr) + } + val chirrtl = emitChirrtl(new MyModule) + chirrtl should include("out[addr] <= in[addr]") + } +} -- cgit v1.2.3 From efd474842738444a030fbe7702d58d4a249d6ebc Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 12 Apr 2022 00:55:14 +0000 Subject: Optimize memory use of naming prefixes (#2471) (#2480) * Use a single field instead of two in HasId (4-bytes per HasId) * Set the prefix to Nil after setting ref to free up memory Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 3aa179f0dc1a29403fd25be7d3dc08630976d018) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index c0007a5f..4180f580 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -105,11 +105,10 @@ private[chisel3] trait HasId extends InstanceId { // Contains the seed computed automatically by the compiler plugin private var auto_seed: Option[String] = None - // Prefix at time when this class is constructed - private val construction_prefix: Prefix = Builder.getPrefix - - // Prefix when the latest [[suggestSeed]] or [[autoSeed]] is called - private var prefix_seed: Prefix = Nil + // Prefix for use in naming + // - Defaults to prefix at time when object is created + // - Overridden when [[suggestSeed]] or [[autoSeed]] is called + private var naming_prefix: Prefix = Builder.getPrefix // Post-seed hooks called to carry the suggested seeds to other candidates as needed private var suggest_postseed_hooks: List[String => Unit] = Nil @@ -134,7 +133,7 @@ private[chisel3] trait HasId extends InstanceId { private[chisel3] def forceAutoSeed(seed: String): this.type = { auto_seed = Some(seed) for (hook <- auto_postseed_hooks.reverse) { hook(seed) } - prefix_seed = Builder.getPrefix + naming_prefix = Builder.getPrefix this } @@ -150,7 +149,7 @@ private[chisel3] trait HasId extends InstanceId { */ def suggestName(seed: => String): this.type = { if (suggested_seed.isEmpty) suggested_seed = Some(seed) - prefix_seed = Builder.getPrefix + naming_prefix = Builder.getPrefix for (hook <- suggest_postseed_hooks.reverse) { hook(seed) } this } @@ -189,12 +188,12 @@ private[chisel3] trait HasId extends InstanceId { } if (hasSeed) { - Some(buildName(seedOpt.get, prefix_seed.reverse)) + Some(buildName(seedOpt.get, naming_prefix.reverse)) } else { defaultSeed.map { default => defaultPrefix match { - case Some(p) => buildName(default, p :: construction_prefix.reverse) - case None => buildName(default, construction_prefix.reverse) + case Some(p) => buildName(default, p :: naming_prefix.reverse) + case None => buildName(default, naming_prefix.reverse) } } } @@ -222,6 +221,8 @@ private[chisel3] trait HasId extends InstanceId { val candidate_name = _computeName(prefix, Some(default)).get val available_name = namespace.name(candidate_name) setRef(Ref(available_name)) + // Clear naming prefix to free memory + naming_prefix = Nil } private var _ref: Option[Arg] = None -- cgit v1.2.3 From b187a0ce5269e735e7bd75620b4fd33f7ed27af5 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 15 Apr 2022 19:45:25 +0000 Subject: Enable Clock Invalidation (#2485) (#2487) Loosen restrictions on clocks to enable them to be connected to DontCare, i.e., be invalidated. Co-authored-by: Jack Koenig Signed-off-by: Schuyler Eldridge Co-authored-by: Jack Koenig (cherry picked from commit 5d8a0c8e406376f7ceda91273fb0fa7a646865aa) Co-authored-by: Schuyler Eldridge --- core/src/main/scala/chisel3/Clock.scala | 2 +- src/test/scala/chiselTests/InvalidateAPISpec.scala | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/Clock.scala b/core/src/main/scala/chisel3/Clock.scala index 68174d7c..64e91c42 100644 --- a/core/src/main/scala/chisel3/Clock.scala +++ b/core/src/main/scala/chisel3/Clock.scala @@ -23,7 +23,7 @@ sealed class Clock(private[chisel3] val width: Width = Width(1)) extends Element override def connect(that: Data)(implicit sourceInfo: SourceInfo, connectCompileOptions: CompileOptions): Unit = that match { - case _: Clock => super.connect(that)(sourceInfo, connectCompileOptions) + case _: Clock | DontCare => super.connect(that)(sourceInfo, connectCompileOptions) case _ => super.badConnect(that)(sourceInfo) } diff --git a/src/test/scala/chiselTests/InvalidateAPISpec.scala b/src/test/scala/chiselTests/InvalidateAPISpec.scala index 2c51e5d2..dbd353a0 100644 --- a/src/test/scala/chiselTests/InvalidateAPISpec.scala +++ b/src/test/scala/chiselTests/InvalidateAPISpec.scala @@ -228,4 +228,12 @@ class InvalidateAPISpec extends ChiselPropSpec with Matchers with BackendCompila val firrtlOutput = myGenerateFirrtl(new ModuleWithoutDontCare) firrtlOutput should include("is invalid") } + + property("a clock should be able to be connected to a DontCare") { + class ClockConnectedToDontCare extends Module { + val foo = IO(Output(Clock())) + foo := DontCare + } + myGenerateFirrtl(new ClockConnectedToDontCare) should include("foo is invalid") + } } -- cgit v1.2.3 From 7fdce0fa158c7d15bc7984ba08ca8692134d3971 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 18 Apr 2022 09:39:32 +0000 Subject: Clarify example in Printable (#2454) (#2456) (cherry picked from commit d6a357d29cfa7120b3c0c90684b33be1863e5599) Co-authored-by: Megan Wachs Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>--- core/src/main/scala/chisel3/Printable.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/Printable.scala b/core/src/main/scala/chisel3/Printable.scala index c477716b..a616f2b0 100644 --- a/core/src/main/scala/chisel3/Printable.scala +++ b/core/src/main/scala/chisel3/Printable.scala @@ -18,7 +18,7 @@ import java.util.{MissingFormatArgumentException, UnknownFormatConversionExcepti * }}} * This is equivalent to writing: * {{{ - * printf(p"The value of wire = %d\n", wire) + * printf("The value of wire = %d\n", wire) * }}} * All Chisel data types have a method `.toPrintable` that gives a default pretty print that can be * accessed via `p"..."`. This works even for aggregate types, for example: -- cgit v1.2.3 From 17c930506046bc59f8a13a0c82734e7334b27613 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 18 Apr 2022 17:13:16 +0000 Subject: Fix small typos in doc comment (#2490) (#2492) (cherry picked from commit 52165fe2796d08c664069c148868aedc64ea3777) Co-authored-by: Lucheng Zhang <79909456+geekLucian@users.noreply.github.com>--- core/src/main/scala/chisel3/Bits.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/chisel3/Bits.scala b/core/src/main/scala/chisel3/Bits.scala index 4133592f..a135a8e5 100644 --- a/core/src/main/scala/chisel3/Bits.scala +++ b/core/src/main/scala/chisel3/Bits.scala @@ -1324,7 +1324,7 @@ sealed class Bool() extends UInt(1.W) with Reset { /** Logical or operator * * @param that a hardware $coll - * @return the lgocial or of this $coll and `that` + * @return the logical or of this $coll and `that` * @note this is equivalent to [[Bool!.|(that:chisel3\.Bool)* Bool.|)]] * @group Logical */ @@ -1336,7 +1336,7 @@ sealed class Bool() extends UInt(1.W) with Reset { /** Logical and operator * * @param that a hardware $coll - * @return the lgocial and of this $coll and `that` + * @return the logical and of this $coll and `that` * @note this is equivalent to [[Bool!.&(that:chisel3\.Bool)* Bool.&]] * @group Logical */ -- cgit v1.2.3 From 9ff8ca7d3f4f71b1e42a136d1465da43baf7085b Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 19 Apr 2022 00:22:50 +0000 Subject: verification: switch order of assert/assume and printf (#2484) (#2493) This is a quick fix for issue #2408 (cherry picked from commit d4ef9a96c4131252a0a49002a28be3391eb67258) Co-authored-by: Kevin Laeufer --- core/src/main/scala/chisel3/VerificationStatement.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/chisel3/VerificationStatement.scala b/core/src/main/scala/chisel3/VerificationStatement.scala index bfdfc26e..7229c412 100644 --- a/core/src/main/scala/chisel3/VerificationStatement.scala +++ b/core/src/main/scala/chisel3/VerificationStatement.scala @@ -92,8 +92,8 @@ object assert { ): Assert = { val id = new Assert() when(!Module.reset.asBool()) { - Builder.pushCommand(Verification(id, Formal.Assert, sourceInfo, Module.clock.ref, cond.ref, "")) failureMessage("Assertion", line, cond, message, data) + Builder.pushCommand(Verification(id, Formal.Assert, sourceInfo, Module.clock.ref, cond.ref, "")) } id } @@ -178,8 +178,8 @@ object assume { ): Assume = { val id = new Assume() when(!Module.reset.asBool()) { - Builder.pushCommand(Verification(id, Formal.Assume, sourceInfo, Module.clock.ref, cond.ref, "")) failureMessage("Assumption", line, cond, message, data) + Builder.pushCommand(Verification(id, Formal.Assume, sourceInfo, Module.clock.ref, cond.ref, "")) } id } -- cgit v1.2.3 From 70da39e140e96a9302a94864f077529e02596ef5 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 19 Apr 2022 02:29:00 +0000 Subject: Allow creating memories without an implicit clock (#2494) (#2495) Fixes #2470 (cherry picked from commit 44165a259bb16733a41798edca6b554b13f1d54a) Co-authored-by: Kevin Laeufer --- core/src/main/scala/chisel3/Mem.scala | 10 ++++--- src/test/scala/chiselTests/Mem.scala | 52 +++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/chisel3/Mem.scala b/core/src/main/scala/chisel3/Mem.scala index 36984a3a..d6ab9c4b 100644 --- a/core/src/main/scala/chisel3/Mem.scala +++ b/core/src/main/scala/chisel3/Mem.scala @@ -56,7 +56,9 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) with SourceInfoDoc { _parent.foreach(_.addId(this)) - private val clockInst: Clock = Builder.forcedClock + // if the memory is created in a scope with an implicit clock (-> clockInst is defined), we will perform checks that + // ensure memory ports are created with the same clock unless explicitly specified to use a different clock + private val clockInst: Option[Clock] = Builder.currentClock protected def clockWarning(sourceInfo: Option[SourceInfo]): Unit = { // Turn into pretty String if possible, if not, Builder.deprecated will find one via stack trace @@ -132,7 +134,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) implicit sourceInfo: SourceInfo, compileOptions: CompileOptions ): T = { - if (warn && clock != clockInst) { + if (warn && clockInst.isDefined && clock != clockInst.get) { clockWarning(Some(sourceInfo)) } makePort(sourceInfo, idx, dir, clock) @@ -164,7 +166,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) )( implicit compileOptions: CompileOptions ): Unit = { - if (warn && clock != clockInst) { + if (warn && clockInst.isDefined && clock != clockInst.get) { clockWarning(None) } implicit val sourceInfo = UnlocatableSourceInfo @@ -223,7 +225,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) compileOptions: CompileOptions ): Unit = { implicit val sourceInfo = UnlocatableSourceInfo - if (warn && clock != clockInst) { + if (warn && clockInst.isDefined && clock != clockInst.get) { clockWarning(None) } val accessor = makePort(sourceInfo, idx, MemPortDirection.WRITE, clock).asInstanceOf[Vec[Data]] diff --git a/src/test/scala/chiselTests/Mem.scala b/src/test/scala/chiselTests/Mem.scala index 4dcb1ad4..c5fcc6b1 100644 --- a/src/test/scala/chiselTests/Mem.scala +++ b/src/test/scala/chiselTests/Mem.scala @@ -3,6 +3,7 @@ package chiselTests import chisel3._ +import chisel3.stage.ChiselStage import chisel3.util._ import chisel3.testers.BasicTester @@ -141,6 +142,52 @@ class MemBundleTester extends BasicTester { } } +private class TrueDualPortMemoryIO(val addrW: Int, val dataW: Int) extends Bundle { + require(addrW > 0, "address width must be greater than 0") + require(dataW > 0, "data width must be greater than 0") + + val clka = Input(Clock()) + val ena = Input(Bool()) + val wea = Input(Bool()) + val addra = Input(UInt(addrW.W)) + val dia = Input(UInt(dataW.W)) + val doa = Output(UInt(dataW.W)) + + val clkb = Input(Clock()) + val enb = Input(Bool()) + val web = Input(Bool()) + val addrb = Input(UInt(addrW.W)) + val dib = Input(UInt(dataW.W)) + val dob = Output(UInt(dataW.W)) +} + +private class TrueDualPortMemory(addrW: Int, dataW: Int) extends RawModule { + val io = IO(new TrueDualPortMemoryIO(addrW, dataW)) + val ram = SyncReadMem(1 << addrW, UInt(dataW.W)) + + // Port a + withClock(io.clka) { + io.doa := DontCare + when(io.ena) { + when(io.wea) { + ram(io.addra) := io.dia + } + io.doa := ram(io.addra) + } + } + + // Port b + withClock(io.clkb) { + io.dob := DontCare + when(io.enb) { + when(io.web) { + ram(io.addrb) := io.dib + } + io.dob := ram(io.addrb) + } + } +} + class MemorySpec extends ChiselPropSpec { property("Mem of Vec should work") { assertTesterPasses { new MemVecTester } @@ -186,4 +233,9 @@ class MemorySpec extends ChiselPropSpec { |} |""".stripMargin should compile } + + property("memories in modules without implicit clock should compile without warning or error") { + val stage = new ChiselStage + stage.emitVerilog(new TrueDualPortMemory(4, 32)) + } } -- cgit v1.2.3 From a16a8a52a3b2d72d80a27434217aaeba7be2d3a8 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 20 Apr 2022 21:10:07 +0000 Subject: Generate a balanced tree with reduceTree (#2318) (#2499) The difference in logic depth for various paths now has a maximum of 1. Also make treeReduce order the same for 2.12 and 2.13 .grouped(_) returns an Iterator .toSeq on an Iterator returns a Stream in 2.12 and a List in 2.13 This can lead to changes in order when bumping from 2.12 to 2.13 that can be avoided by simply using an eager collection explicitly. Co-authored-by: Jack Koenig (cherry picked from commit 6975f77f3325dec46c613552eac663c29011a67c) Co-authored-by: Martin Schoeberl --- core/src/main/scala/chisel3/Aggregate.scala | 28 ++++++- src/test/scala/chiselTests/ReduceTreeSpec.scala | 106 ++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 4 deletions(-) create mode 100644 src/test/scala/chiselTests/ReduceTreeSpec.scala diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 06ae36f3..cc5b83d9 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -14,6 +14,7 @@ import chisel3.internal.Builder.pushCommand import chisel3.internal.firrtl._ import chisel3.internal.sourceinfo._ +import java.lang.Math.{floor, log10, pow} import scala.collection.mutable class AliasedAggregateFieldException(message: String) extends ChiselException(message) @@ -381,11 +382,30 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend compileOptions: CompileOptions ): T = { require(!isEmpty, "Cannot apply reduction on a vec of size 0") - var curLayer: Seq[T] = this - while (curLayer.length > 1) { - curLayer = curLayer.grouped(2).map(x => if (x.length == 1) layerOp(x(0)) else redOp(x(0), x(1))).toSeq + + def recReduce[T](s: Seq[T], op: (T, T) => T, lop: (T) => T): T = { + + val n = s.length + n match { + case 1 => lop(s(0)) + case 2 => op(s(0), s(1)) + case _ => + val m = pow(2, floor(log10(n - 1) / log10(2))).toInt // number of nodes in next level, will be a power of 2 + val p = 2 * m - n // number of nodes promoted + + val l = s.take(p).map(lop) + val r = s + .drop(p) + .grouped(2) + .map { + case Seq(a, b) => op(a, b) + } + .toVector + recReduce(l ++ r, op, lop) + } } - curLayer(0) + + recReduce(this, redOp, layerOp) } /** Creates a Vec literal of this type with specified values. this must be a chisel type. diff --git a/src/test/scala/chiselTests/ReduceTreeSpec.scala b/src/test/scala/chiselTests/ReduceTreeSpec.scala new file mode 100644 index 00000000..3f078106 --- /dev/null +++ b/src/test/scala/chiselTests/ReduceTreeSpec.scala @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chiselTests + +import chisel3._ +import chisel3.util._ +import chisel3.testers.BasicTester + +class Arbiter[T <: Data: Manifest](n: Int, private val gen: T) extends Module { + val io = IO(new Bundle { + val in = Flipped(Vec(n, new DecoupledIO(gen))) + val out = new DecoupledIO(gen) + }) + + def arbitrateTwo(a: DecoupledIO[T], b: DecoupledIO[T]) = { + + val idleA :: idleB :: hasA :: hasB :: Nil = Enum(4) + val regData = Reg(gen) + val regState = RegInit(idleA) + val out = Wire(new DecoupledIO(gen)) + + a.ready := regState === idleA + b.ready := regState === idleB + out.valid := (regState === hasA || regState === hasB) + + switch(regState) { + is(idleA) { + when(a.valid) { + regData := a.bits + regState := hasA + }.otherwise { + regState := idleB + } + } + is(idleB) { + when(b.valid) { + regData := b.bits + regState := hasB + }.otherwise { + regState := idleA + } + } + is(hasA) { + when(out.ready) { + regState := idleB + } + } + is(hasB) { + when(out.ready) { + regState := idleA + } + } + } + + out.bits := regData.asUInt + 1.U + out + } + + io.out <> io.in.reduceTree(arbitrateTwo) +} + +class ReduceTreeBalancedTester(nodes: Int) extends BasicTester { + + val cnt = RegInit(0.U(8.W)) + val min = RegInit(99.U(8.W)) + val max = RegInit(0.U(8.W)) + + val dut = Module(new Arbiter(nodes, UInt(16.W))) + for (i <- 0 until nodes) { + dut.io.in(i).valid := true.B + dut.io.in(i).bits := 0.U + } + dut.io.out.ready := true.B + + when(dut.io.out.valid) { + val hops = dut.io.out.bits + when(hops < min) { + min := hops + } + when(hops > max) { + max := hops + } + } + + when(!(max === 0.U || min === 99.U)) { + assert(max - min <= 1.U) + } + + cnt := cnt + 1.U + when(cnt === 10.U) { + stop() + } +} + +class ReduceTreeBalancedSpec extends ChiselPropSpec { + property("Tree shall be fair and shall have a maximum difference of one hop for each node") { + + // This test will fail for 5 nodes due to an unbalanced tree. + // A fix is on the way. + for (n <- 1 to 5) { + assertTesterPasses { + new ReduceTreeBalancedTester(n) + } + } + } +} -- cgit v1.2.3 From 5a90f27ab1311994b4df73a85cd0facab3ae0b3a Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 25 Apr 2022 18:39:41 +0000 Subject: Fix warning injected into user code by @chiselName (#2500) (#2503) In Scala 2.13, Auto-application to `()` is deprecated. Any nullary method (ie. a method with no arguments) that is defined with () must now be called with (). @chiselName used to inject a case of this warning into user code which would then cause a warning on the @chiselName macro that the user has no way to fix. (cherry picked from commit ca902680df772445371e6c3dd907c01113afb1f5) Co-authored-by: Jack Koenig --- macros/src/main/scala/chisel3/internal/naming/NamingAnnotations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/macros/src/main/scala/chisel3/internal/naming/NamingAnnotations.scala b/macros/src/main/scala/chisel3/internal/naming/NamingAnnotations.scala index 82223d78..b12826fc 100644 --- a/macros/src/main/scala/chisel3/internal/naming/NamingAnnotations.scala +++ b/macros/src/main/scala/chisel3/internal/naming/NamingAnnotations.scala @@ -125,7 +125,7 @@ class NamingTransforms(val c: Context) { q""" val $contextVar = $globalNamingStack.pushContext() ..$transformedBody - if($globalNamingStack.length == 1){ + if($globalNamingStack.length() == 1){ $contextVar.namePrefix("") } $globalNamingStack.popReturnContext(this, $contextVar) -- cgit v1.2.3 From 3e551ef38fb4c01b9b46f45dc68c2d9f5c9e9acb Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 25 Apr 2022 23:19:37 +0000 Subject: Fix error message for BlackBox without val io <: Record (#2504) (#2505) (cherry picked from commit f9aee1f72744abc6ee13aafc4d1a51a2783cbab8) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/BlackBox.scala | 2 +- src/test/scala/chiselTests/BlackBox.scala | 29 ++++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala index 2f04fb2e..f3fc2711 100644 --- a/core/src/main/scala/chisel3/BlackBox.scala +++ b/core/src/main/scala/chisel3/BlackBox.scala @@ -157,7 +157,7 @@ abstract class BlackBox( _compatAutoWrapPorts() // pre-IO(...) compatibility hack // Restrict IO to just io, clock, and reset - if (!_io.forall(portsContains)) { + if (!_io.exists(portsContains)) { throwException(s"BlackBox '$this' must have a port named 'io' of type Record wrapped in IO(...)!") } diff --git a/src/test/scala/chiselTests/BlackBox.scala b/src/test/scala/chiselTests/BlackBox.scala index 27cdbbc4..f923a94a 100644 --- a/src/test/scala/chiselTests/BlackBox.scala +++ b/src/test/scala/chiselTests/BlackBox.scala @@ -145,6 +145,17 @@ class BlackBoxTypeParam(w: Int, raw: String) extends BlackBox(Map("T" -> RawPara }) } +class BlackBoxNoIO extends BlackBox { + // Whoops! typo + val ioo = IO(new Bundle { + val out = Output(UInt(8.W)) + }) +} + +class BlackBoxUIntIO extends BlackBox { + val io = IO(Output(UInt(8.W))) +} + class BlackBoxWithParamsTester extends BasicTester { val blackBoxOne = Module(new BlackBoxConstant(1)) val blackBoxFour = Module(new BlackBoxConstant(4)) @@ -192,7 +203,23 @@ class BlackBoxSpec extends ChiselFlatSpec { assert(DataMirror.modulePorts(m) == Seq("in" -> m.io.in, "out" -> m.io.out)) }) } - "A BlackBoxed using suggestName(\"io\")" should "work (but don't do this)" in { + "A BlackBox using suggestName(\"io\")" should "work (but don't do this)" in { assertTesterPasses({ new BlackBoxTesterSuggestName }, Seq("/chisel3/BlackBoxTest.v"), TesterDriver.verilatorOnly) } + + "A BlackBox with no 'val io'" should "give a reasonable error message" in { + (the[ChiselException] thrownBy { + ChiselStage.elaborate(new Module { + val inst = Module(new BlackBoxNoIO) + }) + }).getMessage should include("must have a port named 'io' of type Record") + } + + "A BlackBox with non-Record 'val io'" should "give a reasonable error message" in { + (the[ChiselException] thrownBy { + ChiselStage.elaborate(new Module { + val inst = Module(new BlackBoxUIntIO) + }) + }).getMessage should include("must have a port named 'io' of type Record") + } } -- cgit v1.2.3 From d5a964f6e7beea1f38f9623224fc65e2397e1fe7 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 26 Apr 2022 00:49:16 +0000 Subject: Fix spurious warning from Bundle plugin (#2506) (#2507) For traits or abstract classes that extend Bundle but have no concrete methods, the plugin will print a benign warning that the user cannot fix. It will no longer print that warning. (cherry picked from commit ed4a98188ee1fe43efbd3a1ba43a657a128764d6) Co-authored-by: Jack Koenig --- plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala b/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala index d768175d..e3ec0a04 100644 --- a/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala +++ b/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala @@ -111,7 +111,7 @@ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPlugi override def transform(tree: Tree): Tree = tree match { - case bundle: ClassDef if isBundle(bundle.symbol) => + case bundle: ClassDef if isBundle(bundle.symbol) && !bundle.mods.hasFlag(Flag.ABSTRACT) => // ==================== Generate _cloneTypeImpl ==================== val (con, params) = getConstructorAndParams(bundle.impl.body) if (con.isEmpty) { -- cgit v1.2.3