From 12785c6b2b8e378a5aa9db1833df7486d8f2a486 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 9 Jan 2023 22:57:22 +0000 Subject: Improve performance of ChiselEnum annotations (#2923) (#2927) ChiselEnums check if they should create annotations every time an instance of them is bound. Because so many annotations would be created, they check to see if an equivalent annotation has already been added to the annotations. Previously, this used a linear search of the annotations, now it uses a HashSet. (cherry picked from commit 96bde092e449281dc70ebdb05f21695468c3e5fa) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'core/src/main/scala/chisel3/internal') diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index d06b7992..fe7d7bea 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -485,6 +485,10 @@ private[chisel3] class DynamicContext( val newAnnotations = ArrayBuffer[ChiselMultiAnnotation]() var currentModule: Option[BaseModule] = None + // Enum annotations are added every time a StrongEnum is bound + // To keep the number down, we keep them unique in the annotations + val enumAnnos = mutable.HashSet[ChiselAnnotation]() + /** Contains a mapping from a elaborated module to their aspect * Set by [[ModuleAspect]] */ @@ -553,6 +557,8 @@ private[chisel3] object Builder extends LazyLogging { def components: ArrayBuffer[Component] = dynamicContext.components def annotations: ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations + def enumAnnos: mutable.HashSet[ChiselAnnotation] = dynamicContext.enumAnnos + // TODO : Unify this with annotations in the future - done this way for backward compatability def newAnnotations: ArrayBuffer[ChiselMultiAnnotation] = dynamicContext.newAnnotations -- cgit v1.2.3 From 9a7945fd86fcad02da0556d8f4a30daa4b005f9d Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 10 Jan 2023 07:19:45 +0000 Subject: Check for Vec subaccess in NamedComponent and throw a nicer error. (backport #2907) (#2928) * Check for Vec subaccess in NamedComponent and throw a nicer error. (#2907) This would previously end up throwing an exception later, when trying to create a component name and realizing that it was invalid. Instead, this detects Vec subaccesses early, and gives a more precise error and suggestion. (cherry picked from commit d8c30961c7b293ee19024a487698630367ee71c6) # Conflicts: # core/src/main/scala/chisel3/internal/Builder.scala * Resolve backport conflicts Co-authored-by: Mike Urbach --- core/src/main/scala/chisel3/internal/Builder.scala | 46 ++++++++++++++++------ 1 file changed, 35 insertions(+), 11 deletions(-) (limited to 'core/src/main/scala/chisel3/internal') diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index fe7d7bea..be2c4cfb 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -299,25 +299,31 @@ private[chisel3] trait HasId extends InstanceId { // Builder.deprecated mechanism, we have to create our own one off ErrorLog and print the // warning right away. // It's especially bad because --warnings-as-errors does not work with these warnings - val nameGuess = _computeName(None) match { - case Some(name) => s": '$name'" - case None => "" - } - val parentGuess = _parent match { - case Some(ViewParent) => s", in module '${reifyParent.pathName}'" - case Some(p) => s", in module '${p.pathName}'" - case None => "" - } val errors = new ErrorLog(false) val logger = new _root_.logger.Logger(this.getClass.getName) val msg = - "Accessing the .instanceName or .toTarget of non-hardware Data is deprecated" + nameGuess + parentGuess + ". " + + "Accessing the .instanceName or .toTarget of non-hardware Data is deprecated" + _errorContext + ". " + "This will become an error in Chisel 3.6." errors.deprecated(msg, None) errors.checkpoint(logger) _computeName(None).get } + private[chisel3] def _errorContext: String = { + val nameGuess: String = _computeName(None) match { + case Some(name) => s": '$name'" + case None => "" + } + + val parentGuess: String = _parent match { + case Some(ViewParent) => s", in module '${reifyParent.pathName}'" + case Some(p) => s", in module '${p.pathName}'" + case None => "" + } + + nameGuess + parentGuess + } + // Helper for reifying views if they map to a single Target private[chisel3] def reifyTarget: Option[Data] = this match { case d: Data => reifySingleData(d) // Only Data can be views @@ -393,13 +399,16 @@ private[chisel3] trait NamedComponent extends HasId { /** Returns a FIRRTL ComponentName that references this object * @note Should not be called until circuit elaboration is complete */ - final def toNamed: ComponentName = + final def toNamed: ComponentName = { + assertValidTarget() ComponentName(this.instanceName, ModuleName(this.parentModName, CircuitName(this.circuitName))) + } /** Returns a FIRRTL ReferenceTarget that references this object * @note Should not be called until circuit elaboration is complete */ final def toTarget: ReferenceTarget = { + assertValidTarget() val name = this.instanceName if (!validComponentName(name)) throwException(s"Illegal component name: $name (note: literals are illegal)") import _root_.firrtl.annotations.{Target, TargetToken} @@ -423,6 +432,21 @@ private[chisel3] trait NamedComponent extends HasId { case None => localTarget } } + + private def assertValidTarget(): Unit = { + val isVecSubaccess = getOptionRef.map { + case Index(_, _: ULit) => true // Vec literal indexing + case Index(_, _: Node) => true // Vec dynamic indexing + case _ => false + }.getOrElse(false) + + if (isVecSubaccess) { + throwException( + s"You cannot target Vec subaccess" + _errorContext + + ". Instead, assign it to a temporary (for example, with WireInit) and target the temporary." + ) + } + } } // Mutable global state for chisel that can appear outside a Builder context -- cgit v1.2.3 From d4570fb9d29371c35641ba79b76662f99677f192 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 11 Jan 2023 22:40:32 +0000 Subject: Promote ChiselEnum user APIs from experimental (backport #2929) (#2931) * Promote ChiselEnum user APIs from experimental (#2929) They are commonly used and if we were ever to change them, we will need to go through a deprecation process anyway. Note that the EnumAnnotations remain in chisel3.experimental because, like all Chisel annotation support, they are slated to be deprecated and removed. (cherry picked from commit 424e9446f1675fe0168e22bdfbbe85db997376e6) # Conflicts: # docs/src/cookbooks/verilog-vs-chisel.md # docs/src/explanations/chisel-enum.md * Maintain binary compatbility, use aliases in chisel3 package * Deprecate the actual classes in chisel3.experimental * Also fix backport conflicts Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 2 +- core/src/main/scala/chisel3/internal/MonoConnect.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'core/src/main/scala/chisel3/internal') diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index be2c4cfb..ab1435c5 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -509,7 +509,7 @@ private[chisel3] class DynamicContext( val newAnnotations = ArrayBuffer[ChiselMultiAnnotation]() var currentModule: Option[BaseModule] = None - // Enum annotations are added every time a StrongEnum is bound + // Enum annotations are added every time a ChiselEnum is bound // To keep the number down, we keep them unique in the annotations val enumAnnos = mutable.HashSet[ChiselAnnotation]() diff --git a/core/src/main/scala/chisel3/internal/MonoConnect.scala b/core/src/main/scala/chisel3/internal/MonoConnect.scala index 4e762a7c..a0cca4a6 100644 --- a/core/src/main/scala/chisel3/internal/MonoConnect.scala +++ b/core/src/main/scala/chisel3/internal/MonoConnect.scala @@ -3,7 +3,7 @@ package chisel3.internal import chisel3._ -import chisel3.experimental.{Analog, BaseModule, EnumType, FixedPoint, Interval, UnsafeEnum} +import chisel3.experimental.{Analog, BaseModule, FixedPoint, Interval, UnsafeEnum} import chisel3.internal.Builder.pushCommand import chisel3.internal.firrtl.{Connect, Converter, DefInvalid} import chisel3.experimental.dataview.{isView, reify, reifyToAggregate} -- cgit v1.2.3