From db18ae16a26dab5231ca83172c88b9735a977582 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 5 Aug 2022 00:59:23 +0000 Subject: Replace some options with nullable vars (backport #2658) (#2659) * Replace some options with nullable vars (#2658) Co-authored-by: Jack Koenig (cherry picked from commit ac460bfeb16c8e7d0dc00975bb03f73c0fea2103) # Conflicts: # core/src/main/scala/chisel3/internal/Builder.scala * Fix backport conflicts (#2661) Co-authored-by: Zachary Yedidia --- core/src/main/scala/chisel3/Data.scala | 10 +++++--- core/src/main/scala/chisel3/internal/Builder.scala | 30 +++++++++++++++------- 2 files changed, 27 insertions(+), 13 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 592ebe25..956c7996 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -501,14 +501,15 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { // Binding stores information about this node's position in the hardware graph. // This information is supplemental (more than is necessary to generate FIRRTL) and is used to // perform checks in Chisel, where more informative error messages are possible. - private var _binding: Option[Binding] = None + private var _bindingVar: Binding = null // using nullable var for better memory usage + private def _binding: Option[Binding] = Option(_bindingVar) // Only valid after node is bound (synthesizable), crashes otherwise protected[chisel3] def binding: Option[Binding] = _binding protected def binding_=(target: Binding) { if (_binding.isDefined) { throw RebindingException(s"Attempted reassignment of binding to $this, from: ${target}") } - _binding = Some(target) + _bindingVar = target } // Similar to topBindingOpt except it explicitly excludes SampleElements which are bound but not @@ -540,14 +541,15 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { // Both are only valid after binding is set. // Direction of this node, accounting for parents (force Input / Output) and children. - private var _direction: Option[ActualDirection] = None + private var _directionVar: ActualDirection = null // using nullable var for better memory usage + private def _direction: Option[ActualDirection] = Option(_directionVar) private[chisel3] def direction: ActualDirection = _direction.get private[chisel3] def direction_=(actualDirection: ActualDirection) { if (_direction.isDefined) { throw RebindingException(s"Attempted reassignment of resolved direction to $this") } - _direction = Some(actualDirection) + _directionVar = actualDirection } private[chisel3] def stringAccessor(chiselType: String): String = { diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 61f94f8f..25f4c44c 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -88,10 +88,19 @@ trait InstanceId { private[chisel3] trait HasId extends InstanceId { private[chisel3] def _onModuleClose: Unit = {} - private[chisel3] var _parent: Option[BaseModule] = Builder.currentModule + // using nullable var for better memory usage + private var _parentVar: BaseModule = Builder.currentModule.getOrElse(null) + private[chisel3] def _parent: Option[BaseModule] = Option(_parentVar) + private[chisel3] def _parent_=(target: Option[BaseModule]): Unit = { + _parentVar = target.getOrElse(null) + } // Set if the returned top-level module of a nested call to the Chisel Builder, see Definition.apply - private[chisel3] var _circuit: Option[BaseModule] = None + private var _circuitVar: BaseModule = null // using nullable var for better memory usage + private[chisel3] def _circuit: Option[BaseModule] = Option(_circuitVar) + private[chisel3] def _circuit_=(target: Option[BaseModule]): Unit = { + _circuitVar = target.getOrElse(null) + } private[chisel3] val _id: Long = Builder.idGen.next @@ -100,10 +109,12 @@ private[chisel3] trait HasId extends InstanceId { override def equals(that: Any): Boolean = super.equals(that) // Contains suggested seed (user-decided seed) - private var suggested_seed: Option[String] = None + private var suggested_seedVar: String = null // using nullable var for better memory usage + private def suggested_seed: Option[String] = Option(suggested_seedVar) // Contains the seed computed automatically by the compiler plugin - private var auto_seed: Option[String] = None + private var auto_seedVar: String = null // using nullable var for better memory usage + private def auto_seed: Option[String] = Option(auto_seedVar) // Prefix for use in naming // - Defaults to prefix at time when object is created @@ -131,7 +142,7 @@ private[chisel3] trait HasId extends InstanceId { private[chisel3] def autoSeed(seed: String): this.type = forceAutoSeed(seed) // 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) + auto_seedVar = seed for (hook <- auto_postseed_hooks.reverse) { hook(seed) } naming_prefix = Builder.getPrefix this @@ -159,7 +170,7 @@ private[chisel3] trait HasId extends InstanceId { * @return this object */ def suggestName(seed: => String): this.type = { - if (suggested_seed.isEmpty) suggested_seed = Some(seed) + if (suggested_seed.isEmpty) suggested_seedVar = seed naming_prefix = Builder.getPrefix for (hook <- suggest_postseed_hooks.reverse) { hook(seed) } this @@ -171,7 +182,7 @@ private[chisel3] trait HasId extends InstanceId { private[chisel3] def forceFinalName(seed: String): this.type = { // This could be called with user prefixes, ignore them noPrefix { - suggested_seed = Some(seed) + suggested_seedVar = seed this.suggestName(seed) } } @@ -212,11 +223,12 @@ private[chisel3] trait HasId extends InstanceId { naming_prefix = Nil } - private var _ref: Option[Arg] = None + private var _refVar: Arg = null // using nullable var for better memory usage + private def _ref: Option[Arg] = Option(_refVar) private[chisel3] def setRef(imm: Arg): Unit = setRef(imm, false) private[chisel3] def setRef(imm: Arg, force: Boolean): Unit = { if (_ref.isEmpty || force) { - _ref = Some(imm) + _refVar = imm } } private[chisel3] def setRef(parent: HasId, name: String): Unit = setRef(Slot(Node(parent), name)) -- cgit v1.2.3 From 7bad3d2ec316f24f3da79d1dfef19e128cfe8bf5 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 12 Aug 2022 20:04:33 +0000 Subject: Add ability to suppress enum cast warnings (#2671) (#2674) (cherry picked from commit 1ad820f7f549eddcd7188b737f59a240e48a7f0a) Co-authored-by: Zachary Yedidia --- core/src/main/scala/chisel3/StrongEnum.scala | 32 +++++++++++++++++++++- core/src/main/scala/chisel3/internal/Builder.scala | 4 +++ 2 files changed, 35 insertions(+), 1 deletion(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/StrongEnum.scala b/core/src/main/scala/chisel3/StrongEnum.scala index cd6f11ee..6d8ceb2f 100644 --- a/core/src/main/scala/chisel3/StrongEnum.scala +++ b/core/src/main/scala/chisel3/StrongEnum.scala @@ -339,7 +339,7 @@ abstract class EnumFactory { } else if (n.getWidth > this.getWidth) { throwException(s"The UInt being cast to $enumTypeName is wider than $enumTypeName's width ($getWidth)") } else { - if (warn && !this.isTotal) { + if (!Builder.suppressEnumCastWarning && warn && !this.isTotal) { Builder.warning( s"Casting non-literal UInt to $enumTypeName. You can use $enumTypeName.safe to cast without this warning." ) @@ -409,3 +409,33 @@ private[chisel3] class UnsafeEnum(override val width: Width) extends EnumType(Un override def cloneType: this.type = new UnsafeEnum(width).asInstanceOf[this.type] } private object UnsafeEnum extends EnumFactory + +/** Suppress enum cast warnings + * + * Users should use [[EnumFactory.safe .safe]] when possible. + * + * This is primarily used for casting from [[UInt]] to a Bundle type that contains an Enum. + * {{{ + * class MyBundle extends Bundle { + * val addr = UInt(8.W) + * val op = OpEnum() + * } + * + * // Since this is a cast to a Bundle, cannot use OpCode.safe + * val bundle = suppressEnumCastWarning { + * someUInt.asTypeOf(new MyBundle) + * } + * }}} + */ +object suppressEnumCastWarning { + def apply[T](block: => T): T = { + val parentWarn = Builder.suppressEnumCastWarning + + Builder.suppressEnumCastWarning = true + + val res = block // execute block + + Builder.suppressEnumCastWarning = parentWarn + res + } +} diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 25f4c44c..b2c98309 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -435,6 +435,7 @@ private[chisel3] class DynamicContext( var currentReset: Option[Reset] = None val errors = new ErrorLog val namingStack = new NamingStack + // Used to indicate if this is the top-level module of full elaboration, or from a Definition var inDefinition: Boolean = false } @@ -451,6 +452,9 @@ private[chisel3] object Builder extends LazyLogging { dynamicContextVar.value.get } + // Used to suppress warnings when casting from a UInt to an Enum + var suppressEnumCastWarning: Boolean = false + // Returns the current dynamic context def captureContext(): DynamicContext = dynamicContext // Sets the current dynamic contents -- cgit v1.2.3 From d344e8a91bdbfedc28527c3fc7d6d243dff9e3e6 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 12 Aug 2022 20:56:42 +0000 Subject: Show equivalent warnings/errors only once (#2673) (#2675) (cherry picked from commit ae76ff4cb303a6646e48dc044be47051b67e7cbb) Co-authored-by: Zachary Yedidia --- core/src/main/scala/chisel3/internal/Error.scala | 41 ++++++++++++++---------- 1 file changed, 24 insertions(+), 17 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/internal/Error.scala b/core/src/main/scala/chisel3/internal/Error.scala index 3b0846eb..98db7109 100644 --- a/core/src/main/scala/chisel3/internal/Error.scala +++ b/core/src/main/scala/chisel3/internal/Error.scala @@ -177,20 +177,31 @@ private[chisel3] object ErrorLog { } private[chisel3] class ErrorLog { + def getLoc(loc: Option[StackTraceElement]): String = { + loc match { + case Some(elt: StackTraceElement) => s"${elt.getFileName}:${elt.getLineNumber}" + case None => "(unknown)" + } + } /** Log an error message */ - def error(m: => String): Unit = - errors += new Error(m, getUserLineNumber) + def error(m: => String): Unit = { + val loc = getUserLineNumber + errors += (((m, getLoc(loc)), new Error(m, loc))) + } /** Log a warning message */ - def warning(m: => String): Unit = - errors += new Warning(m, getUserLineNumber) - - /** Log a warning message without a source locator */ - def warningNoLoc(m: => String): Unit = { - errors += new Warning(m, None) + def warning(m: => String): Unit = { + val loc = getUserLineNumber + errors += (((m, getLoc(loc)), new Warning(m, loc))) } + /** Log a warning message without a source locator. This is used when the + * locator wouldn't be helpful (e.g., due to lazy values). + */ + def warningNoLoc(m: => String): Unit = + errors += (((m, ""), new Warning(m, None))) + /** Emit an informational message */ @deprecated("This method will be removed in 3.5", "3.4") def info(m: String): Unit = @@ -200,11 +211,7 @@ private[chisel3] class ErrorLog { def deprecated(m: => String, location: Option[String]): Unit = { val sourceLoc = location match { case Some(loc) => loc - case None => - getUserLineNumber match { - case Some(elt: StackTraceElement) => s"${elt.getFileName}:${elt.getLineNumber}" - case None => "(unknown)" - } + case None => getLoc(getUserLineNumber) } val thisEntry = (m, sourceLoc) @@ -217,7 +224,7 @@ private[chisel3] class ErrorLog { case ((message, sourceLoc), count) => logger.warn(s"${ErrorLog.depTag} $sourceLoc ($count calls): $message") } - errors.foreach(e => logger.error(e.toString)) + errors.foreach(e => logger.error(e._2.toString)) if (!deprecations.isEmpty) { logger.warn( @@ -233,8 +240,8 @@ private[chisel3] class ErrorLog { logger.warn(s"""${ErrorLog.warnTag} scalacOptions := Seq("-unchecked", "-deprecation")""") } - val allErrors = errors.filter(_.isFatal) - val allWarnings = errors.filter(!_.isFatal) + val allErrors = errors.filter(_._2.isFatal) + val allWarnings = errors.filter(!_._2.isFatal) if (!allWarnings.isEmpty && !allErrors.isEmpty) { logger.warn( @@ -289,7 +296,7 @@ private[chisel3] class ErrorLog { .headOption } - private val errors = ArrayBuffer[LogEntry]() + private val errors = LinkedHashMap[(String, String), LogEntry]() private val deprecations = LinkedHashMap[(String, String), Int]() private val startTime = System.currentTimeMillis -- cgit v1.2.3 From c4dec947d54a52c3092bd7855180d42afaae3776 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Sat, 13 Aug 2022 00:17:56 +0000 Subject: Add option to treat warnings as errors (backport #2676) (#2677) * Add option to treat warnings as errors (#2676) Add --warnings-as-errors option (cherry picked from commit 498946663726955c380a1e420f5d7b9630000aad) # Conflicts: # core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala # core/src/main/scala/chisel3/internal/Builder.scala # src/main/scala/chisel3/aop/injecting/InjectingAspect.scala # src/main/scala/chisel3/stage/ChiselOptions.scala # src/main/scala/chisel3/stage/package.scala # src/main/scala/chisel3/stage/phases/Elaborate.scala * Resolve backport conflicts Co-authored-by: Zachary Yedidia Co-authored-by: Jack Koenig --- .../main/scala/chisel3/experimental/hierarchy/Definition.scala | 2 +- core/src/main/scala/chisel3/internal/Builder.scala | 8 +++++--- core/src/main/scala/chisel3/internal/Error.scala | 9 ++++++--- 3 files changed, 12 insertions(+), 7 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala b/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala index 36bf6f87..99eacc7d 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala @@ -103,7 +103,7 @@ object Definition extends SourceInfoDoc { ): Definition[T] = { val dynamicContext = { val context = Builder.captureContext() - new DynamicContext(Nil, context.throwOnFirstError, context.warnReflectiveNaming) + new DynamicContext(Nil, context.throwOnFirstError, context.warnReflectiveNaming, context.warningsAsErrors) } Builder.globalNamespace.copyTo(dynamicContext.globalNamespace) dynamicContext.inDefinition = true diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index b2c98309..75d08f92 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -244,7 +244,8 @@ private[chisel3] trait HasId extends InstanceId { // These accesses occur after Chisel elaboration so we cannot use the normal // Builder.deprecated mechanism, we have to create our own one off ErrorLog and print the // warning right away. - val errors = new ErrorLog + // It's especially bad because --warnings-as-errors does not work with these warnings + 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. " + "This will become an error in Chisel 3.6." @@ -376,7 +377,8 @@ private[chisel3] class ChiselContext() { private[chisel3] class DynamicContext( val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean, - val warnReflectiveNaming: Boolean) { + val warnReflectiveNaming: Boolean, + val warningsAsErrors: Boolean) { val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a } // Map holding the actual names of extModules @@ -433,7 +435,7 @@ private[chisel3] class DynamicContext( var whenStack: List[WhenContext] = Nil var currentClock: Option[Clock] = None var currentReset: Option[Reset] = None - val errors = new ErrorLog + val errors = new ErrorLog(warningsAsErrors) val namingStack = new NamingStack // Used to indicate if this is the top-level module of full elaboration, or from a Definition diff --git a/core/src/main/scala/chisel3/internal/Error.scala b/core/src/main/scala/chisel3/internal/Error.scala index 98db7109..da968d2a 100644 --- a/core/src/main/scala/chisel3/internal/Error.scala +++ b/core/src/main/scala/chisel3/internal/Error.scala @@ -176,7 +176,7 @@ private[chisel3] object ErrorLog { val errTag = s"[${Console.RED}error${Console.RESET}]" } -private[chisel3] class ErrorLog { +private[chisel3] class ErrorLog(warningsAsErrors: Boolean) { def getLoc(loc: Option[StackTraceElement]): String = { loc match { case Some(elt: StackTraceElement) => s"${elt.getFileName}:${elt.getLineNumber}" @@ -190,17 +190,20 @@ private[chisel3] class ErrorLog { errors += (((m, getLoc(loc)), new Error(m, loc))) } + private def warn(m: => String, loc: Option[StackTraceElement]): LogEntry = + if (warningsAsErrors) new Error(m, loc) else new Warning(m, loc) + /** Log a warning message */ def warning(m: => String): Unit = { val loc = getUserLineNumber - errors += (((m, getLoc(loc)), new Warning(m, loc))) + errors += (((m, getLoc(loc)), warn(m, loc))) } /** Log a warning message without a source locator. This is used when the * locator wouldn't be helpful (e.g., due to lazy values). */ def warningNoLoc(m: => String): Unit = - errors += (((m, ""), new Warning(m, None))) + errors += (((m, ""), warn(m, None))) /** Emit an informational message */ @deprecated("This method will be removed in 3.5", "3.4") -- cgit v1.2.3 From 29702d7dd10a9eb7595a102bfffcee665bcf7394 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 15 Aug 2022 20:37:55 +0000 Subject: Printables for verification preconditions (backport #2663) (#2680) * Printables for verification preconditions (#2663) Add support for printable within assert and assume verification statements Co-authored-by: Girish Pai Co-authored-by: Megan Wachs Co-authored-by: Jack Koenig (cherry picked from commit 7df5653309b5e48e1732b335610d9a7e8449f903) * Waive MiMa false positive Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com> Co-authored-by: Jack Koenig --- .../main/scala/chisel3/VerificationStatement.scala | 230 ++++++++++++++++++--- 1 file changed, 198 insertions(+), 32 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/VerificationStatement.scala b/core/src/main/scala/chisel3/VerificationStatement.scala index 7229c412..1b13b86c 100644 --- a/core/src/main/scala/chisel3/VerificationStatement.scala +++ b/core/src/main/scala/chisel3/VerificationStatement.scala @@ -11,37 +11,71 @@ import chisel3.internal.sourceinfo.SourceInfo import scala.reflect.macros.blackbox -object assert { +/** Scaladoc information for internal verification statement macros + * that are used in objects assert, assume and cover. + * + * @groupdesc VerifPrintMacros + * + *

+ * '''These internal methods are not part of the public-facing API!''' + *

+ *
+ * + * @groupprio VerifPrintMacros 1001 + */ +trait VerifPrintMacrosDoc + +object assert extends VerifPrintMacrosDoc { + + /** Checks for a condition to be valid in the circuit at rising clock edge + * when not in reset. If the condition evaluates to false, the circuit + * simulation stops with an error. + * + * @param cond condition, assertion fires (simulation fails) when false + * @param message optional format string to print when the assertion fires + * @param data optional bits to print in the message formatting + * + * @note See [[printf.apply(fmt:String* printf]] for format string documentation + * @note currently cannot be used in core Chisel / libraries because macro + * defs need to be compiled first and the SBT project is not set up to do + * that + */ + // Macros currently can't take default arguments, so we need two functions to emulate defaults. + def apply( + cond: Bool, + message: String, + data: Bits* + )( + implicit sourceInfo: SourceInfo, + compileOptions: CompileOptions + ): Assert = macro _applyMacroWithStringMessage /** Checks for a condition to be valid in the circuit at all times. If the * condition evaluates to false, the circuit simulation stops with an error. * - * Does not fire when in reset (defined as the encapsulating Module's - * reset). If your definition of reset is not the encapsulating Module's - * reset, you will need to gate this externally. + * Does not fire when in reset (defined as the current implicit reset, e.g. as set by + * the enclosing `withReset` or Module.reset. * * May be called outside of a Module (like defined in a function), so * functions using assert make the standard Module assumptions (single clock * and single reset). * - * @param cond condition, assertion fires (simulation fails) when false - * @param message optional format string to print when the assertion fires - * @param data optional bits to print in the message formatting + * @param cond condition, assertion fires (simulation fails) on a rising clock edge when false and reset is not asserted + * @param message optional chisel Printable type message * - * @note See [[printf.apply(fmt:String* printf]] for format string documentation + * @note See [[printf.apply(fmt:Printable)]] for documentation on printf using Printables * @note currently cannot be used in core Chisel / libraries because macro * defs need to be compiled first and the SBT project is not set up to do * that */ - // Macros currently can't take default arguments, so we need two functions to emulate defaults. def apply( cond: Bool, - message: String, - data: Bits* + message: Printable )( implicit sourceInfo: SourceInfo, compileOptions: CompileOptions - ): Assert = macro _applyMacroWithMessage + ): Assert = macro _applyMacroWithPrintableMessage + def apply(cond: Bool)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Assert = macro _applyMacroWithNoMessage @@ -56,6 +90,11 @@ object assert { import VerificationStatement._ + /** @group VerifPrintMacros */ + @deprecated( + "This method has been deprecated in favor of _applyMacroWithStringMessage. Please use the same.", + "Chisel 3.5" + ) def _applyMacroWithMessage( c: blackbox.Context )(cond: c.Tree, @@ -65,10 +104,38 @@ object assert { compileOptions: c.Tree ): c.Tree = { import c.universe._ - val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLine")) - q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some($message), ..$data)($sourceInfo, $compileOptions)" + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable")) + q"$apply_impl_do($cond, ${getLine(c)},_root_.scala.Some(_root_.chisel3.Printable.pack($message,..$data)))($sourceInfo, $compileOptions)" } + /** @group VerifPrintMacros */ + def _applyMacroWithStringMessage( + c: blackbox.Context + )(cond: c.Tree, + message: c.Tree, + data: c.Tree* + )(sourceInfo: c.Tree, + compileOptions: c.Tree + ): c.Tree = { + import c.universe._ + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable")) + q"$apply_impl_do($cond, ${getLine(c)},_root_.scala.Some(_root_.chisel3.Printable.pack($message,..$data)))($sourceInfo, $compileOptions)" + } + + /** @group VerifPrintMacros */ + def _applyMacroWithPrintableMessage( + c: blackbox.Context + )(cond: c.Tree, + message: c.Tree + )(sourceInfo: c.Tree, + compileOptions: c.Tree + ): c.Tree = { + import c.universe._ + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable")) + q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some($message))($sourceInfo, $compileOptions)" + } + + /** @group VerifPrintMacros */ def _applyMacroWithNoMessage( c: blackbox.Context )(cond: c.Tree @@ -76,11 +143,14 @@ object assert { compileOptions: c.Tree ): c.Tree = { import c.universe._ - val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLine")) + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable")) q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.None)($sourceInfo, $compileOptions)" } - /** Used by our macros. Do not call directly! */ + /** This will be removed in Chisel 3.6 in favor of the Printable version + * + * @group VerifPrintMacros + */ def _applyWithSourceLine( cond: Bool, line: SourceLineInfo, @@ -92,14 +162,31 @@ object assert { ): Assert = { val id = new Assert() when(!Module.reset.asBool()) { - failureMessage("Assertion", line, cond, message, data) + failureMessage("Assertion", line, cond, message.map(Printable.pack(_, data: _*))) + Builder.pushCommand(Verification(id, Formal.Assert, sourceInfo, Module.clock.ref, cond.ref, "")) + } + id + } + + /** @group VerifPrintMacros */ + def _applyWithSourceLinePrintable( + cond: Bool, + line: SourceLineInfo, + message: Option[Printable] + )( + implicit sourceInfo: SourceInfo, + compileOptions: CompileOptions + ): Assert = { + val id = new Assert() + when(!Module.reset.asBool()) { + failureMessage("Assertion", line, cond, message) Builder.pushCommand(Verification(id, Formal.Assert, sourceInfo, Module.clock.ref, cond.ref, "")) } id } } -object assume { +object assume extends VerifPrintMacrosDoc { /** Assumes a condition to be valid in the circuit at all times. * Acts like an assertion in simulation and imposes a declarative @@ -127,7 +214,33 @@ object assume { )( implicit sourceInfo: SourceInfo, compileOptions: CompileOptions - ): Assume = macro _applyMacroWithMessage + ): Assume = macro _applyMacroWithStringMessage + + /** Assumes a condition to be valid in the circuit at all times. + * Acts like an assertion in simulation and imposes a declarative + * assumption on the state explored by formal tools. + * + * Does not fire when in reset (defined as the encapsulating Module's + * reset). If your definition of reset is not the encapsulating Module's + * reset, you will need to gate this externally. + * + * May be called outside of a Module (like defined in a function), so + * functions using assert make the standard Module assumptions (single clock + * and single reset). + * + * @param cond condition, assertion fires (simulation fails) when false + * @param message optional Printable type message when the assertion fires + * + * @note See [[printf.apply(fmt:Printable]] for documentation on printf using Printables + */ + def apply( + cond: Bool, + message: Printable + )( + implicit sourceInfo: SourceInfo, + compileOptions: CompileOptions + ): Assume = macro _applyMacroWithPrintableMessage + def apply(cond: Bool)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Assume = macro _applyMacroWithNoMessage @@ -142,6 +255,11 @@ object assume { import VerificationStatement._ + /** @group VerifPrintMacros */ + @deprecated( + "This method has been deprecated in favor of _applyMacroWithStringMessage. Please use the same.", + "Chisel 3.5" + ) def _applyMacroWithMessage( c: blackbox.Context )(cond: c.Tree, @@ -151,10 +269,38 @@ object assume { compileOptions: c.Tree ): c.Tree = { import c.universe._ - val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLine")) - q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some($message), ..$data)($sourceInfo, $compileOptions)" + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable")) + q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some(_root_.chisel3.Printable.pack($message, ..$data)))($sourceInfo, $compileOptions)" } + /** @group VerifPrintMacros */ + def _applyMacroWithStringMessage( + c: blackbox.Context + )(cond: c.Tree, + message: c.Tree, + data: c.Tree* + )(sourceInfo: c.Tree, + compileOptions: c.Tree + ): c.Tree = { + import c.universe._ + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable")) + q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some(_root_.chisel3.Printable.pack($message, ..$data)))($sourceInfo, $compileOptions)" + } + + /** @group VerifPrintMacros */ + def _applyMacroWithPrintableMessage( + c: blackbox.Context + )(cond: c.Tree, + message: c.Tree + )(sourceInfo: c.Tree, + compileOptions: c.Tree + ): c.Tree = { + import c.universe._ + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable")) + q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some($message))($sourceInfo, $compileOptions)" + } + + /** @group VerifPrintMacros */ def _applyMacroWithNoMessage( c: blackbox.Context )(cond: c.Tree @@ -162,11 +308,14 @@ object assume { compileOptions: c.Tree ): c.Tree = { import c.universe._ - val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLine")) + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable")) q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.None)($sourceInfo, $compileOptions)" } - /** Used by our macros. Do not call directly! */ + /** This will be removed in Chisel 3.6 in favor of the Printable version + * + * @group VerifPrintMacros + */ def _applyWithSourceLine( cond: Bool, line: SourceLineInfo, @@ -178,14 +327,31 @@ object assume { ): Assume = { val id = new Assume() when(!Module.reset.asBool()) { - failureMessage("Assumption", line, cond, message, data) + failureMessage("Assumption", line, cond, message.map(Printable.pack(_, data: _*))) + Builder.pushCommand(Verification(id, Formal.Assume, sourceInfo, Module.clock.ref, cond.ref, "")) + } + id + } + + /** @group VerifPrintMacros */ + def _applyWithSourceLinePrintable( + cond: Bool, + line: SourceLineInfo, + message: Option[Printable] + )( + implicit sourceInfo: SourceInfo, + compileOptions: CompileOptions + ): Assume = { + val id = new Assume() + when(!Module.reset.asBool()) { + failureMessage("Assumption", line, cond, message) Builder.pushCommand(Verification(id, Formal.Assume, sourceInfo, Module.clock.ref, cond.ref, "")) } id } } -object cover { +object cover extends VerifPrintMacrosDoc { /** Declares a condition to be covered. * At ever clock event, a counter is incremented iff the condition is active @@ -213,6 +379,7 @@ object cover { import VerificationStatement._ + /** @group VerifPrintMacros */ def _applyMacroWithNoMessage( c: blackbox.Context )(cond: c.Tree @@ -224,6 +391,7 @@ object cover { q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.None)($sourceInfo, $compileOptions)" } + /** @group VerifPrintMacros */ def _applyMacroWithMessage( c: blackbox.Context )(cond: c.Tree, @@ -236,7 +404,7 @@ object cover { q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some($message))($sourceInfo, $compileOptions)" } - /** Used by our macros. Do not call directly! */ + /** @group VerifPrintMacros */ def _applyWithSourceLine( cond: Bool, line: SourceLineInfo, @@ -299,13 +467,11 @@ private object VerificationStatement { (p.source.file.name, p.line, p.lineContent.trim) } - // creates a printf to inform the user of a failed assertion or assumption def failureMessage( kind: String, lineInfo: SourceLineInfo, cond: Bool, - message: Option[String], - data: Seq[Bits] + message: Option[Printable] )( implicit sourceInfo: SourceInfo, compileOptions: CompileOptions @@ -314,11 +480,11 @@ private object VerificationStatement { val lineMsg = s"$filename:$line $content".replaceAll("%", "%%") val fmt = message match { case Some(msg) => - s"$kind failed: $msg\n at $lineMsg\n" - case None => s"$kind failed\n at $lineMsg\n" + p"$kind failed: $msg\n at $lineMsg\n" + case None => p"$kind failed\n at $lineMsg\n" } when(!cond) { - printf.printfWithoutReset(fmt, data: _*) + printf.printfWithoutReset(fmt) } } } -- cgit v1.2.3 From 21415b11f1005c3cdfdc07fc02294849ce1c81a4 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 16 Aug 2022 00:35:35 +0000 Subject: Privatize trait VerifPrintMacros (#2683) (#2685) (cherry picked from commit de76e70bc5905fc4ebc8a2e323e16620fa6832ec) Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com>--- core/src/main/scala/chisel3/VerificationStatement.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/VerificationStatement.scala b/core/src/main/scala/chisel3/VerificationStatement.scala index 1b13b86c..3b6da3f6 100644 --- a/core/src/main/scala/chisel3/VerificationStatement.scala +++ b/core/src/main/scala/chisel3/VerificationStatement.scala @@ -23,7 +23,7 @@ import scala.reflect.macros.blackbox * * @groupprio VerifPrintMacros 1001 */ -trait VerifPrintMacrosDoc +private[chisel3] trait VerifPrintMacrosDoc object assert extends VerifPrintMacrosDoc { -- cgit v1.2.3 From 96830a4ad502019ff1040889a89375ff1e3a6c6b Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 16 Aug 2022 16:57:34 +0000 Subject: Add a cookbook and publicly visible scaladoc for prefix, noPrefix (#2687) (#2690) * Add a cookbook and publicly visible scaladoc for prefix, noPrefix (cherry picked from commit ae7dc30b3b99f1fbd91c35f54bc19be7c55f74a3) Co-authored-by: Megan Wachs --- .../main/scala/chisel3/experimental/package.scala | 28 ++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/experimental/package.scala b/core/src/main/scala/chisel3/experimental/package.scala index b1d9cae4..39131943 100644 --- a/core/src/main/scala/chisel3/experimental/package.scala +++ b/core/src/main/scala/chisel3/experimental/package.scala @@ -235,9 +235,33 @@ package object experimental { } } - // Use to add a prefix to any component generated in input scope + /** Use to add a prefix to any components generated in the provided scope. + * + * @example {{{ + * + * val x1 = prefix("first") { + * // Anything generated here will be prefixed with "first" + * } + * + * val x2 = prefix(mysignal) { + * // Anything generated here will be prefixed with the name of mysignal + * } + * + * }}} + */ val prefix = chisel3.internal.prefix - // Use to remove prefixes not in provided scope + + /** Use to clear existing prefixes so no signals within the scope are prefixed + * by signals/names outside the scope + * + * @example {{{ + * + * val x1 = prefix("first") { + * // Anything generated here will have no prefix. + * // The result returned from this would *still* be called `x1` however. + * } + * }}} + */ val noPrefix = chisel3.internal.noPrefix // ****************************** Hardware equivalents of Scala Tuples ****************************** -- cgit v1.2.3 From 23ef9aa7ffef5bbf8fe124fc9be7683f005c3612 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 16 Aug 2022 19:04:28 +0000 Subject: Add OpaqueType support to Records (backport #2662) (#2679) * Add OpaqueType support to Records (#2662) OpaqueTypes are essentially type aliases that hide the underlying type. They are implemented in Chisel as Records of a single, unnamed element where the wrapping Record does not exist in the emitted FIRRTL. Co-authored-by: Jack Koenig (cherry picked from commit df5afee2d41b5bcd82d4013b977965c0dfe046fd) * Fix test compilation * Fix OpaqueType tests in RecordSpec Need to implement cloneType correctly and to warn instead of error when accessing .toTarget of non-hardware types because that is a warning (not error) in 3.5. * Waive MiMa false positives Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com> Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Aggregate.scala | 22 +++++++++++++++++++++- core/src/main/scala/chisel3/internal/Builder.scala | 11 ++++++++--- .../scala/chisel3/internal/firrtl/Converter.scala | 10 ++++++++-- .../main/scala/chisel3/internal/firrtl/IR.scala | 6 ++++++ 4 files changed, 43 insertions(+), 6 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 82c02cbc..042e78b1 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -923,6 +923,20 @@ trait VecLike[T <: Data] extends IndexedSeq[T] with HasId with SourceInfoDoc { */ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptions) extends Aggregate { + /** Indicates if this Record represents an "Opaque Type" + * + * Opaque types provide a mechanism for user-defined types + * that do not impose any "boxing" overhead in the emitted FIRRTL and Verilog. + * You can think about an opaque type Record as a box around + * a single element that only exists at Chisel elaboration time. + * Put another way, if opaqueType is overridden to true, + * The Record may only contain a single element with an empty name + * and there will be no `_` in the name for that element in the emitted Verilog. + * + * @see RecordSpec in Chisel's tests for example usage and expected output + */ + def opaqueType: Boolean = false + // Doing this earlier than onModuleClose allows field names to be available for prefixing the names // of hardware created when connecting to one of these elements private def setElementRefs(): Unit = { @@ -930,7 +944,13 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio // identifier; however, Namespace sanitizes identifiers to make them legal for Firrtl/Verilog // which can cause collisions val _namespace = Namespace.empty - for ((name, elt) <- elements) { elt.setRef(this, _namespace.name(name, leadingDigitOk = true)) } + 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(", ")}" + ) + for ((name, elt) <- elements) { + elt.setRef(this, _namespace.name(name, leadingDigitOk = true), opaque = opaqueType) + } } private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = { diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 75d08f92..07d8e7f7 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -231,9 +231,13 @@ private[chisel3] trait HasId extends InstanceId { _refVar = imm } } - private[chisel3] def setRef(parent: HasId, name: String): Unit = setRef(Slot(Node(parent), name)) - private[chisel3] def setRef(parent: HasId, index: Int): Unit = setRef(Index(Node(parent), ILit(index))) - private[chisel3] def setRef(parent: HasId, index: UInt): Unit = setRef(Index(Node(parent), index.ref)) + private[chisel3] def setRef(parent: HasId, name: String, opaque: Boolean = false): Unit = { + if (!opaque) setRef(Slot(Node(parent), name)) + else setRef(OpaqueSlot(Node(parent), name)) + } + + private[chisel3] def setRef(parent: HasId, index: Int): Unit = setRef(Index(Node(parent), ILit(index))) + private[chisel3] def setRef(parent: HasId, index: UInt): Unit = setRef(Index(Node(parent), index.ref)) private[chisel3] def getRef: Arg = _ref.get private[chisel3] def getOptionRef: Option[Arg] = _ref @@ -514,6 +518,7 @@ private[chisel3] object Builder extends LazyLogging { def buildAggName(id: HasId): Option[String] = { def getSubName(field: Data): Option[String] = field.getOptionRef.flatMap { case Slot(_, field) => Some(field) // Record + case OpaqueSlot(_, field) => None // Record with single element case Index(_, ILit(n)) => Some(n.toString) // Vec static indexing case Index(_, ULit(n, _)) => Some(n.toString) // Vec lit indexing case Index(_, _: Node) => None // Vec dynamic indexing diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index 2d21a7cb..56422b85 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -68,6 +68,8 @@ private[chisel3] object Converter { fir.Reference(name, fir.UnknownType) case Slot(imm, name) => fir.SubField(convert(imm, ctx, info), name, fir.UnknownType) + case OpaqueSlot(imm, name) => + convert(imm, ctx, info) case Index(imm, ILit(idx)) => fir.SubIndex(convert(imm, ctx, info), castToInt(idx, "Index"), fir.UnknownType) case Index(imm, value) => @@ -301,7 +303,7 @@ private[chisel3] object Converter { case d: Interval => fir.IntervalType(d.range.lowerBound, d.range.upperBound, d.range.firrtlBinaryPoint) case d: Analog => fir.AnalogType(convert(d.width)) case d: Vec[_] => fir.VectorType(extractType(d.sample_element, clearDir, info), d.length) - case d: Record => + case d: Record => { val childClearDir = clearDir || d.specifiedDirection == SpecifiedDirection.Input || d.specifiedDirection == SpecifiedDirection.Output def eltField(elt: Data): fir.Field = (childClearDir, firrtlUserDirOf(elt)) match { @@ -311,7 +313,11 @@ private[chisel3] object Converter { case (false, SpecifiedDirection.Flip | SpecifiedDirection.Input) => fir.Field(getRef(elt, info).name, fir.Flip, extractType(elt, false, info)) } - fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) }) + if (!d.opaqueType) + fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) }) + else + extractType(d.elements.head._2, true, info) + } } def convert(name: String, param: Param): fir.Param = param match { diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala index dc9ab027..37fb2f8b 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala @@ -91,6 +91,7 @@ object Arg { case Some(Index(Node(imm), Node(value))) => s"${earlyLocalName(imm)}[${earlyLocalName(imm)}]" case Some(Index(Node(imm), arg)) => s"${earlyLocalName(imm)}[${arg.localName}]" case Some(Slot(Node(imm), name)) => s"${earlyLocalName(imm)}.$name" + case Some(OpaqueSlot(Node(imm), name)) => s"${earlyLocalName(imm)}" case Some(arg) => arg.name case None => id match { @@ -218,6 +219,11 @@ case class Slot(imm: Node, name: String) extends Arg { } } +case class OpaqueSlot(imm: Node, name: String) extends Arg { + override def contextualName(ctx: Component): String = imm.name + override def localName: String = imm.name +} + case class Index(imm: Arg, value: Arg) extends Arg { def name: String = s"[$value]" override def contextualName(ctx: Component): String = s"${imm.contextualName(ctx)}[${value.contextualName(ctx)}]" -- cgit v1.2.3 From 16dfc84d6667f1f6bbca46935cb445bc288c96d4 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 18 Aug 2022 00:30:39 +0000 Subject: Add generic `Data` equality (===) via extension method (#2669) (#2691) (cherry picked from commit 67cff8253740f19642006dba7eff58b1e5fa1291) Co-authored-by: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com>--- core/src/main/scala/chisel3/Data.scala | 80 +++++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 956c7996..d434735a 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -5,7 +5,7 @@ package chisel3 import chisel3.experimental.dataview.reify import scala.language.experimental.macros -import chisel3.experimental.{Analog, BaseModule, DataMirror, FixedPoint, Interval} +import chisel3.experimental.{Analog, BaseModule, DataMirror, EnumType, FixedPoint, Interval} import chisel3.internal.Builder.pushCommand import chisel3.internal._ import chisel3.internal.firrtl._ @@ -885,6 +885,84 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { def toPrintable: Printable } +object Data { + + /** + * Provides generic, recursive equality for [[Bundle]] and [[Vec]] hardware. This avoids the + * need to use workarounds such as `bundle1.asUInt === bundle2.asUInt` by allowing users + * to instead write `bundle1 === bundle2`. + * + * Static type safety of this comparison is guaranteed at compile time as the extension + * method requires the same parameterized type for both the left-hand and right-hand + * sides. It is, however, possible to get around this type safety using `Bundle` subtypes + * that can differ during runtime (e.g. through a generator). These cases are + * subsequently raised as elaboration errors. + * + * @param lhs The [[Data]] hardware on the left-hand side of the equality + */ + implicit class DataEquality[T <: Data](lhs: T)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions) { + + /** Dynamic recursive equality operator for generic [[Data]] + * + * @param rhs a hardware [[Data]] to compare `lhs` to + * @return a hardware [[Bool]] asserted if `lhs` is equal to `rhs` + * @throws ChiselException when `lhs` and `rhs` are different types during elaboration time + */ + def ===(rhs: T): Bool = { + (lhs, rhs) match { + case (thiz: UInt, that: UInt) => thiz === that + case (thiz: SInt, that: SInt) => thiz === that + case (thiz: AsyncReset, that: AsyncReset) => thiz.asBool === that.asBool + case (thiz: Reset, that: Reset) => thiz === that + case (thiz: Interval, that: Interval) => thiz === that + case (thiz: FixedPoint, that: FixedPoint) => thiz === that + case (thiz: EnumType, that: EnumType) => thiz === that + case (thiz: Clock, that: Clock) => thiz.asUInt === that.asUInt + case (thiz: Vec[_], that: Vec[_]) => + if (thiz.length != that.length) { + throwException(s"Cannot compare Vecs $thiz and $that: Vec sizes differ") + } else { + thiz.getElements + .zip(that.getElements) + .map { case (thisData, thatData) => thisData === thatData } + .reduce(_ && _) + } + case (thiz: Record, that: Record) => + if (thiz.elements.size != that.elements.size) { + throwException(s"Cannot compare Bundles $thiz and $that: Bundle types differ") + } else { + thiz.elements.map { + case (thisName, thisData) => + 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) + + try { + thisData === thatData + } catch { + case e: ChiselException => + throwException( + s"Cannot compare field $thisName in Bundles $thiz and $that: ${e.getMessage.split(": ").last}" + ) + } + } + .reduce(_ && _) + } + // This should be matching to (DontCare, DontCare) but the compiler wasn't happy with that + case (_: DontCare.type, _: DontCare.type) => true.B + + case (thiz: Analog, that: Analog) => + throwException(s"Cannot compare Analog values $thiz and $that: Equality isn't defined for Analog values") + // Runtime types are different + case (thiz, that) => throwException(s"Cannot compare $thiz and $that: Runtime types differ") + } + } + } +} + trait WireFactory { /** Construct a [[Wire]] from a type template -- cgit v1.2.3 From c3b06d773f1fabd2519d6c705d68381d13f1c07f Mon Sep 17 00:00:00 2001 From: Zachary Yedidia Date: Tue, 23 Aug 2022 17:20:12 -0700 Subject: Backport .toTarget deprecation warning information (3.5.x) (#2697) --- core/src/main/scala/chisel3/internal/Builder.scala | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 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 07d8e7f7..07fc80eb 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -249,10 +249,20 @@ 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. " + - "This will become an error in Chisel 3.6." + val msg = + "Accessing the .instanceName or .toTarget of non-hardware Data is deprecated" + nameGuess + parentGuess + ". " + + "This will become an error in Chisel 3.6." errors.deprecated(msg, None) errors.checkpoint(logger) _computeName(None).get -- cgit v1.2.3 From 998913f9379440db26b6aeeaa09e7a11d7615351 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 25 Aug 2022 18:14:54 +0000 Subject: Bugfix - OpaqueSlot replace invalid localName (backport #2701) (#2702) * Bugfix - OpaqueSlot replace invalid localName (#2701) (cherry picked from commit fb8ea2a2fac227f2570da992d7877de2eb1cf801) * Fix cloneTypes (#2703) Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com>--- core/src/main/scala/chisel3/internal/Builder.scala | 4 ++-- core/src/main/scala/chisel3/internal/firrtl/Converter.scala | 2 +- core/src/main/scala/chisel3/internal/firrtl/IR.scala | 6 +++--- 3 files changed, 6 insertions(+), 6 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 07fc80eb..9f79fe1e 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -233,7 +233,7 @@ private[chisel3] trait HasId extends InstanceId { } private[chisel3] def setRef(parent: HasId, name: String, opaque: Boolean = false): Unit = { if (!opaque) setRef(Slot(Node(parent), name)) - else setRef(OpaqueSlot(Node(parent), name)) + else setRef(OpaqueSlot(Node(parent))) } private[chisel3] def setRef(parent: HasId, index: Int): Unit = setRef(Index(Node(parent), ILit(index))) @@ -528,7 +528,7 @@ private[chisel3] object Builder extends LazyLogging { def buildAggName(id: HasId): Option[String] = { def getSubName(field: Data): Option[String] = field.getOptionRef.flatMap { case Slot(_, field) => Some(field) // Record - case OpaqueSlot(_, field) => None // Record with single element + case OpaqueSlot(_) => None // OpaqueSlots don't contribute to the name case Index(_, ILit(n)) => Some(n.toString) // Vec static indexing case Index(_, ULit(n, _)) => Some(n.toString) // Vec lit indexing case Index(_, _: Node) => None // Vec dynamic indexing diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index 56422b85..fe95445c 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -68,7 +68,7 @@ private[chisel3] object Converter { fir.Reference(name, fir.UnknownType) case Slot(imm, name) => fir.SubField(convert(imm, ctx, info), name, fir.UnknownType) - case OpaqueSlot(imm, name) => + case OpaqueSlot(imm) => convert(imm, ctx, info) case Index(imm, ILit(idx)) => fir.SubIndex(convert(imm, ctx, info), castToInt(idx, "Index"), fir.UnknownType) diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala index 37fb2f8b..d177c859 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala @@ -91,7 +91,7 @@ object Arg { case Some(Index(Node(imm), Node(value))) => s"${earlyLocalName(imm)}[${earlyLocalName(imm)}]" case Some(Index(Node(imm), arg)) => s"${earlyLocalName(imm)}[${arg.localName}]" case Some(Slot(Node(imm), name)) => s"${earlyLocalName(imm)}.$name" - case Some(OpaqueSlot(Node(imm), name)) => s"${earlyLocalName(imm)}" + case Some(OpaqueSlot(Node(imm))) => s"${earlyLocalName(imm)}" case Some(arg) => arg.name case None => id match { @@ -219,9 +219,9 @@ case class Slot(imm: Node, name: String) extends Arg { } } -case class OpaqueSlot(imm: Node, name: String) extends Arg { +case class OpaqueSlot(imm: Node) extends Arg { override def contextualName(ctx: Component): String = imm.name - override def localName: String = imm.name + override def name: String = imm.name } case class Index(imm: Arg, value: Arg) extends Arg { -- cgit v1.2.3 From 9f1484572e2e4185e87a9cfb03b253870636c12c Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 29 Aug 2022 21:05:56 +0000 Subject: Fix OpaqueSlot handling of contextual names (#2708) (#2712) We need to ensure that contextual names stay contextual (ie. sensitive to the module context which is important for naming ports). (cherry picked from commit cee255216c4a1bb658a2d8ddc03d966ce7ffb877) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/firrtl/IR.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala index d177c859..ddad6b10 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala @@ -220,7 +220,7 @@ case class Slot(imm: Node, name: String) extends Arg { } case class OpaqueSlot(imm: Node) extends Arg { - override def contextualName(ctx: Component): String = imm.name + override def contextualName(ctx: Component): String = imm.contextualName(ctx) override def name: String = imm.name } -- cgit v1.2.3 From bb3ef96d8911dbba4e22926ad4ce71eb8ab0d869 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 31 Aug 2022 01:41:09 +0000 Subject: Wires should have source location information in firrtl (#2714) (#2716) - Remove line defeating having wire locators `implicit val noSourceInfo = UnlocatableSourceInfo` from `WireDefault#apply` - Add test to show locators (cherry picked from commit f701a9f8151891e3bf9019cd3229cb3f2cd1833b) Co-authored-by: Chick Markley --- core/src/main/scala/chisel3/Data.scala | 3 +-- 1 file changed, 1 insertion(+), 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 d434735a..7c8ec1a9 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -9,7 +9,7 @@ import chisel3.experimental.{Analog, BaseModule, DataMirror, EnumType, FixedPoin import chisel3.internal.Builder.pushCommand import chisel3.internal._ import chisel3.internal.firrtl._ -import chisel3.internal.sourceinfo.{DeprecatedSourceInfo, SourceInfo, SourceInfoTransform, UnlocatableSourceInfo} +import chisel3.internal.sourceinfo.{SourceInfo, SourceInfoTransform, UnlocatableSourceInfo} import scala.collection.immutable.LazyList // Needed for 2.12 alias import scala.reflect.ClassTag @@ -1075,7 +1075,6 @@ object WireDefault { implicit sourceInfo: SourceInfo, compileOptions: CompileOptions ): T = { - implicit val noSourceInfo = UnlocatableSourceInfo val x = Wire(t) requireIsHardware(init, "wire initializer") x := init -- cgit v1.2.3 From 7ff5fd4aa7dc01099614da9cc3b72d53f61d1cdb Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 1 Sep 2022 01:50:58 +0000 Subject: Revert "Privatize trait VerifPrintMacros (#2683)" (#2718) (#2719) This reverts commit de76e70bc5905fc4ebc8a2e323e16620fa6832ec. (cherry picked from commit e1e0503d969c8f4bb68a3beedebca5d9238192fd) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/VerificationStatement.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/VerificationStatement.scala b/core/src/main/scala/chisel3/VerificationStatement.scala index 3b6da3f6..1b13b86c 100644 --- a/core/src/main/scala/chisel3/VerificationStatement.scala +++ b/core/src/main/scala/chisel3/VerificationStatement.scala @@ -23,7 +23,7 @@ import scala.reflect.macros.blackbox * * @groupprio VerifPrintMacros 1001 */ -private[chisel3] trait VerifPrintMacrosDoc +trait VerifPrintMacrosDoc object assert extends VerifPrintMacrosDoc { -- cgit v1.2.3 From 341dd51d76b8b068c59b184ceb952624d42abbfa Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 1 Sep 2022 23:55:26 +0000 Subject: Remove incorrect clock warning on Mem.read (backport #2721) (#2722) * Remove incorrect clock warning on Mem.read (#2721) Mem.read is combinational and thus unaffected by the clock, and so it does not make sense to issue warnings about the current clock in this context. (cherry picked from commit 5fdf74f95e64cb69d6097547f20d789a83dbd735) * Keep old version of MemBase.clockWarning for binary compatibility This method is impossible for users to call, but it is easy enough to keep around a version of it to make MiMa happy. Co-authored-by: Andrew Waterman Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Mem.scala | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Mem.scala b/core/src/main/scala/chisel3/Mem.scala index d6ab9c4b..91872979 100644 --- a/core/src/main/scala/chisel3/Mem.scala +++ b/core/src/main/scala/chisel3/Mem.scala @@ -60,7 +60,10 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) // 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 = { + // Only kept for binary compatibility reasons, impossible for users to call + protected def clockWarning(sourceInfo: Option[SourceInfo]): Unit = clockWarning(sourceInfo, MemPortDirection.INFER) + + protected def clockWarning(sourceInfo: Option[SourceInfo], dir: MemPortDirection): Unit = { // Turn into pretty String if possible, if not, Builder.deprecated will find one via stack trace val infoStr = sourceInfo.collect { case SourceLine(file, line, col) => s"$file:$line:$col" } Builder.deprecated( @@ -135,7 +138,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) compileOptions: CompileOptions ): T = { if (warn && clockInst.isDefined && clock != clockInst.get) { - clockWarning(Some(sourceInfo)) + clockWarning(Some(sourceInfo), dir) } makePort(sourceInfo, idx, dir, clock) } @@ -167,7 +170,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) implicit compileOptions: CompileOptions ): Unit = { if (warn && clockInst.isDefined && clock != clockInst.get) { - clockWarning(None) + clockWarning(None, MemPortDirection.WRITE) } implicit val sourceInfo = UnlocatableSourceInfo makePort(UnlocatableSourceInfo, idx, MemPortDirection.WRITE, clock) := data @@ -226,7 +229,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) ): Unit = { implicit val sourceInfo = UnlocatableSourceInfo if (warn && clockInst.isDefined && clock != clockInst.get) { - clockWarning(None) + clockWarning(None, MemPortDirection.WRITE) } val accessor = makePort(sourceInfo, idx, MemPortDirection.WRITE, clock).asInstanceOf[Vec[Data]] val dataVec = data.asInstanceOf[Vec[Data]] @@ -274,7 +277,13 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) * @note when multiple conflicting writes are performed on a Mem element, the * result is undefined (unlike Vec, where the last assignment wins) */ -sealed class Mem[T <: Data] private[chisel3] (t: T, length: BigInt) extends MemBase(t, length) +sealed class Mem[T <: Data] private[chisel3] (t: T, length: BigInt) extends MemBase(t, length) { + override protected def clockWarning(sourceInfo: Option[SourceInfo], dir: MemPortDirection): Unit = { + // Do not issue clock warnings on reads, since they are not clocked + if (dir != MemPortDirection.READ) + super.clockWarning(sourceInfo, dir) + } +} object SyncReadMem { -- cgit v1.2.3 From 03f62c8c9bc2f6cc65ab34b8902f5e9a61701595 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 15 Sep 2022 20:46:10 +0000 Subject: Change description for SInt unary negation (#2729) (#2734) Referenced to: chipsalliance/chisel3#2728 (cherry picked from commit a4dae9c340c71c063cf0fdec290a6e011b82746d) Co-authored-by: Marco Origlia <30799310+moriglia@users.noreply.github.com>--- core/src/main/scala/chisel3/Bits.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Bits.scala b/core/src/main/scala/chisel3/Bits.scala index 72094a65..70704e01 100644 --- a/core/src/main/scala/chisel3/Bits.scala +++ b/core/src/main/scala/chisel3/Bits.scala @@ -896,7 +896,7 @@ sealed class SInt private[chisel3] (width: Width) extends Bits(width) with Num[S private[chisel3] override def cloneTypeWidth(w: Width): this.type = new SInt(w).asInstanceOf[this.type] - /** Unary negation (expanding width) + /** Unary negation (constant width) * * @return a hardware $coll equal to zero minus this $coll * $constantWidth -- cgit v1.2.3 From 5a79814631bdc8c71c5a7b4722cd43712f7ff445 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 29 Sep 2022 18:53:44 +0000 Subject: Add lexical scope checks to Assert, Assume and Printf (#2706) (#2753) (cherry picked from commit f462c9f9307bebf3012da52432c3729cd752321c) Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com>--- core/src/main/scala/chisel3/Data.scala | 2 +- core/src/main/scala/chisel3/Printable.scala | 13 +++++++++++++ core/src/main/scala/chisel3/Printf.scala | 3 +++ core/src/main/scala/chisel3/VerificationStatement.scala | 2 ++ 4 files changed, 19 insertions(+), 1 deletion(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 7c8ec1a9..f52f99de 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -662,7 +662,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { */ private[chisel3] def typeEquivalent(that: Data): Boolean - private def requireVisible(): Unit = { + private[chisel3] def requireVisible(): Unit = { val mod = topBindingOpt.flatMap(_.location) topBindingOpt match { case Some(tb: TopBinding) if (mod == Builder.currentModule) => diff --git a/core/src/main/scala/chisel3/Printable.scala b/core/src/main/scala/chisel3/Printable.scala index 78655517..82054ee1 100644 --- a/core/src/main/scala/chisel3/Printable.scala +++ b/core/src/main/scala/chisel3/Printable.scala @@ -134,6 +134,19 @@ object Printable { val bufEscapeBackSlash = buf.map(_.replace("\\", "\\\\")) StringContext(bufEscapeBackSlash.toSeq: _*).cf(data: _*) } + + private[chisel3] def checkScope(message: Printable): Unit = { + def getData(x: Printable): Seq[Data] = { + x match { + case y: FirrtlFormat => Seq(y.bits) + case Name(d) => Seq(d) + case FullName(d) => Seq(d) + case Printables(p) => p.flatMap(getData(_)).toSeq + case _ => Seq() // Handles subtypes PString and Percent + } + } + getData(message).foreach(_.requireVisible()) + } } case class Printables(pables: Iterable[Printable]) extends Printable { diff --git a/core/src/main/scala/chisel3/Printf.scala b/core/src/main/scala/chisel3/Printf.scala index bdcca8e1..9410a409 100644 --- a/core/src/main/scala/chisel3/Printf.scala +++ b/core/src/main/scala/chisel3/Printf.scala @@ -108,6 +108,9 @@ object printf { ): Printf = { val clock = Builder.forcedClock val printfId = new Printf(pable) + + Printable.checkScope(pable) + pushCommand(chisel3.internal.firrtl.Printf(printfId, sourceInfo, clock.ref, pable)) printfId } diff --git a/core/src/main/scala/chisel3/VerificationStatement.scala b/core/src/main/scala/chisel3/VerificationStatement.scala index 1b13b86c..10cece60 100644 --- a/core/src/main/scala/chisel3/VerificationStatement.scala +++ b/core/src/main/scala/chisel3/VerificationStatement.scala @@ -178,6 +178,7 @@ object assert extends VerifPrintMacrosDoc { compileOptions: CompileOptions ): Assert = { val id = new Assert() + message.foreach(Printable.checkScope(_)) when(!Module.reset.asBool()) { failureMessage("Assertion", line, cond, message) Builder.pushCommand(Verification(id, Formal.Assert, sourceInfo, Module.clock.ref, cond.ref, "")) @@ -343,6 +344,7 @@ object assume extends VerifPrintMacrosDoc { compileOptions: CompileOptions ): Assume = { val id = new Assume() + message.foreach(Printable.checkScope(_)) when(!Module.reset.asBool()) { failureMessage("Assumption", line, cond, message) Builder.pushCommand(Verification(id, Formal.Assume, sourceInfo, Module.clock.ref, cond.ref, "")) -- cgit v1.2.3 From b72cc42f4f23906db0f201b1d9543a64accbc2ec Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 6 Oct 2022 21:26:30 +0000 Subject: Update toPrintable for Enums (#2707) (#2763) (cherry picked from commit 0ff99ca8d573e3487ef496a21c95d962689c3cba) Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com>--- core/src/main/scala/chisel3/StrongEnum.scala | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/StrongEnum.scala b/core/src/main/scala/chisel3/StrongEnum.scala index 6d8ceb2f..3c9f4105 100644 --- a/core/src/main/scala/chisel3/StrongEnum.scala +++ b/core/src/main/scala/chisel3/StrongEnum.scala @@ -251,7 +251,28 @@ abstract class EnumType(private[chisel3] val factory: EnumFactory, selfAnnotatin protected def enumTypeName: String = factory.enumTypeName - def toPrintable: Printable = FullName(this) // TODO: Find a better pretty printer + def toPrintable: Printable = { + implicit val sourceInfo = UnlocatableSourceInfo + implicit val compileOptions = ExplicitCompileOptions.Strict + val allNames = factory.allNames.zip(factory.all) + val nameSize = allNames.map(_._1.length).max + def leftPad(str: String): String = { + str.reverse.padTo(nameSize, ' ').reverse + } + val allNamesPadded = allNames.map { case (name, value) => leftPad(name) -> value } + + val result = Wire(Vec(nameSize, UInt(8.W))).suggestName(s"_${enumTypeName}Printable") + result.foreach(_ := '?'.U) + + for ((name, value) <- allNamesPadded) { + when(this === value) { + for ((r, c) <- result.zip(name)) { + r := c.toChar.U + } + } + } + result.map(Character(_)).foldLeft(p"")(_ + _) + } } abstract class EnumFactory { @@ -284,6 +305,8 @@ abstract class EnumFactory { def getWidth: Int = width.get def all: Seq[Type] = enumInstances + /* Accessor for Seq of names in enumRecords */ + def allNames: Seq[String] = enumNames private[chisel3] def nameOfValue(id: BigInt): Option[String] = { enumRecords.find(_.inst.litValue == id).map(_.name) -- cgit v1.2.3 From 5b13d04b28ddd05e4acbc5b9b3755c92ac0d9515 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 7 Oct 2022 19:56:19 +0000 Subject: Make nested IsInstantiables with Data in them work (#2761) (#2766) * Add unit test for Issue 2760 * checkpoint: Fix for nested instance * remove comments about stuff not working * make the test check the output a little more * relax the requirement on returning empty ioMap * Update core/src/main/scala/chisel3/experimental/hierarchy/core/Lookupable.scala * Update core/src/main/scala/chisel3/Data.scala * Update core/src/main/scala/chisel3/experimental/hierarchy/core/Lookupable.scala Co-authored-by: Jack Koenig * Update src/test/scala/chiselTests/experimental/hierarchy/InstanceSpec.scala Co-authored-by: Jack Koenig * Update core/src/main/scala/chisel3/experimental/hierarchy/core/Lookupable.scala * Add another unit test which unfortunately still passes * Update core/src/main/scala/chisel3/Data.scala * Update src/test/scala/chiselTests/experimental/hierarchy/InstanceSpec.scala Co-authored-by: Jack Koenig Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 1f9f26dc2bffcb4cc4daf2dc16c5cb455c6769ef) Co-authored-by: Megan Wachs --- .../chisel3/experimental/hierarchy/Lookupable.scala | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala b/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala index c83479b0..aa35455d 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala @@ -335,11 +335,20 @@ object Lookupable { } def instanceLookup[A](that: A => B, instance: Instance[A]): C = { val ret = that(instance.proto) - val ioMap: Option[Map[Data, Data]] = instance.underlying match { - case Clone(x: ModuleClone[_]) => Some(x.ioMap) - case Proto(x: BaseModule) => Some(x.getChiselPorts.map { case (_, data) => data -> data }.toMap) - case _ => None + + def getIoMap(hierarchy: Hierarchy[_]): Option[Map[Data, Data]] = { + hierarchy.underlying match { + case Clone(x: ModuleClone[_]) => Some(x.ioMap) + case Proto(x: BaseModule) => Some(x.getChiselPorts.map { case (_, data) => data -> data }.toMap) + case Clone(x: InstantiableClone[_]) => getIoMap(x._innerContext) + case Clone(x: InstanceClone[_]) => None + case other => { + Builder.exception(s"Internal Error! Unexpected case where we can't get IO Map: $other") + } + } } + val ioMap = getIoMap(instance) + if (isView(ret)) { cloneViewToContext(ret, instance.cache, ioMap, instance.getInnerDataContext) } else { -- cgit v1.2.3 From 721adc5c5509af48118afae44afa6b8a0107a926 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 10 Oct 2022 20:21:22 +0000 Subject: Fix traceName module type to RawModule (backport #2765) (#2768) * Fix traceName module type to RawModule (#2765) Change the type of modules that the traceName API can be used for from "Module" to "RawModule". This fixes a bug where this API couldn't be used for RawModules even though it totally works. Signed-off-by: Schuyler Eldridge Signed-off-by: Schuyler Eldridge (cherry picked from commit 74f1c85060cc72ebffe59a49f8d4539a464a4a19) * Fix binary compatibility issue Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/experimental/Trace.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/experimental/Trace.scala b/core/src/main/scala/chisel3/experimental/Trace.scala index 4ab615a5..3cc27162 100644 --- a/core/src/main/scala/chisel3/experimental/Trace.scala +++ b/core/src/main/scala/chisel3/experimental/Trace.scala @@ -1,7 +1,7 @@ package chisel3.experimental import chisel3.internal.HasId -import chisel3.{Aggregate, Data, Element, Module} +import chisel3.{Aggregate, Data, Element, Module, RawModule} import firrtl.AnnotationSeq import firrtl.annotations.{Annotation, CompleteTarget, SingleTargetAnnotation} import firrtl.transforms.DontTouchAllTargets @@ -22,7 +22,10 @@ import firrtl.transforms.DontTouchAllTargets object Trace { /** Trace a Instance name. */ - def traceName(x: Module): Unit = { + def traceName(x: Module): Unit = traceName(x: RawModule) + + /** Trace a Instance name. */ + def traceName(x: RawModule): Unit = { annotate(new ChiselAnnotation { def toFirrtl: Annotation = TraceNameAnnotation(x.toAbsoluteTarget, x.toAbsoluteTarget) }) -- cgit v1.2.3 From 1957f5ef5c43439144cf779a343707872ca92d6a Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 17 Oct 2022 22:50:55 +0000 Subject: Add opt-in AutoCloneType for Records (backport #2781) (#2785) * Add opt-in AutoCloneType for Records (#2781) There is a new trait, chisel3.experimental.AutoCloneType that is mixed in to Bundle and can optionally be mixed in to user-defined Records. The compiler plugin prints a deprecation warning on any user-defined implementation of cloneType, telling the user to mix in AutoCloneType before upgrading to 3.6. (cherry picked from commit a234fd48ac8f5942c38fef5797292014e407b586) # Conflicts: # core/src/main/scala/chisel3/Aggregate.scala # plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala * Resolve backport conflicts * Do not make MixedVec extend AutoCloneType It is a binary incompatible change that can wait for 3.6. * Waive MiMa false positives Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Aggregate.scala | 32 ++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 10 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 042e78b1..aacf0b1c 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -1158,6 +1158,16 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio * Results in "`\$className(elt0.name -> elt0.value, ...)`" */ def toPrintable: Printable = toPrintableHelper(elements.toList) + + /** Implementation of cloneType that is [optionally for Record] overridden by the compiler plugin + * + * @note This should _never_ be overridden or called in user-code + */ + protected def _cloneTypeImpl: Record = { + throwException( + s"Internal Error! This should have been implemented by the chisel3-plugin. Please file an issue against chisel3" + ) + } } /** @@ -1182,6 +1192,15 @@ package experimental { class BundleLiteralException(message: String) extends ChiselException(message) class VecLiteralException(message: String) extends ChiselException(message) + /** Indicates that the compiler plugin should generate [[cloneType]] for this type + * + * All user-defined [[Record]]s should mix this trait in as it will be required for upgrading to Chisel 3.6. + */ + trait AutoCloneType { self: Record => + + override def cloneType: this.type = _cloneTypeImpl.asInstanceOf[this.type] + + } } /** Base class for data types defined as a bundle of other data types. @@ -1217,7 +1236,7 @@ package experimental { * } * }}} */ -abstract class Bundle(implicit compileOptions: CompileOptions) extends Record { +abstract class Bundle(implicit compileOptions: CompileOptions) extends Record with experimental.AutoCloneType { assert( _usingPlugin, "The Chisel compiler plugin is now required for compiling Chisel code. " + @@ -1385,15 +1404,8 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record { clone } - /** Implementation of cloneType using runtime reflection. This should _never_ be overridden or called in user-code - * - * @note This is overridden by the compiler plugin (this implementation is never called) - */ - protected def _cloneTypeImpl: Bundle = { - throwException( - s"Internal Error! This should have been implemented by the chisel3-plugin. Please file an issue against chisel3" - ) - } + // This is overriden for binary compatibility reasons in 3.5 + override protected def _cloneTypeImpl: Bundle = super._cloneTypeImpl.asInstanceOf[Bundle] /** Default "pretty-print" implementation * Analogous to printing a Map -- cgit v1.2.3 From 9b8536b6af9f029a0edfb1c8df4f47a67e861c9d Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 18 Oct 2022 18:48:54 +0000 Subject: Add traceNameV2 for backwards compat. of traceName (#2784) (#2786) Add utilities to enable backwards compatibility of the Trace.traceName API to Chisel 3.5.x. This adds a Trace.traceNameV2 utility which aliases to Trace.traceName. This also removes the TraceNameAnnotation and renames it TraceAnnotation. In 3.5.x, traceName will point at TraceNameAnnotation (which has don't touch behavior) and will be deprecated telling people to use traceNameV2 which will point at TraceAnnotation (which does not have don't touch behavior). This will require fixups to the backport associated with this PR. Signed-off-by: Schuyler Eldridge (cherry picked from commit 47b7227e1ac7ccb0d48cefef03510542cc7e157e) # Conflicts: # core/src/main/scala/chisel3/experimental/Trace.scala Co-authored-by: Schuyler Eldridge --- .../main/scala/chisel3/experimental/Trace.scala | 43 +++++++++++++++++++++- 1 file changed, 41 insertions(+), 2 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/experimental/Trace.scala b/core/src/main/scala/chisel3/experimental/Trace.scala index 3cc27162..33d18147 100644 --- a/core/src/main/scala/chisel3/experimental/Trace.scala +++ b/core/src/main/scala/chisel3/experimental/Trace.scala @@ -22,16 +22,22 @@ import firrtl.transforms.DontTouchAllTargets object Trace { /** Trace a Instance name. */ + @deprecated("switch to traceNameV2 (until Chisel 3.6)", "3.5.5") def traceName(x: Module): Unit = traceName(x: RawModule) /** Trace a Instance name. */ + @deprecated("switch to traceNameV2 (until Chisel 3.6)", "3.5.5") def traceName(x: RawModule): Unit = { annotate(new ChiselAnnotation { def toFirrtl: Annotation = TraceNameAnnotation(x.toAbsoluteTarget, x.toAbsoluteTarget) }) } - /** Trace a Data name. */ + /** Trace a Data name. This adds "don't touch" semantics to anything traced. */ + @deprecated( + "switch to traceNameV2 (until Chisel 3.6) and add dontTouch if you want \"don't touch\" behavior", + "3.5.5" + ) def traceName(x: Data): Unit = { x match { case aggregate: Aggregate => @@ -46,7 +52,29 @@ object Trace { } } - /** An Annotation that records the original target annotate from Chisel. + /** Trace an Instance name. */ + def traceNameV2(x: RawModule): Unit = { + annotate(new ChiselAnnotation { + def toFirrtl: Annotation = TraceAnnotation(x.toAbsoluteTarget, x.toAbsoluteTarget) + }) + } + + /** Trace a Data name. This does NOT add "don't touch" semantics to the traced data. If you want this behavior, use an explicit [[chisel3.dontTouch]]. */ + def traceNameV2(x: Data): Unit = { + x match { + case aggregate: Aggregate => + annotate(new ChiselAnnotation { + def toFirrtl: Annotation = TraceAnnotation(aggregate.toAbsoluteTarget, aggregate.toAbsoluteTarget) + }) + aggregate.getElements.foreach(traceNameV2) + case element: Element => + annotate(new ChiselAnnotation { + def toFirrtl: Annotation = TraceAnnotation(element.toAbsoluteTarget, element.toAbsoluteTarget) + }) + } + } + + /** An Annotation that records the original target annotate from Chisel. This adds don't touch behavior. * * @param target target that should be renamed by [[firrtl.RenameMap]] in the firrtl transforms. * @param chiselTarget original annotated target in Chisel, which should not be changed or renamed in FIRRTL. @@ -57,6 +85,16 @@ object Trace { def duplicate(n: T): Annotation = this.copy(target = n) } + /** An Annotation that records the original target annotate from Chisel. This does NOT add don't touch behavior. + * + * @param target target that should be renamed by [[firrtl.RenameMap]] in the firrtl transforms. + * @param chiselTarget original annotated target in Chisel, which should not be changed or renamed in FIRRTL. + */ + private case class TraceAnnotation[T <: CompleteTarget](target: T, chiselTarget: T) + extends SingleTargetAnnotation[T] { + def duplicate(n: T): Annotation = this.copy(target = n) + } + /** Get [[CompleteTarget]] of the target `x` for `annos`. * This API can be used to find the final reference to a signal or module which is marked by `traceName` */ @@ -68,5 +106,6 @@ object Trace { */ def finalTargetMap(annos: AnnotationSeq): Map[CompleteTarget, Seq[CompleteTarget]] = annos.collect { case TraceNameAnnotation(t, chiselTarget) => chiselTarget -> t + case TraceAnnotation(t, chiselTarget) => chiselTarget -> t }.groupBy(_._1).map { case (k, v) => k -> v.map(_._2) } } -- cgit v1.2.3 From 80b3b28f451efa85be50994f732599f043f83d86 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 19 Oct 2022 14:28:34 -0700 Subject: Don't modify the Builder prefix if reinvoking suggestName on a Data (backport #2789) (#2790) * Only set the chisel3 Builder prefix during the first invocation of suggestName (cherry picked from commit b684506abab2f7b99d56181d548cb8119d317323) # Conflicts: # core/src/main/scala/chisel3/internal/Builder.scala * Add simple test to show bug fix (cherry picked from commit 255068b105de77a045a0016e3a157b52a81c86d6) * Fix merge conflict * Fix test to not use Hardware inside a bundle for prefixing Co-authored-by: Jared Barocsi --- core/src/main/scala/chisel3/internal/Builder.scala | 9 ++++++--- 1 file changed, 6 insertions(+), 3 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 9f79fe1e..e3dfff09 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -170,9 +170,12 @@ private[chisel3] trait HasId extends InstanceId { * @return this object */ def suggestName(seed: => String): this.type = { - if (suggested_seed.isEmpty) suggested_seedVar = seed - naming_prefix = Builder.getPrefix - for (hook <- suggest_postseed_hooks.reverse) { hook(seed) } + if (suggested_seed.isEmpty) { + suggested_seedVar = seed + // Only set the prefix if a seed hasn't been suggested + naming_prefix = Builder.getPrefix + for (hook <- suggest_postseed_hooks.reverse) { hook(seed) } + } this } -- cgit v1.2.3 From d997acb05e5a307afb7c9ad4c136b9b4e1506efc Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Sun, 23 Oct 2022 19:01:43 +0000 Subject: Don't invalidate ExtModule ports in an explicitInvalidate = true context (backport #2795) (#2799) * Don't invalidate ExtModule ports in an explicitInvalidate = true context (#2795) * Don't invalidate ExtModule ports in an explicitInvalidate = true context ExtModule ports were previously invalidated in the emitted FIRRTL, which is correct in a NonStrict / `Chisel._` compatibility context but not in newer chisel3 code where `explicitInvalidate = true`. (cherry picked from commit 8e24a281545d25f6501dcc872eabdfb30bacd69d) # Conflicts: # core/src/main/scala/chisel3/BlackBox.scala * Resolve backport conflicts Co-authored-by: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com> Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/BlackBox.scala | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala index f618901f..c3cb3e66 100644 --- a/core/src/main/scala/chisel3/BlackBox.scala +++ b/core/src/main/scala/chisel3/BlackBox.scala @@ -92,8 +92,10 @@ package experimental { private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = { implicit val sourceInfo = UnlocatableSourceInfo - for (x <- getModulePorts) { - pushCommand(DefInvalid(sourceInfo, x.ref)) + if (!parentCompileOptions.explicitInvalidate) { + for (x <- getModulePorts) { + pushCommand(DefInvalid(sourceInfo, x.ref)) + } } } } @@ -192,8 +194,10 @@ abstract class BlackBox( } private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = { - for ((_, port) <- _io.map(_.elements).getOrElse(Nil)) { - pushCommand(DefInvalid(UnlocatableSourceInfo, port.ref)) + if (!parentCompileOptions.explicitInvalidate) { + for ((_, port) <- _io.map(_.elements).getOrElse(Nil)) { + pushCommand(DefInvalid(UnlocatableSourceInfo, port.ref)) + } } } } -- cgit v1.2.3 From f86c1ff7b39146f23cd1959bcc63dcb3b0b27125 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Sun, 23 Oct 2022 22:27:06 +0000 Subject: Fix for <> to BlackBox.IO with Compatibility Bundles (#2801) (#2803) MonoConnect.traceFlow now properly handles coerced directions. Also minor improvement to getClassName especially useful in test case printf debugging. (cherry picked from commit 3aba755bdcf996c0fbd846d13268fd6641b29e96) Co-authored-by: Megan Wachs --- core/src/main/scala/chisel3/Aggregate.scala | 20 +++++++++++---- .../main/scala/chisel3/internal/BiConnect.scala | 9 ++++--- .../main/scala/chisel3/internal/MonoConnect.scala | 30 ++++++++++++++++------ 3 files changed, 43 insertions(+), 16 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index aacf0b1c..4fc9b20f 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -1116,7 +1116,12 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio def elements: SeqMap[String, Data] /** Name for Pretty Printing */ - def className: String = this.getClass.getSimpleName + def className: String = try { + this.getClass.getSimpleName + } catch { + // This happens if your class is defined in an object and is anonymous + case e: java.lang.InternalError if e.getMessage == "Malformed class name" => this.getClass.toString + } private[chisel3] override def typeEquivalent(that: Data): Boolean = that match { case that: Record => @@ -1243,10 +1248,15 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record wi "Please see https://github.com/chipsalliance/chisel3#build-your-own-chisel-projects." ) - override def className: String = this.getClass.getSimpleName match { - case name if name.startsWith("$anon$") => "AnonymousBundle" // fallback for anonymous Bundle case - case "" => "AnonymousBundle" // ditto, but on other platforms - case name => name + override def className: String = try { + this.getClass.getSimpleName match { + case name if name.startsWith("$anon$") => "AnonymousBundle" // fallback for anonymous Bundle case + case "" => "AnonymousBundle" // ditto, but on other platforms + case name => name + } + } catch { + // This happens if you have nested objects which your class is defined in + case e: java.lang.InternalError if e.getMessage == "Malformed class name" => this.getClass.toString } /** The collection of [[Data]] diff --git a/core/src/main/scala/chisel3/internal/BiConnect.scala b/core/src/main/scala/chisel3/internal/BiConnect.scala index e8fb2361..74376598 100644 --- a/core/src/main/scala/chisel3/internal/BiConnect.scala +++ b/core/src/main/scala/chisel3/internal/BiConnect.scala @@ -227,9 +227,12 @@ private[chisel3] object BiConnect { context_mod: RawModule ): Unit = { // Verify right has no extra fields that left doesn't have - for ((field, right_sub) <- right_r.elements) { - if (!left_r.elements.isDefinedAt(field)) { - if (connectCompileOptions.connectFieldsMustMatch) { + + // For each field in left, descend with right. + // Don't bother doing this check if we don't expect it to necessarily pass. + if (connectCompileOptions.connectFieldsMustMatch) { + for ((field, right_sub) <- right_r.elements) { + if (!left_r.elements.isDefinedAt(field)) { throw MissingLeftFieldException(field) } } diff --git a/core/src/main/scala/chisel3/internal/MonoConnect.scala b/core/src/main/scala/chisel3/internal/MonoConnect.scala index 31364804..4e762a7c 100644 --- a/core/src/main/scala/chisel3/internal/MonoConnect.scala +++ b/core/src/main/scala/chisel3/internal/MonoConnect.scala @@ -322,21 +322,35 @@ private[chisel3] object MonoConnect { else false } - /** Trace flow from child Data to its parent. */ - @tailrec private[chisel3] def traceFlow(currentlyFlipped: Boolean, data: Data, context_mod: RawModule): Boolean = { - import SpecifiedDirection.{Input => SInput, Flip => SFlip} + /** Trace flow from child Data to its parent. + * + * Returns true if, given the context, + * this signal can be a sink when wantsToBeSink = true, + * or if it can be a source when wantsToBeSink = false. + * Always returns true if the Data does not actually correspond + * to a Port. + */ + @tailrec private[chisel3] def traceFlow( + wantToBeSink: Boolean, + currentlyFlipped: Boolean, + data: Data, + context_mod: RawModule + ): Boolean = { val sdir = data.specifiedDirection - val flipped = sdir == SInput || sdir == SFlip + val coercedFlip = sdir == SpecifiedDirection.Input + val coercedAlign = sdir == SpecifiedDirection.Output + val flipped = sdir == SpecifiedDirection.Flip + val traceFlipped = ((flipped ^ currentlyFlipped) || coercedFlip) && (!coercedAlign) data.binding.get match { - case ChildBinding(parent) => traceFlow(flipped ^ currentlyFlipped, parent, context_mod) + case ChildBinding(parent) => traceFlow(wantToBeSink, traceFlipped, parent, context_mod) case PortBinding(enclosure) => val childPort = enclosure != context_mod - childPort ^ flipped ^ currentlyFlipped + wantToBeSink ^ childPort ^ traceFlipped case _ => true } } - def canBeSink(data: Data, context_mod: RawModule): Boolean = traceFlow(true, data, context_mod) - def canBeSource(data: Data, context_mod: RawModule): Boolean = traceFlow(false, data, context_mod) + def canBeSink(data: Data, context_mod: RawModule): Boolean = traceFlow(true, false, data, context_mod) + def canBeSource(data: Data, context_mod: RawModule): Boolean = traceFlow(false, false, data, context_mod) /** Check whether two aggregates can be bulk connected (<=) in FIRRTL. (MonoConnect case) * -- cgit v1.2.3 From 4149157df6531d124483d992daf96cf4e62a0f0c Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 4 Nov 2022 18:20:07 +0000 Subject: Add PartialDataView.supertype (backport #2826) (#2827) * Add PartialDataView.supertype (#2826) This factory method makes it easy to create PartialDataViews from a Bundle type to its supertype. Because of the typing relationship, there is no need to provide a mapping between fields. The only thing necessary is to provide a function for constructing an instance of the supertype from an instance of the subtype. (cherry picked from commit 251d454a224e5a961438ba0ea41134d7da7a5992) # Conflicts: # core/src/main/scala/chisel3/experimental/dataview/package.scala # src/test/scala/chiselTests/experimental/DataView.scala * Resolve backport conflicts Co-authored-by: Jack Koenig --- .../chisel3/experimental/dataview/DataView.scala | 23 ++++++++++++++++++++++ .../chisel3/experimental/dataview/package.scala | 14 ++----------- 2 files changed, 25 insertions(+), 12 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/experimental/dataview/DataView.scala b/core/src/main/scala/chisel3/experimental/dataview/DataView.scala index 7f20964d..cc555b11 100644 --- a/core/src/main/scala/chisel3/experimental/dataview/DataView.scala +++ b/core/src/main/scala/chisel3/experimental/dataview/DataView.scala @@ -592,4 +592,27 @@ object PartialDataView { implicit sourceInfo: SourceInfo ): DataView[T, V] = new DataView[T, V](mkView, mapping, _total = false) + + /** Constructs a non-total [[DataView]] mapping from a [[Bundle]] type to a parent [[Bundle]] type + * + * @param mkView a function constructing an instance `V` from an instance of `T` + * @return the [[DataView]] that enables viewing instances of a [[Bundle]] as instances of a parent type + */ + def supertype[T <: Bundle, V <: Bundle]( + mkView: T => V + )( + implicit ev: SubTypeOf[T, V], + sourceInfo: SourceInfo + ): DataView[T, V] = + mapping[T, V]( + mkView, + { + case (a, b) => + val aElts = a.elements + val bElts = b.elements + val bKeys = bElts.keySet + val keys = aElts.keysIterator.filter(bKeys.contains) + keys.map(k => aElts(k) -> bElts(k)).toSeq + } + ) } diff --git a/core/src/main/scala/chisel3/experimental/dataview/package.scala b/core/src/main/scala/chisel3/experimental/dataview/package.scala index 71ae2d8f..a52e88cf 100644 --- a/core/src/main/scala/chisel3/experimental/dataview/package.scala +++ b/core/src/main/scala/chisel3/experimental/dataview/package.scala @@ -43,24 +43,14 @@ package object dataview { "${A} is not a subtype of ${B}! Did you mean .viewAs[${B}]? " + "Please see https://www.chisel-lang.org/chisel3/docs/cookbooks/dataview" ) - private type SubTypeOf[A, B] = A <:< B + private[dataview] type SubTypeOf[A, B] = A <:< B /** Provides `viewAsSupertype` for subclasses of [[Bundle]] */ implicit class BundleUpcastable[T <: Bundle](target: T) { /** View a [[Bundle]] or [[Record]] as a parent type (upcast) */ def viewAsSupertype[V <: Bundle](proto: V)(implicit ev: SubTypeOf[T, V], sourceInfo: SourceInfo): V = { - implicit val dataView = PartialDataView.mapping[T, V]( - _ => proto, - { - case (a, b) => - val aElts = a.elements - val bElts = b.elements - val bKeys = bElts.keySet - val keys = aElts.keysIterator.filter(bKeys.contains) - keys.map(k => aElts(k) -> bElts(k)).toSeq - } - ) + implicit val dataView = PartialDataView.supertype[T, V](_ => proto) target.viewAs[V] } } -- cgit v1.2.3 From 017bd6b9c96974df2a3c4f35e069d60fec001f2e Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Sat, 5 Nov 2022 22:31:07 +0000 Subject: Support Analog in DataView (#2782) (#2828) Co-authored-by: Megan Wachs (cherry picked from commit 26100a875c69bf56f7442fac82ca9c74ad3596eb) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Attach.scala | 45 ---------------------- core/src/main/scala/chisel3/Data.scala | 1 + .../main/scala/chisel3/experimental/Analog.scala | 3 +- .../main/scala/chisel3/experimental/Attach.scala | 45 ++++++++++++++++++++++ 4 files changed, 48 insertions(+), 46 deletions(-) delete mode 100644 core/src/main/scala/chisel3/Attach.scala create mode 100644 core/src/main/scala/chisel3/experimental/Attach.scala (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Attach.scala b/core/src/main/scala/chisel3/Attach.scala deleted file mode 100644 index 5c9cfe53..00000000 --- a/core/src/main/scala/chisel3/Attach.scala +++ /dev/null @@ -1,45 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -package chisel3.experimental - -import chisel3.RawModule -import chisel3.internal._ -import chisel3.internal.Builder.pushCommand -import chisel3.internal.firrtl._ -import chisel3.internal.sourceinfo.SourceInfo - -object attach { - // Exceptions that can be generated by attach - case class AttachException(message: String) extends ChiselException(message) - def ConditionalAttachException: AttachException = - AttachException(": Conditional attach is not allowed!") - - // Actual implementation - private[chisel3] def impl(elts: Seq[Analog], contextModule: RawModule)(implicit sourceInfo: SourceInfo): Unit = { - if (Builder.whenDepth != 0) throw ConditionalAttachException - - // TODO Check that references are valid and can be attached - - pushCommand(Attach(sourceInfo, elts.map(_.lref))) - } - - /** Create an electrical connection between [[Analog]] components - * - * @param elts The components to attach - * - * @example - * {{{ - * val a1 = Wire(Analog(32.W)) - * val a2 = Wire(Analog(32.W)) - * attach(a1, a2) - * }}} - */ - def apply(elts: Analog*)(implicit sourceInfo: SourceInfo): Unit = { - try { - impl(elts, Builder.forcedUserModule) - } catch { - case AttachException(message) => - throwException(elts.mkString("Attaching (", ", ", s") failed @$message")) - } - } -} diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index f52f99de..52cc041c 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -684,6 +684,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { topBindingOpt match { case Some(binding: ReadOnlyBinding) => throwException(s"internal error: attempted to generate LHS ref to ReadOnlyBinding $binding") + case Some(ViewBinding(target)) => reify(target).lref case Some(binding: TopBinding) => Node(this) case opt => throwException(s"internal error: unknown binding $opt in generating LHS ref") } diff --git a/core/src/main/scala/chisel3/experimental/Analog.scala b/core/src/main/scala/chisel3/experimental/Analog.scala index a366f0c3..7d89025c 100644 --- a/core/src/main/scala/chisel3/experimental/Analog.scala +++ b/core/src/main/scala/chisel3/experimental/Analog.scala @@ -69,7 +69,8 @@ final class Analog private (private[chisel3] val width: Width) extends Element { } targetTopBinding match { - case _: WireBinding | _: PortBinding => direction = ActualDirection.Bidirectional(ActualDirection.Default) + case _: WireBinding | _: PortBinding | _: ViewBinding | _: AggregateViewBinding => + direction = ActualDirection.Bidirectional(ActualDirection.Default) case x => throwException(s"Analog can only be Ports and Wires, not '$x'") } binding = target diff --git a/core/src/main/scala/chisel3/experimental/Attach.scala b/core/src/main/scala/chisel3/experimental/Attach.scala new file mode 100644 index 00000000..5c9cfe53 --- /dev/null +++ b/core/src/main/scala/chisel3/experimental/Attach.scala @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chisel3.experimental + +import chisel3.RawModule +import chisel3.internal._ +import chisel3.internal.Builder.pushCommand +import chisel3.internal.firrtl._ +import chisel3.internal.sourceinfo.SourceInfo + +object attach { + // Exceptions that can be generated by attach + case class AttachException(message: String) extends ChiselException(message) + def ConditionalAttachException: AttachException = + AttachException(": Conditional attach is not allowed!") + + // Actual implementation + private[chisel3] def impl(elts: Seq[Analog], contextModule: RawModule)(implicit sourceInfo: SourceInfo): Unit = { + if (Builder.whenDepth != 0) throw ConditionalAttachException + + // TODO Check that references are valid and can be attached + + pushCommand(Attach(sourceInfo, elts.map(_.lref))) + } + + /** Create an electrical connection between [[Analog]] components + * + * @param elts The components to attach + * + * @example + * {{{ + * val a1 = Wire(Analog(32.W)) + * val a2 = Wire(Analog(32.W)) + * attach(a1, a2) + * }}} + */ + def apply(elts: Analog*)(implicit sourceInfo: SourceInfo): Unit = { + try { + impl(elts, Builder.forcedUserModule) + } catch { + case AttachException(message) => + throwException(elts.mkString("Attaching (", ", ", s") failed @$message")) + } + } +} -- cgit v1.2.3 From 086c6806708d14ad5144ca064d4c644d0f62592d Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 7 Nov 2022 18:29:31 +0000 Subject: Add DataMirror.getParent for getting parents of Modules (#2825) (#2833) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit fce8394bb0ddc9ae0d9c6668e034e483bd6b71c5) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Data.scala | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 52cc041c..3af5ade1 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -265,6 +265,14 @@ package experimental { } } + /** Returns the parent module within which a module instance is instantiated + * + * @note Top-level modules in any given elaboration do not have a parent + * @param target a module instance + * @return the parent of the `target`, if one exists + */ + def getParent(target: BaseModule): Option[BaseModule] = target._parent + // Internal reflection-style APIs, subject to change and removal whenever. object internal { def isSynthesizable(target: Data): Boolean = target.isSynthesizable -- cgit v1.2.3 From 76ada881d077118384907f498576b3b338291ff6 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 7 Nov 2022 19:13:49 +0000 Subject: Bugfix converter clearing flips (backport #2788) (#2832) * Bugfix converter clearing flips (#2788) * Bugfix: Output on Vec of bundle with mixed field orientations * Bugfix OpaqueTypes clearing flips (cherry picked from commit f05bff1a337589bafebd08783bb0f6a72092a95a) # Conflicts: # src/test/scala/chiselTests/Direction.scala * Resolve backport conflicts Co-authored-by: Adam Izraelevitz Co-authored-by: Jack Koenig Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>--- core/src/main/scala/chisel3/internal/firrtl/Converter.scala | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index fe95445c..f73e85d2 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -301,8 +301,11 @@ private[chisel3] object Converter { case d: SInt => fir.SIntType(convert(d.width)) case d: FixedPoint => fir.FixedType(convert(d.width), convert(d.binaryPoint)) case d: Interval => fir.IntervalType(d.range.lowerBound, d.range.upperBound, d.range.firrtlBinaryPoint) - case d: Analog => fir.AnalogType(convert(d.width)) - case d: Vec[_] => fir.VectorType(extractType(d.sample_element, clearDir, info), d.length) + case d: Analog => fir.AnalogType(convert(d.width)) + case d: Vec[_] => + val childClearDir = clearDir || + d.specifiedDirection == SpecifiedDirection.Input || d.specifiedDirection == SpecifiedDirection.Output + fir.VectorType(extractType(d.sample_element, childClearDir, info), d.length) case d: Record => { val childClearDir = clearDir || d.specifiedDirection == SpecifiedDirection.Input || d.specifiedDirection == SpecifiedDirection.Output @@ -316,7 +319,7 @@ private[chisel3] object Converter { if (!d.opaqueType) fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) }) else - extractType(d.elements.head._2, true, info) + extractType(d.elements.head._2, childClearDir, info) } } -- cgit v1.2.3 From f2ef3a8ee378a307661bd598cd44d4b895b9352e Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 8 Nov 2022 07:05:53 +0000 Subject: Improve Record.bind and Detect Records with unstable elements (backport #2829) (#2831) * Add Aggregate.elementsIterator and micro-optimize elementsIterator provides a more efficient API for iterating on the elements of Aggregates. It is especially useful for Records where getElements returns a Seq and thus eagerly constructs a new datastructure which may then just be iterated on anyway. This new elementsIterator API is then used throughout the codebase where it makes sense. Also change Vec.getElements to just return the underlying self instead of constructing a new Seq. (cherry picked from commit defa440b349031475daeff4024fad04925cccee6) # Conflicts: # core/src/main/scala/chisel3/Aggregate.scala # core/src/main/scala/chisel3/Module.scala # core/src/main/scala/chisel3/experimental/Trace.scala * Move Aggregate.bind inline into Record.bind Vec overrides bind and does not call the version in Aggregate so the version in Aggregate is misleading in that its only ever used by Records. Now there is no version in Aggregate and the actual functionality and use is more clear. (cherry picked from commit b054c30ba47026cb2a9b28c696a0a0a58b1e2ee7) # Conflicts: # core/src/main/scala/chisel3/Aggregate.scala * Extract and optimize duplicate checking Record.bind This replaces an immutable.Map with a single mutable.HashSet and saves the allocation of # elements Seqs. (cherry picked from commit 832ea52bc23424bb75b9654422b725a9cafaef40) # Conflicts: # core/src/main/scala/chisel3/Aggregate.scala * Add check for Records that define def elements (cherry picked from commit a4f223415de19e2a732e0b6a8fe681f706a19a56) * Resolve backport conflicts * Make elementsIterator final and package private * Waive false MiMa failure Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Aggregate.scala | 123 ++++++++++++--------- core/src/main/scala/chisel3/Data.scala | 16 +-- core/src/main/scala/chisel3/Module.scala | 4 +- .../main/scala/chisel3/experimental/Trace.scala | 2 +- 4 files changed, 81 insertions(+), 64 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 4fc9b20f..16611277 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -23,52 +23,6 @@ class AliasedAggregateFieldException(message: String) extends ChiselException(me * of) other Data objects. */ sealed abstract class Aggregate extends Data { - private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = { - _parent.foreach(_.addId(this)) - binding = target - - val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection) - val duplicates = getElements.groupBy(identity).collect { case (x, elts) if elts.size > 1 => x } - if (!duplicates.isEmpty) { - this match { - case b: Record => - // show groups of names of fields with duplicate id's - // The sorts make the displayed order of fields deterministic and matching the order of occurrence in the Bundle. - // It's a bit convoluted but happens rarely and makes the error message easier to understand - val dupNames = duplicates.toSeq - .sortBy(_._id) - .map { duplicate => - b.elements.collect { case x if x._2._id == duplicate._id => x }.toSeq - .sortBy(_._2._id) - .map(_._1) - .reverse - .mkString("(", ",", ")") - } - .mkString(",") - throw new AliasedAggregateFieldException( - s"${b.className} contains aliased fields named ${dupNames}" - ) - case _ => - throw new AliasedAggregateFieldException( - s"Aggregate ${this.getClass} contains aliased fields $duplicates ${duplicates.mkString(",")}" - ) - } - } - for (child <- getElements) { - child.bind(ChildBinding(this), resolvedDirection) - } - - // Check that children obey the directionality rules. - val childDirections = getElements.map(_.direction).toSet - ActualDirection.Empty - direction = ActualDirection.fromChildren(childDirections, resolvedDirection) match { - case Some(dir) => dir - case None => - val childWithDirections = getElements.zip(getElements.map(_.direction)) - throw MixedDirectionAggregateException( - s"Aggregate '$this' can't have elements that are both directioned and undirectioned: $childWithDirections" - ) - } - } /** 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. @@ -100,7 +54,10 @@ sealed abstract class Aggregate extends Data { */ def getElements: Seq[Data] - private[chisel3] def width: Width = getElements.map(_.width).foldLeft(0.W)(_ + _) + /** Similar to [[getElements]] but allows for more optimized use */ + private[chisel3] def elementsIterator: Iterator[Data] + + private[chisel3] def width: Width = elementsIterator.map(_.width).foldLeft(0.W)(_ + _) private[chisel3] def legacyConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = { // If the source is a DontCare, generate a DefInvalid for the sink, @@ -221,7 +178,7 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection) sample_element.bind(SampleElementBinding(this), resolvedDirection) - for (child <- getElements) { // assume that all children are the same + for (child <- elementsIterator) { // assume that all children are the same child.bind(ChildBinding(this), resolvedDirection) } @@ -342,8 +299,9 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend new Vec(gen.cloneTypeFull, length).asInstanceOf[this.type] } - override def getElements: Seq[Data] = - (0 until length).map(apply(_)) + override def getElements: Seq[Data] = self + + final override private[chisel3] def elementsIterator: Iterator[Data] = self.iterator /** Default "pretty-print" implementation * Analogous to printing a Seq @@ -953,9 +911,66 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio } } + /** Checks that there are no duplicate elements (aka aliased fields) in the Record */ + private def checkForAndReportDuplicates(): Unit = { + // Using List to avoid allocation in the common case of no duplicates + var duplicates: List[Data] = Nil + // Is there a more optimized datastructure we could use with the Int identities? BitSet? Requires benchmarking. + val seen = mutable.HashSet.empty[Data] + this.elementsIterator.foreach { e => + if (seen(e)) { + duplicates = e :: duplicates + } + seen += e + } + if (!duplicates.isEmpty) { + // show groups of names of fields with duplicate id's + // The sorts make the displayed order of fields deterministic and matching the order of occurrence in the Bundle. + // It's a bit convoluted but happens rarely and makes the error message easier to understand + val dupNames = duplicates.toSeq + .sortBy(_._id) + .map { duplicate => + this.elements.collect { case x if x._2._id == duplicate._id => x }.toSeq + .sortBy(_._2._id) + .map(_._1) + .reverse + .mkString("(", ",", ")") + } + .mkString(",") + throw new AliasedAggregateFieldException( + s"${this.className} contains aliased fields named ${dupNames}" + ) + } + } + private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = { try { - super.bind(target, parentDirection) + _parent.foreach(_.addId(this)) + binding = target + + val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection) + + checkForAndReportDuplicates() + + for ((child, sameChild) <- this.elementsIterator.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?" + ) + } + child.bind(ChildBinding(this), resolvedDirection) + } + + // Check that children obey the directionality rules. + val childDirections = elementsIterator.map(_.direction).toSet - ActualDirection.Empty + direction = ActualDirection.fromChildren(childDirections, resolvedDirection) match { + case Some(dir) => dir + case None => + val childWithDirections = getElements.zip(getElements.map(_.direction)) + throw MixedDirectionAggregateException( + s"Aggregate '$this' can't have elements that are both directioned and undirectioned: $childWithDirections" + ) + } } catch { // nasty compatibility mode shim, where anything flies case e: MixedDirectionAggregateException if !compileOptions.dontAssumeDirectionality => val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection) @@ -1142,9 +1157,11 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio } } - private[chisel3] final def allElements: Seq[Element] = elements.toIndexedSeq.flatMap(_._2.allElements) + private[chisel3] final def allElements: Seq[Element] = elementsIterator.flatMap(_.allElements).toIndexedSeq + + override def getElements: Seq[Data] = elementsIterator.toIndexedSeq - override def getElements: Seq[Data] = elements.toIndexedSeq.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 = { diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 3af5ade1..50093333 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -361,7 +361,7 @@ private[chisel3] object getRecursiveFields { _ ++ _ } case data: Vec[_] => - data.getElements.zipWithIndex.map { + data.elementsIterator.zipWithIndex.map { case (fieldData, fieldIndex) => getRecursiveFields(fieldData, path = s"$path($fieldIndex)") }.fold(Seq(data -> path)) { @@ -379,7 +379,7 @@ private[chisel3] object getRecursiveFields { } case data: Vec[_] => LazyList(data -> path) ++ - data.getElements.view.zipWithIndex.flatMap { + data.elementsIterator.zipWithIndex.flatMap { case (fieldData, fieldIndex) => getRecursiveFields(fieldData, path = s"$path($fieldIndex)") } @@ -406,8 +406,8 @@ private[chisel3] object getMatchedFields { _ ++ _ } case (x: Vec[_], y: Vec[_]) => - (x.getElements - .zip(y.getElements)) + (x.elementsIterator + .zip(y.elementsIterator)) .map { case (xElt, yElt) => getMatchedFields(xElt, yElt) @@ -464,7 +464,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { @deprecated("pending removal once all instances replaced", "chisel3") private[chisel3] def flatten: IndexedSeq[Element] = { this match { - case elt: Aggregate => elt.getElements.toIndexedSeq.flatMap { _.flatten } + case elt: Aggregate => elt.elementsIterator.toIndexedSeq.flatMap { _.flatten } case elt: Element => IndexedSeq(elt) case elt => throwException(s"Cannot flatten type ${elt.getClass}") } @@ -749,7 +749,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { data match { case _: Element => case agg: Aggregate => - agg.getElements.foreach(rec) + agg.elementsIterator.foreach(rec) } } rec(this) @@ -931,8 +931,8 @@ object Data { if (thiz.length != that.length) { throwException(s"Cannot compare Vecs $thiz and $that: Vec sizes differ") } else { - thiz.getElements - .zip(that.getElements) + thiz.elementsIterator + .zip(that.elementsIterator) .map { case (thisData, thatData) => thisData === thatData } .reduce(_ && _) } diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index ba2d2e32..9315a44b 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -709,9 +709,9 @@ package experimental { data match { case record: Record => val compatRecord = !record.compileOptions.dontAssumeDirectionality - record.getElements.foreach(assignCompatDir(_, compatRecord)) + record.elementsIterator.foreach(assignCompatDir(_, compatRecord)) case vec: Vec[_] => - vec.getElements.foreach(assignCompatDir(_, insideCompat)) + vec.elementsIterator.foreach(assignCompatDir(_, insideCompat)) } case SpecifiedDirection.Input | SpecifiedDirection.Output => // forced assign, nothing to do } diff --git a/core/src/main/scala/chisel3/experimental/Trace.scala b/core/src/main/scala/chisel3/experimental/Trace.scala index 33d18147..eb2ed46a 100644 --- a/core/src/main/scala/chisel3/experimental/Trace.scala +++ b/core/src/main/scala/chisel3/experimental/Trace.scala @@ -66,7 +66,7 @@ object Trace { annotate(new ChiselAnnotation { def toFirrtl: Annotation = TraceAnnotation(aggregate.toAbsoluteTarget, aggregate.toAbsoluteTarget) }) - aggregate.getElements.foreach(traceNameV2) + aggregate.elementsIterator.foreach(traceNameV2) case element: Element => annotate(new ChiselAnnotation { def toFirrtl: Annotation = TraceAnnotation(element.toAbsoluteTarget, element.toAbsoluteTarget) -- cgit v1.2.3 From bfa9f7465e6069b1e624126f9e14245b69e7c0a9 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 8 Nov 2022 17:27:07 +0000 Subject: Switch to using experimental trait for OpaqueTypes (backport #2783) (#2836) * Switch to using experimental trait for OpaqueTypes (#2783) This makes it more clear that the feature is experimental. Users may still override the opaqueType method for more dynamic control over when instances of a given Record are OpaqueTypes or not, but they are discouraged from doing so. (cherry picked from commit 7525dc71ccc2050d8e4a68b38f3b76920ba693fc) * Fix cloneType in RecordSpec Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Aggregate.scala | 20 +++++----------- .../scala/chisel3/experimental/OpaqueType.scala | 27 ++++++++++++++++++++++ .../scala/chisel3/internal/firrtl/Converter.scala | 2 +- 3 files changed, 34 insertions(+), 15 deletions(-) create mode 100644 core/src/main/scala/chisel3/experimental/OpaqueType.scala (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 16611277..f22f5e63 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -8,7 +8,7 @@ import chisel3.experimental.dataview.{isView, reifySingleData, InvalidViewExcept import scala.collection.immutable.{SeqMap, VectorMap} import scala.collection.mutable.{HashSet, LinkedHashMap} import scala.language.experimental.macros -import chisel3.experimental.{BaseModule, BundleLiteralException, ChiselEnum, EnumType, VecLiteralException} +import chisel3.experimental.{BaseModule, BundleLiteralException, ChiselEnum, EnumType, OpaqueType, VecLiteralException} import chisel3.internal._ import chisel3.internal.Builder.pushCommand import chisel3.internal.firrtl._ @@ -881,23 +881,15 @@ trait VecLike[T <: Data] extends IndexedSeq[T] with HasId with SourceInfoDoc { */ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptions) extends Aggregate { - /** Indicates if this Record represents an "Opaque Type" - * - * Opaque types provide a mechanism for user-defined types - * that do not impose any "boxing" overhead in the emitted FIRRTL and Verilog. - * You can think about an opaque type Record as a box around - * a single element that only exists at Chisel elaboration time. - * Put another way, if opaqueType is overridden to true, - * The Record may only contain a single element with an empty name - * and there will be no `_` in the name for that element in the emitted Verilog. - * - * @see RecordSpec in Chisel's tests for example usage and expected output - */ - def opaqueType: Boolean = false + private[chisel3] def _isOpaqueType: Boolean = this match { + case maybe: OpaqueType => maybe.opaqueType + case _ => false + } // Doing this earlier than onModuleClose allows field names to be available for prefixing the names // of hardware created when connecting to one of these elements private def setElementRefs(): Unit = { + val opaqueType = this._isOpaqueType // Since elements is a map, it is impossible for two elements to have the same // identifier; however, Namespace sanitizes identifiers to make them legal for Firrtl/Verilog // which can cause collisions diff --git a/core/src/main/scala/chisel3/experimental/OpaqueType.scala b/core/src/main/scala/chisel3/experimental/OpaqueType.scala new file mode 100644 index 00000000..e7a2a15d --- /dev/null +++ b/core/src/main/scala/chisel3/experimental/OpaqueType.scala @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chisel3.experimental + +import chisel3._ + +/** Indicates if this Record represents an "Opaque Type" + * + * Opaque types provide a mechanism for user-defined types + * that do not impose any "boxing" overhead in the emitted FIRRTL and Verilog. + * You can think about an opaque type Record as a box around + * a single element that only exists at Chisel elaboration time. + * Put another way, if this trait is mixed into a Record, + * the Record may only contain a single element with an empty name + * and there will be no `_` in the name for that element in the emitted Verilog. + * + * @see RecordSpec in Chisel's tests for example usage and expected output + */ +trait OpaqueType { self: Record => + + /** If set to true, indicates that this Record is an OpaqueType + * + * Users can override this if they need more dynamic control over the behavior for when + * instances of this type are considered opaque + */ + def opaqueType: Boolean = true +} diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index f73e85d2..3d6e0d79 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -316,7 +316,7 @@ private[chisel3] object Converter { case (false, SpecifiedDirection.Flip | SpecifiedDirection.Input) => fir.Field(getRef(elt, info).name, fir.Flip, extractType(elt, false, info)) } - if (!d.opaqueType) + if (!d._isOpaqueType) fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) }) else extractType(d.elements.head._2, childClearDir, info) -- cgit v1.2.3 From be4463a7756351dcab09ba3f576f5e3687fb0ebf Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 10 Nov 2022 20:03:45 +0000 Subject: Unify Chisel2 and chisel3 directionality (backport #2634) (#2837) * Unify Chisel2 and chisel3 directionality (#2634) Co-authored-by: Jack Koenig (cherry picked from commit 1aea4ef96466cbe08150d20c85c88b81e4e4f80f) # Conflicts: # core/src/main/scala/chisel3/Aggregate.scala # core/src/main/scala/chisel3/Module.scala # src/test/scala/chiselTests/Direction.scala * fix up backport * fix up backport * clean up diff * make test order like it was on master Co-authored-by: Adam Izraelevitz Co-authored-by: Megan Wachs --- core/src/main/scala/chisel3/Aggregate.scala | 42 ++++++++++++----------------- core/src/main/scala/chisel3/Module.scala | 27 ++++++++++--------- 2 files changed, 31 insertions(+), 38 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index f22f5e63..b6836ea7 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -936,37 +936,29 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio } private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = { - try { - _parent.foreach(_.addId(this)) - binding = target + _parent.foreach(_.addId(this)) + binding = target - val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection) + val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection) - checkForAndReportDuplicates() + checkForAndReportDuplicates() - for ((child, sameChild) <- this.elementsIterator.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?" - ) - } - child.bind(ChildBinding(this), resolvedDirection) + for ((child, sameChild) <- this.elementsIterator.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?" + ) } + child.bind(ChildBinding(this), resolvedDirection) + } - // Check that children obey the directionality rules. - val childDirections = elementsIterator.map(_.direction).toSet - ActualDirection.Empty - direction = ActualDirection.fromChildren(childDirections, resolvedDirection) match { - case Some(dir) => dir - case None => - val childWithDirections = getElements.zip(getElements.map(_.direction)) - throw MixedDirectionAggregateException( - s"Aggregate '$this' can't have elements that are both directioned and undirectioned: $childWithDirections" - ) - } - } catch { // nasty compatibility mode shim, where anything flies - case e: MixedDirectionAggregateException if !compileOptions.dontAssumeDirectionality => + // Check that children obey the directionality rules. + val childDirections = elementsIterator.map(_.direction).toSet - ActualDirection.Empty + direction = ActualDirection.fromChildren(childDirections, resolvedDirection) match { + case Some(dir) => dir + case None => val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection) - direction = resolvedDirection match { + resolvedDirection match { case SpecifiedDirection.Unspecified => ActualDirection.Bidirectional(ActualDirection.Default) case SpecifiedDirection.Flip => ActualDirection.Bidirectional(ActualDirection.Flipped) case _ => ActualDirection.Bidirectional(ActualDirection.Default) diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 9315a44b..48c33083 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -692,33 +692,34 @@ package experimental { */ protected def _bindIoInPlace(iodef: Data): Unit = { // Compatibility code: Chisel2 did not require explicit direction on nodes - // (unspecified treated as output, and flip on nothing was input). - // This sets assigns the explicit directions required by newer semantics on - // Bundles defined in compatibility mode. + // (unspecified treated as output, and flip on nothing was input). + // However, we are going to go back to Chisel2 semantics, so we need to make it work + // even for chisel3 code. + // This assigns the explicit directions required by both semantics on all Bundles. // This recursively walks the tree, and assigns directions if no explicit - // direction given by upper-levels (override Input / Output) AND element is - // directly inside a compatibility Bundle determined by compile options. - def assignCompatDir(data: Data, insideCompat: Boolean): Unit = { + // direction given by upper-levels (override Input / Output) + def assignCompatDir(data: Data): Unit = { data match { - case data: Element if insideCompat => data._assignCompatibilityExplicitDirection - case data: Element => // Not inside a compatibility Bundle, nothing to be done + case data: Element => data._assignCompatibilityExplicitDirection case data: Aggregate => data.specifiedDirection match { // Recurse into children to ensure explicit direction set somewhere case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip => data match { case record: Record => - val compatRecord = !record.compileOptions.dontAssumeDirectionality - record.elementsIterator.foreach(assignCompatDir(_, compatRecord)) + record.elementsIterator.foreach(assignCompatDir(_)) case vec: Vec[_] => - vec.elementsIterator.foreach(assignCompatDir(_, insideCompat)) + vec.elementsIterator.foreach(assignCompatDir(_)) } - case SpecifiedDirection.Input | SpecifiedDirection.Output => // forced assign, nothing to do + case SpecifiedDirection.Input | SpecifiedDirection.Output => + // forced assign, nothing to do + // Note this is because Input and Output recurse down their types to align all fields to that SpecifiedDirection + // Thus, no implicit assigment is necessary. } } } - assignCompatDir(iodef, false) + assignCompatDir(iodef) iodef.bind(PortBinding(this)) _ports += iodef -- cgit v1.2.3 From 17c04998d8cd5eeb4eff9506465fd2d6892793d2 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 10 Nov 2022 21:11:55 +0000 Subject: Add unit tests and fix for #2794 , add unit tests for #2773 (backport #2792) (#2834) * Fixup and unit tests for D/I of IOs without explicit Input/Output (#2792) (cherry picked from commit f24a624863f0fc460fd862238688ea8612ffdf5e) # Conflicts: # core/src/main/scala/chisel3/Module.scala * Resolve backport conflicts Co-authored-by: Megan Wachs Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Module.scala | 67 ++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 29 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 48c33083..e5c2a848 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -106,6 +106,38 @@ object Module extends SourceInfoDoc { module } + + /** Assign directionality on any IOs that are still Unspecified/Flipped + * + * Chisel2 did not require explicit direction on nodes + * (unspecified treated as output, and flip on nothing was input). + * As of 3.6, chisel3 is now also using these semantics, so we need to make it work + * even for chisel3 code. + * This assigns the explicit directions required by both semantics on all Bundles. + * This recursively walks the tree, and assigns directions if no explicit + * direction given by upper-levels (override Input / Output) + */ + private[chisel3] def assignCompatDir(data: Data): Unit = { + data match { + case data: Element => data._assignCompatibilityExplicitDirection + case data: Aggregate => + data.specifiedDirection match { + // Recurse into children to ensure explicit direction set somewhere + case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip => + data match { + case record: Record => + record.elementsIterator.foreach(assignCompatDir(_)) + case vec: Vec[_] => + vec.elementsIterator.foreach(assignCompatDir(_)) + assignCompatDir(vec.sample_element) // This is used in fromChildren computation + } + case SpecifiedDirection.Input | SpecifiedDirection.Output => + // forced assign, nothing to do + // The .bind algorithm will automatically assign the direction here. + // Thus, no implicit assignment is necessary. + } + } + } } /** Abstract base class for Modules, which behave much like Verilog modules. @@ -382,6 +414,10 @@ package internal { new ClonePorts(proto.getChiselPorts :+ ("io" -> b._io.get): _*) case _ => new ClonePorts(proto.getChiselPorts: _*) } + // getChiselPorts (nor cloneTypeFull in general) + // does not recursively copy the right specifiedDirection, + // still need to fix it up here. + Module.assignCompatDir(clonePorts) clonePorts.bind(PortBinding(cloneParent)) clonePorts.setAllParents(Some(cloneParent)) cloneParent._portsRecord = clonePorts @@ -691,35 +727,8 @@ package experimental { * io, then do operations on it. This binds a Chisel type in-place (mutably) as an IO. */ protected def _bindIoInPlace(iodef: Data): Unit = { - // Compatibility code: Chisel2 did not require explicit direction on nodes - // (unspecified treated as output, and flip on nothing was input). - // However, we are going to go back to Chisel2 semantics, so we need to make it work - // even for chisel3 code. - // This assigns the explicit directions required by both semantics on all Bundles. - // This recursively walks the tree, and assigns directions if no explicit - // direction given by upper-levels (override Input / Output) - def assignCompatDir(data: Data): Unit = { - data match { - case data: Element => data._assignCompatibilityExplicitDirection - case data: Aggregate => - data.specifiedDirection match { - // Recurse into children to ensure explicit direction set somewhere - case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip => - data match { - case record: Record => - record.elementsIterator.foreach(assignCompatDir(_)) - case vec: Vec[_] => - vec.elementsIterator.foreach(assignCompatDir(_)) - } - case SpecifiedDirection.Input | SpecifiedDirection.Output => - // forced assign, nothing to do - // Note this is because Input and Output recurse down their types to align all fields to that SpecifiedDirection - // Thus, no implicit assigment is necessary. - } - } - } - - assignCompatDir(iodef) + // Assign any signals (Chisel or chisel3) with Unspecified/Flipped directions to Output/Input + Module.assignCompatDir(iodef) iodef.bind(PortBinding(this)) _ports += iodef -- cgit v1.2.3 From c51fcfea32b6c73e623657442460fb782ff0733b Mon Sep 17 00:00:00 2001 From: Aditya Naik Date: Thu, 10 Nov 2022 13:41:00 -0800 Subject: Warn on S-interpolator usage for assert, assume and printf (backport #2751) (#2757) * Add internal methods to maintain binary compatibility Co-authored-by: Megan Wachs Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Printf.scala | 52 ++++++++++++++++++-- .../main/scala/chisel3/VerificationStatement.scala | 56 ++++++++++++++++++++-- 2 files changed, 101 insertions(+), 7 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/Printf.scala b/core/src/main/scala/chisel3/Printf.scala index 9410a409..a7338072 100644 --- a/core/src/main/scala/chisel3/Printf.scala +++ b/core/src/main/scala/chisel3/Printf.scala @@ -2,10 +2,11 @@ package chisel3 -import scala.language.experimental.macros import chisel3.internal._ import chisel3.internal.Builder.pushCommand import chisel3.internal.sourceinfo.SourceInfo +import scala.language.experimental.macros +import scala.reflect.macros.blackbox /** Prints a message in simulation * @@ -76,7 +77,44 @@ object printf { * @param data format string varargs containing data to print */ def apply(fmt: String, data: Bits*)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf = - apply(Printable.pack(fmt, data: _*)) + macro _applyMacroWithInterpolatorCheck + + def _applyMacroWithInterpolatorCheck( + c: blackbox.Context + )(fmt: c.Tree, + data: c.Tree* + )(sourceInfo: c.Tree, + compileOptions: c.Tree + ): c.Tree = { + import c.universe._ + fmt match { + case q"scala.StringContext.apply(..$_).s(..$_)" => + c.warning( + c.enclosingPosition, + "The s-interpolator prints the Scala .toString of Data objects rather than the value " + + "of the hardware wire during simulation. Use the cf-interpolator instead. If you want " + + "an elaboration time print, use println." + ) + case _ => + } + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("printfWithReset")) + q"$apply_impl_do(_root_.chisel3.Printable.pack($fmt, ..$data))($sourceInfo, $compileOptions)" + } + + // Private internal methods that serve to maintain binary + // compatibility after interpolator check updates + @deprecated("This Printf.apply method has been deprecated and will be removed in Chisel 3.6") + def apply(fmt: String, sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf = + apply(fmt, Nil, sourceInfo, compileOptions) + + @deprecated("This Printf.apply method has been deprecated and will be removed in Chisel 3.6") + def apply( + fmt: String, + data: Seq[Bits], + sourceInfo: SourceInfo, + compileOptions: CompileOptions + ): Printf = + apply(Printable.pack(fmt, data: _*))(sourceInfo, compileOptions) /** Prints a message in simulation * @@ -92,7 +130,15 @@ object printf { * @see [[Printable]] documentation * @param pable [[Printable]] to print */ - def apply(pable: Printable)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf = { + def apply(pable: Printable)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf = + printfWithReset(pable)(sourceInfo, compileOptions) + + private[chisel3] def printfWithReset( + pable: Printable + )( + implicit sourceInfo: SourceInfo, + compileOptions: CompileOptions + ): Printf = { var printfId: Printf = null when(!Module.reset.asBool) { printfId = printfWithoutReset(pable) diff --git a/core/src/main/scala/chisel3/VerificationStatement.scala b/core/src/main/scala/chisel3/VerificationStatement.scala index 10cece60..a0040d78 100644 --- a/core/src/main/scala/chisel3/VerificationStatement.scala +++ b/core/src/main/scala/chisel3/VerificationStatement.scala @@ -48,7 +48,7 @@ object assert extends VerifPrintMacrosDoc { )( implicit sourceInfo: SourceInfo, compileOptions: CompileOptions - ): Assert = macro _applyMacroWithStringMessage + ): Assert = macro _applyMacroWithInterpolatorCheck /** Checks for a condition to be valid in the circuit at all times. If the * condition evaluates to false, the circuit simulation stops with an error. @@ -79,6 +79,32 @@ object assert extends VerifPrintMacrosDoc { def apply(cond: Bool)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Assert = macro _applyMacroWithNoMessage + import VerificationStatement._ + + /** @group VerifPrintMacros */ + def _applyMacroWithInterpolatorCheck( + c: blackbox.Context + )(cond: c.Tree, + message: c.Tree, + data: c.Tree* + )(sourceInfo: c.Tree, + compileOptions: c.Tree + ): c.Tree = { + import c.universe._ + message match { + case q"scala.StringContext.apply(..$_).s(..$_)" => + c.warning( + c.enclosingPosition, + "The s-interpolator prints the Scala .toString of Data objects rather than the value " + + "of the hardware wire during simulation. Use the cf-interpolator instead. If you want " + + "an elaboration time check, call assert with a Boolean condition." + ) + case _ => + } + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable")) + q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some(_root_.chisel3.Printable.pack($message, ..$data)))($sourceInfo, $compileOptions)" + } + /** An elaboration-time assertion. Calls the built-in Scala assert function. */ def apply(cond: Boolean, message: => String): Unit = Predef.assert(cond, message) @@ -88,8 +114,6 @@ object assert extends VerifPrintMacrosDoc { /** Named class for assertions. */ final class Assert private[chisel3] () extends VerificationStatement - import VerificationStatement._ - /** @group VerifPrintMacros */ @deprecated( "This method has been deprecated in favor of _applyMacroWithStringMessage. Please use the same.", @@ -215,7 +239,7 @@ object assume extends VerifPrintMacrosDoc { )( implicit sourceInfo: SourceInfo, compileOptions: CompileOptions - ): Assume = macro _applyMacroWithStringMessage + ): Assume = macro _applyMacroWithInterpolatorCheck /** Assumes a condition to be valid in the circuit at all times. * Acts like an assertion in simulation and imposes a declarative @@ -256,6 +280,30 @@ object assume extends VerifPrintMacrosDoc { import VerificationStatement._ + /** @group VerifPrintMacros */ + def _applyMacroWithInterpolatorCheck( + c: blackbox.Context + )(cond: c.Tree, + message: c.Tree, + data: c.Tree* + )(sourceInfo: c.Tree, + compileOptions: c.Tree + ): c.Tree = { + import c.universe._ + message match { + case q"scala.StringContext.apply(..$_).s(..$_)" => + c.warning( + c.enclosingPosition, + "The s-interpolator prints the Scala .toString of Data objects rather than the value " + + "of the hardware wire during simulation. Use the cf-interpolator instead. If you want " + + "an elaboration time check, call assert with a Boolean condition." + ) + case _ => + } + val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable")) + q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some(_root_.chisel3.Printable.pack($message, ..$data)))($sourceInfo, $compileOptions)" + } + /** @group VerifPrintMacros */ @deprecated( "This method has been deprecated in favor of _applyMacroWithStringMessage. Please use the same.", -- cgit v1.2.3 From c8046636a25474be4c547c6fe9c6d742ea7b1d13 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 11 Nov 2022 01:40:53 +0000 Subject: Change RawModule._commands to a VectorBuilder (backport #2839) (#2841) * Change RawModule._commands to a VectorBuilder (#2839) * Change RawModule._commands to a VectorBuilder Use the resulting Vector to build the underlying Component's commands and then use those instead of copying the original ArrayBuffer when iterating on commands. Previously, the Component was using a List to hold the commands which is particularly memory inefficient, especially for large modules. * Optimize Converter's handling of Seq[Command] It previously converted the Commands to a List (which, while not captured in the type system, they were already a List) and then used head and tail iteration. This is less efficient with the new underlying Vector implementation. (cherry picked from commit 48a1ef0a3872c6b68d46145764d977926923a270) * Waive false binary compatibility failures Co-authored-by: Jack Koenig Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>--- core/src/main/scala/chisel3/RawModule.scala | 14 ++- .../scala/chisel3/internal/firrtl/Converter.scala | 116 ++++++++++++--------- 2 files changed, 77 insertions(+), 53 deletions(-) (limited to 'core/src/main') diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala index f2ce4c70..9668313a 100644 --- a/core/src/main/scala/chisel3/RawModule.scala +++ b/core/src/main/scala/chisel3/RawModule.scala @@ -2,7 +2,6 @@ package chisel3 -import scala.collection.mutable.{ArrayBuffer, HashMap} import scala.util.Try import scala.language.experimental.macros import scala.annotation.nowarn @@ -13,6 +12,7 @@ import chisel3.internal.Builder._ import chisel3.internal.firrtl._ import chisel3.internal.sourceinfo.UnlocatableSourceInfo import _root_.firrtl.annotations.{IsModule, ModuleTarget} +import scala.collection.immutable.VectorBuilder /** Abstract base class for Modules that contain Chisel RTL. * This abstract base class is a user-defined module which does not include implicit clock and reset and supports @@ -23,14 +23,18 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions) extends // // RTL construction internals // - private val _commands = ArrayBuffer[Command]() + // Perhaps this should be an ArrayBuffer (or ArrayBuilder), but DefModule is public and has Seq[Command] + // so our best option is to share a single Seq datastructure with that + private val _commands = new VectorBuilder[Command]() private[chisel3] def addCommand(c: Command) { require(!_closed, "Can't write to module after module close") _commands += c } - protected def getCommands = { + protected def getCommands: Seq[Command] = { require(_closed, "Can't get commands before module close") - _commands.toSeq + // Unsafe cast but we know that any RawModule uses a DefModule + // _component is defined as a var on BaseModule and we cannot override mutable vars + _component.get.asInstanceOf[DefModule].commands } // @@ -153,7 +157,7 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions) extends Seq() } } - val component = DefModule(this, name, firrtlPorts, invalidateCommands ++ getCommands) + val component = DefModule(this, name, firrtlPorts, invalidateCommands ++: _commands.result()) _component = Some(component) _component } diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index 3d6e0d79..ca39608f 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -8,7 +8,7 @@ import firrtl.{ir => fir} import chisel3.internal.{castToInt, throwException, HasId} import scala.annotation.{nowarn, tailrec} -import scala.collection.immutable.Queue +import scala.collection.immutable.{Queue, VectorBuilder} import scala.collection.immutable.LazyList // Needed for 2.12 alias @nowarn("msg=class Port") // delete when Port becomes private @@ -215,7 +215,7 @@ private[chisel3] object Converter { * @param alt Indicates if currently processing commands in the alternate (else) of the when scope */ // TODO we should probably have a different structure in the IR to close elses - private case class WhenFrame(when: fir.Conditionally, outer: Queue[fir.Statement], alt: Boolean) + private case class WhenFrame(when: fir.Conditionally, outer: VectorBuilder[fir.Statement], alt: Boolean) /** Convert Chisel IR Commands into FIRRTL Statements * @@ -226,52 +226,72 @@ private[chisel3] object Converter { * @return FIRRTL Statement that is equivalent to the input cmds */ def convert(cmds: Seq[Command], ctx: Component): fir.Statement = { - @tailrec - def rec(acc: Queue[fir.Statement], scope: List[WhenFrame])(cmds: Seq[Command]): Seq[fir.Statement] = { - if (cmds.isEmpty) { - assert(scope.isEmpty) - acc - } else - convertSimpleCommand(cmds.head, ctx) match { - // Most Commands map 1:1 - case Some(stmt) => - rec(acc :+ stmt, scope)(cmds.tail) - // When scoping logic does not map 1:1 and requires pushing/popping WhenFrames - // Please see WhenFrame for more details - case None => - cmds.head match { - case WhenBegin(info, pred) => - val when = fir.Conditionally(convert(info), convert(pred, ctx, info), fir.EmptyStmt, fir.EmptyStmt) - val frame = WhenFrame(when, acc, false) - rec(Queue.empty, frame +: scope)(cmds.tail) - case WhenEnd(info, depth, _) => - val frame = scope.head - val when = - if (frame.alt) frame.when.copy(alt = fir.Block(acc)) - else frame.when.copy(conseq = fir.Block(acc)) - // Check if this when has an else - cmds.tail.headOption match { - case Some(AltBegin(_)) => - assert(!frame.alt, "Internal Error! Unexpected when structure!") // Only 1 else per when - rec(Queue.empty, frame.copy(when = when, alt = true) +: scope.tail)(cmds.drop(2)) - case _ => // Not followed by otherwise - // If depth > 0 then we need to close multiple When scopes so we add a new WhenEnd - // If we're nested we need to add more WhenEnds to ensure each When scope gets - // properly closed - val cmdsx = if (depth > 0) WhenEnd(info, depth - 1, false) +: cmds.tail else cmds.tail - rec(frame.outer :+ when, scope.tail)(cmdsx) - } - case OtherwiseEnd(info, depth) => - val frame = scope.head - val when = frame.when.copy(alt = fir.Block(acc)) - // TODO For some reason depth == 1 indicates the last closing otherwise whereas - // depth == 0 indicates last closing when - val cmdsx = if (depth > 1) OtherwiseEnd(info, depth - 1) +: cmds.tail else cmds.tail - rec(scope.head.outer :+ when, scope.tail)(cmdsx) - } - } + var stmts = new VectorBuilder[fir.Statement]() + var scope: List[WhenFrame] = Nil + var cmdsIt = cmds.iterator.buffered + // Extra var because sometimes we want to push a Command to the head of cmdsIt + // This is more efficient than changing the iterator + var nextCmd: Command = null + while (nextCmd != null || cmdsIt.hasNext) { + val cmd = if (nextCmd != null) { + val _nextCmd = nextCmd + nextCmd = null + _nextCmd + } else { + cmdsIt.next() + } + convertSimpleCommand(cmd, ctx) match { + // Most Commands map 1:1 + case Some(stmt) => + stmts += stmt + // When scoping logic does not map 1:1 and requires pushing/popping WhenFrames + // Please see WhenFrame for more details + case None => + cmd match { + case WhenBegin(info, pred) => + val when = fir.Conditionally(convert(info), convert(pred, ctx, info), fir.EmptyStmt, fir.EmptyStmt) + val frame = WhenFrame(when, stmts, false) + stmts = new VectorBuilder[fir.Statement] + scope = frame :: scope + case WhenEnd(info, depth, _) => + val frame = scope.head + val when = + if (frame.alt) frame.when.copy(alt = fir.Block(stmts.result())) + else frame.when.copy(conseq = fir.Block(stmts.result())) + // Check if this when has an else + cmdsIt.headOption match { + case Some(AltBegin(_)) => + assert(!frame.alt, "Internal Error! Unexpected when structure!") // Only 1 else per when + scope = frame.copy(when = when, alt = true) :: scope.tail + cmdsIt.next() // Consume the AltBegin + stmts = new VectorBuilder[fir.Statement] + case _ => // Not followed by otherwise + // If depth > 0 then we need to close multiple When scopes so we add a new WhenEnd + // If we're nested we need to add more WhenEnds to ensure each When scope gets + // properly closed + if (depth > 0) { + nextCmd = WhenEnd(info, depth - 1, false) + } + stmts = frame.outer + stmts += when + scope = scope.tail + } + case OtherwiseEnd(info, depth) => + val frame = scope.head + val when = frame.when.copy(alt = fir.Block(stmts.result())) + // TODO For some reason depth == 1 indicates the last closing otherwise whereas + // depth == 0 indicates last closing when + if (depth > 1) { + nextCmd = OtherwiseEnd(info, depth - 1) + } + stmts = frame.outer + stmts += when + scope = scope.tail + } + } } - fir.Block(rec(Queue.empty, List.empty)(cmds)) + assert(scope.isEmpty) + fir.Block(stmts.result()) } def convert(width: Width): fir.Width = width match { @@ -347,7 +367,7 @@ private[chisel3] object Converter { def convert(component: Component): fir.DefModule = component match { case ctx @ DefModule(_, name, ports, cmds) => - fir.Module(fir.NoInfo, name, ports.map(p => convert(p)), convert(cmds.toList, ctx)) + fir.Module(fir.NoInfo, name, ports.map(p => convert(p)), convert(cmds, ctx)) case ctx @ DefBlackBox(id, name, ports, topDir, params) => fir.ExtModule( fir.NoInfo, -- cgit v1.2.3