From 1ee9adbec48bc8393e1c3d0ed86a548f8510d13f Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 12 May 2022 17:31:59 +0000 Subject: Support separately elaborating definition and instance in ChiselStage (backport #2512) (#2520) * Support separately elaborating definition and instance in ChiselStage (#2512) (cherry picked from commit a0aa4d1550e3fbde199a98529cffeb176fb4bed8) # Conflicts: # core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala # core/src/main/scala/chisel3/experimental/hierarchy/Instance.scala # core/src/main/scala/chisel3/internal/Builder.scala * fixing imports (#2522) Co-authored-by: Deborah Soung --- .../experimental/hierarchy/Definition.scala | 7 ++++++ .../chisel3/experimental/hierarchy/Instance.scala | 29 ++++++++++++++++++++-- core/src/main/scala/chisel3/internal/Builder.scala | 19 +++++++++++++- 3 files changed, 52 insertions(+), 3 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala b/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala index 59b4c692..a8cfac2f 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala @@ -11,6 +11,7 @@ import chisel3.internal.sourceinfo.{DefinitionTransform, DefinitionWrapTransform import chisel3.experimental.BaseModule import chisel3.internal.BaseModule.IsClone import firrtl.annotations.{IsModule, ModuleTarget} +import firrtl.annotations.{IsModule, ModuleTarget, NoTargetAnnotation} /** User-facing Definition type. * Represents a definition of an object of type [[A]] which are marked as @instantiable @@ -112,3 +113,9 @@ object Definition extends SourceInfoDoc { } } + +/** Stores a [[Definition]] that is imported so that its Instances can be + * compiled separately. + */ +case class ImportDefinitionAnnotation[T <: BaseModule with IsInstantiable](definition: Definition[T]) + extends NoTargetAnnotation diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/Instance.scala b/core/src/main/scala/chisel3/experimental/hierarchy/Instance.scala index cc926771..9f96dff1 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/Instance.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/Instance.scala @@ -2,12 +2,13 @@ package chisel3.experimental.hierarchy -import scala.collection.mutable.{ArrayBuffer, HashMap} import scala.language.experimental.macros import chisel3._ import chisel3.internal.BaseModule.{InstantiableClone, IsClone, ModuleClone} +import chisel3.internal.Builder import chisel3.internal.sourceinfo.{InstanceTransform, SourceInfo} -import chisel3.experimental.BaseModule +import chisel3.experimental.{BaseModule, ExtModule} +import chisel3.internal.firrtl.{Component, DefBlackBox, DefModule, Port} import firrtl.annotations.IsModule /** User-facing Instance type. @@ -107,9 +108,33 @@ object Instance extends SourceInfoDoc { implicit sourceInfo: SourceInfo, compileOptions: CompileOptions ): Instance[T] = { + // Check to see if the module is already defined internally or externally + val existingMod = Builder.components.map { + case c: DefModule if c.id == definition.proto => Some(c) + case c: DefBlackBox if c.name == definition.proto.name => Some(c) + case _ => None + }.flatten + + if (existingMod.isEmpty) { + // Add a Definition that will get emitted as an ExtModule so that FIRRTL + // does not complain about a missing element + class EmptyExtModule extends ExtModule { + override def desiredName: String = definition.proto.name + override def generateComponent(): Option[Component] = { + require(!_closed, s"Can't generate $desiredName module more than once") + _closed = true + val firrtlPorts = definition.proto.getModulePorts.map { port => Port(port, port.specifiedDirection) } + val component = DefBlackBox(this, definition.proto.name, firrtlPorts, SpecifiedDirection.Unspecified, params) + Some(component) + } + } + Definition(new EmptyExtModule() {}) + } + val ports = experimental.CloneModuleAsRecord(definition.proto) val clone = ports._parent.get.asInstanceOf[ModuleClone[T]] clone._madeFromDefinition = true + new Instance(Clone(clone)) } diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 4180f580..69455455 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -6,7 +6,7 @@ import scala.util.DynamicVariable import scala.collection.mutable.ArrayBuffer import chisel3._ import chisel3.experimental._ -import chisel3.experimental.hierarchy.{Clone, Instance} +import chisel3.experimental.hierarchy.{Clone, ImportDefinitionAnnotation, Instance} import chisel3.internal.firrtl._ import chisel3.internal.naming._ import _root_.firrtl.annotations.{CircuitName, ComponentName, IsMember, ModuleName, Named, ReferenceTarget} @@ -364,7 +364,24 @@ private[chisel3] class ChiselContext() { } private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) { + val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a } + + // Ensure there are no repeated names for imported Definitions + val importDefinitionNames = importDefinitionAnnos.map { a => a.definition.proto.name } + if (importDefinitionNames.distinct.length < importDefinitionNames.length) { + val duplicates = importDefinitionNames.diff(importDefinitionNames.distinct).mkString(", ") + throwException(s"Expected distinct imported Definition names but found duplicates for: $duplicates") + } + val globalNamespace = Namespace.empty + + // Ensure imported Definitions emit as ExtModules with the correct name so + // that instantiations will also use the correct name and prevent any name + // conflicts with Modules/Definitions in this elaboration + importDefinitionNames.foreach { importDefName => + globalNamespace.name(importDefName) + } + val components = ArrayBuffer[Component]() val annotations = ArrayBuffer[ChiselAnnotation]() var currentModule: Option[BaseModule] = None -- cgit v1.2.3 From 7825b432ece7abee9c955f2429046d1c48222437 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 19 May 2022 18:32:30 +0000 Subject: Support := views to DontCare (#2536) (#2539) (cherry picked from commit 77a6c93592d5766d66f199720fc6d69478005091) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/MonoConnect.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/internal/MonoConnect.scala b/core/src/main/scala/chisel3/internal/MonoConnect.scala index 40056c89..31364804 100644 --- a/core/src/main/scala/chisel3/internal/MonoConnect.scala +++ b/core/src/main/scala/chisel3/internal/MonoConnect.scala @@ -210,7 +210,9 @@ private[chisel3] object MonoConnect { } // Source is DontCare - it may be connected to anything. It generates a defInvalid for the sink. - case (sink, DontCare) => pushCommand(DefInvalid(sourceInfo, sink.lref)) + case (_sink: Element, DontCare) => + val sink = reify(_sink) // Handle views + pushCommand(DefInvalid(sourceInfo, sink.lref)) // DontCare as a sink is illegal. case (DontCare, _) => throw DontCareCantBeSink // Analog is illegal in mono connections. -- cgit v1.2.3 From 2453ac10fae363455398dd1ef5bcdb79e6d23f27 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 24 May 2022 22:02:52 +0000 Subject: Support Vecs of empty Bundles (#2543) (#2545) (cherry picked from commit a1e3a6b5324997864168111bee8c02a60abb0acc) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Aggregate.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index cc5b83d9..82c02cbc 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -226,8 +226,8 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend } // Since all children are the same, we can just use the sample_element rather than all children - // .get is safe because None means mixed directions, we only pass 1 so that's not possible - direction = ActualDirection.fromChildren(Set(sample_element.direction), resolvedDirection).get + direction = + ActualDirection.fromChildren(Set(sample_element.direction), resolvedDirection).getOrElse(ActualDirection.Empty) } // Note: the constructor takes a gen() function instead of a Seq to enforce @@ -321,6 +321,7 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend case ActualDirection.Bidirectional(ActualDirection.Default) | ActualDirection.Unspecified => SpecifiedDirection.Unspecified case ActualDirection.Bidirectional(ActualDirection.Flipped) => SpecifiedDirection.Flip + case ActualDirection.Empty => SpecifiedDirection.Unspecified } // TODO port technically isn't directly child of this data structure, but the result of some // muxes / demuxes. However, this does make access consistent with the top-level bindings. -- cgit v1.2.3 From 3aed65709aedc22810926751db33fe9ba767a03b Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 27 May 2022 22:06:36 +0000 Subject: Make ExtModule port naming consistent with Module (#2548) (#2549) ExtModule now uses the same namePorts implementation as regular Modules. Previously, ExtModules only allowed port naming via runtime reflection. This meant that .suggestName and other naming APIs do not work. It also breaks FlatIO for ExtModule which is a potential replacement API for BlackBox's special `val io` handling. (cherry picked from commit 83cccfb782d9141bf2c843246c2a525c62392924) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/BlackBox.scala | 7 ++----- core/src/main/scala/chisel3/Module.scala | 21 +++++++++++++++++++++ core/src/main/scala/chisel3/RawModule.scala | 21 --------------------- 3 files changed, 23 insertions(+), 26 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala index f3fc2711..f618901f 100644 --- a/core/src/main/scala/chisel3/BlackBox.scala +++ b/core/src/main/scala/chisel3/BlackBox.scala @@ -71,11 +71,8 @@ package experimental { val names = nameIds(classOf[ExtModule]) - // Name ports based on reflection - for (port <- getModulePorts) { - require(names.contains(port), s"Unable to name port $port in $this") - port.setRef(ModuleIO(this, _namespace.name(names(port)))) - } + // Ports are named in the same way as regular Modules + namePorts(names) // All suggestions are in, force names to every node. // While BlackBoxes are not supposed to have an implementation, we still need to call diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 84139630..3382cd1b 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -486,6 +486,27 @@ package experimental { */ private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit + private[chisel3] def namePorts(names: HashMap[HasId, String]): Unit = { + for (port <- getModulePorts) { + port._computeName(None, None).orElse(names.get(port)) match { + case Some(name) => + if (_namespace.contains(name)) { + Builder.error( + s"""Unable to name port $port to "$name" in $this,""" + + " name is already taken by another port!" + ) + } + port.setRef(ModuleIO(this, _namespace.name(name))) + case None => + Builder.error( + s"Unable to name port $port in $this, " + + "try making it a public field of the Module" + ) + port.setRef(ModuleIO(this, "")) + } + } + } + // // Chisel Internals // diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala index 12b6a76a..c257f0c6 100644 --- a/core/src/main/scala/chisel3/RawModule.scala +++ b/core/src/main/scala/chisel3/RawModule.scala @@ -43,27 +43,6 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions) extends val compileOptions = moduleCompileOptions - private[chisel3] def namePorts(names: HashMap[HasId, String]): Unit = { - for (port <- getModulePorts) { - port._computeName(None, None).orElse(names.get(port)) match { - case Some(name) => - if (_namespace.contains(name)) { - Builder.error( - s"""Unable to name port $port to "$name" in $this,""" + - " name is already taken by another port!" - ) - } - port.setRef(ModuleIO(this, _namespace.name(name))) - case None => - Builder.error( - s"Unable to name port $port in $this, " + - "try making it a public field of the Module" - ) - port.setRef(ModuleIO(this, "")) - } - } - } - private[chisel3] override def generateComponent(): Option[Component] = { require(!_closed, "Can't generate module more than once") _closed = true -- cgit v1.2.3 From 0c811b490f47f20f2e81c58706924e56611b6ba2 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Sun, 29 May 2022 22:07:56 +0000 Subject: Deprecate accessing the name of non-hardware Data (#2550) (#2552) This includes (and is tested) for both the old .*Name APIs and .toTarget (cherry picked from commit 6e0d8d6b12e9d8f94c2cc43b92b2366ec70dfd50) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 69455455..97c3bc49 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -240,7 +240,18 @@ private[chisel3] trait HasId extends InstanceId { private def refName(c: Component): String = _ref match { case Some(arg) => arg.fullName(c) - case None => _computeName(None, None).get + case None => + // This is super hacky but this is just for a short term deprecation + // 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 + 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." + errors.deprecated(msg, None) + errors.checkpoint(logger) + _computeName(None, None).get } // Helper for reifying views if they map to a single Target -- cgit v1.2.3 From 97fde23f666a560d4eba9333e4230f901d7f5361 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 1 Jun 2022 20:32:31 +0000 Subject: Add formatted Printable interpolator `cf` (#2528) (#2553) This is a formatted version of the p"..." interpolator analogous to Scala's f"..." interpolator. The primary difference is that it supports formatting interpolated variables by following the variable with "%". For example: printf(cf"myWire = $myWire%x\n") This will format the hardware value "myWire" as a hexidecimal value in the emitted Verilog. Note that literal "%" must be escaped as "%%". Scala types and format specifiers are supported and are handled in the same manner as in standard Scala f"..." interpolators. (cherry picked from commit 037f7b2ff3a46184d1b82e1b590a7572bfa6a76b) Co-authored-by: Girish Pai --- core/src/main/scala/chisel3/Printable.scala | 77 ++++++++------ core/src/main/scala/chisel3/package.scala | 153 ++++++++++++++++++++++++---- 2 files changed, 182 insertions(+), 48 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/Printable.scala b/core/src/main/scala/chisel3/Printable.scala index a616f2b0..78655517 100644 --- a/core/src/main/scala/chisel3/Printable.scala +++ b/core/src/main/scala/chisel3/Printable.scala @@ -63,57 +63,76 @@ object Printable { */ def pack(fmt: String, data: Data*): Printable = { val args = data.toIterator - // Error handling def carrotAt(index: Int) = (" " * index) + "^" def errorMsg(index: Int) = s"""| fmt = "$fmt" | ${carrotAt(index)} | data = ${data.mkString(", ")}""".stripMargin - def getArg(i: Int): Data = { + + def checkArg(i: Int): Unit = { if (!args.hasNext) { val msg = "has no matching argument!\n" + errorMsg(i) // Exception wraps msg in s"Format Specifier '$msg'" throw new MissingFormatArgumentException(msg) } - args.next() + val _ = args.next() } + var iter = 0 + var curr_start = 0 + val buf = mutable.ListBuffer.empty[String] + while (iter < fmt.size) { + // Encountered % which is either + // 1. Describing a format specifier. + // 2. Literal Percent + // 3. Dangling percent - most likely due to a typo - intended literal percent or forgot the specifier. + // Try to give meaningful error reports + if (fmt(iter) == '%') { + if (iter != fmt.size - 1 && (fmt(iter + 1) != '%' && !fmt(iter + 1).isWhitespace)) { + checkArg(iter) + buf += fmt.substring(curr_start, iter) + curr_start = iter + iter += 1 + } - val pables = mutable.ListBuffer.empty[Printable] - var str = "" - var percent = false - for ((c, i) <- fmt.zipWithIndex) { - if (percent) { - val arg = c match { - case FirrtlFormat(x) => FirrtlFormat(x.toString, getArg(i)) - case 'n' => Name(getArg(i)) - case 'N' => FullName(getArg(i)) - case '%' => Percent - case x => - val msg = s"Illegal format specifier '$x'!\n" + errorMsg(i) - throw new UnknownFormatConversionException(msg) + // Last character is %. + else if (iter == fmt.size - 1) { + val msg = s"Trailing %\n" + errorMsg(fmt.size - 1) + throw new UnknownFormatConversionException(msg) + } + + // A lone % + else if (fmt(iter + 1).isWhitespace) { + val msg = s"Unescaped % - add % if literal or add proper specifier if not\n" + errorMsg(iter + 1) + throw new UnknownFormatConversionException(msg) + } + + // A literal percent - hence increment by 2. + else { + iter += 2 } - pables += PString(str.dropRight(1)) // remove format % - pables += arg - str = "" - percent = false - } else { - str += c - percent = c == '%' } - } - if (percent) { - val msg = s"Trailing %\n" + errorMsg(fmt.size - 1) - throw new UnknownFormatConversionException(msg) + + // Normal progression + else { + iter += 1 + } } require( !args.hasNext, s"Too many arguments! More format specifier(s) expected!\n" + errorMsg(fmt.size) ) + buf += fmt.substring(curr_start, iter) + + // The string received as an input to pack is already + // treated i.e. escape sequences are processed. + // Since StringContext API assumes the parts are un-treated + // treatEscapes is called within the implemented custom interpolators. + // The literal \ needs to be escaped before sending to the custom cf interpolator. - pables += PString(str) - Printables(pables) + val bufEscapeBackSlash = buf.map(_.replace("\\", "\\\\")) + StringContext(bufEscapeBackSlash.toSeq: _*).cf(data: _*) } } diff --git a/core/src/main/scala/chisel3/package.scala b/core/src/main/scala/chisel3/package.scala index bd088e21..5521c51e 100644 --- a/core/src/main/scala/chisel3/package.scala +++ b/core/src/main/scala/chisel3/package.scala @@ -1,6 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 import chisel3.internal.firrtl.BinaryPoint +import java.util.{MissingFormatArgumentException, UnknownFormatConversionException} +import scala.collection.mutable /** This package contains the main chisel3 API. */ @@ -210,29 +212,142 @@ package object chisel3 { implicit class PrintableHelper(val sc: StringContext) extends AnyVal { /** Custom string interpolator for generating Printables: p"..." - * Will call .toString on any non-Printable arguments (mimicking s"...") + * mimicks s"..." for non-Printable data) */ def p(args: Any*): Printable = { - sc.checkLengths(args) // Enforce sc.parts.size == pargs.size + 1 - val pargs: Seq[Option[Printable]] = args.map { - case p: Printable => Some(p) - case d: Data => Some(d.toPrintable) - case any => - for { - v <- Option(any) // Handle null inputs - str = v.toString - if !str.isEmpty // Handle empty Strings - } yield PString(str) + // P interpolator does not treat % differently - hence need to add % before sending to cf. + val t = sc.parts.map(_.replaceAll("%", "%%")) + StringContext(t: _*).cf(args: _*) + } + + /** Custom string interpolator for generating formatted Printables : cf"..." + * + * Enhanced version of scala's `f` interpolator. + * Each expression (argument) referenced within the string is + * converted to a particular Printable depending + * on the format specifier and type. + * + * ==== For Chisel types referenced within the String ==== + * + * - %n - Returns [[Name]] Printable. + * - %N - Returns [[FullName]] Printable. + * - %b,%d,%x,%c - Only applicable for types of [[Bits]] or dreived from it. - returns ([[Binary]],[[Decimal]], + * [[Hexadecimal]],[[Character]]) Printable respectively. + * - Default - If no specifier given call [[Data.toPrintable]] on the Chisel Type. + * + * ==== For [[Printable]] type: ==== + * No explicit format specifier supported - just return the Printable. + * + * ==== For regular scala types ==== + * Call String.format with the argument and specifier. + * Default is %s if no specifier is given. + * Wrap the result in [[PString]] Printable. + * + * ==== For the parts of the StringContext ==== + * Remove format specifiers and if literal percents (need to be escaped with %) + * are present convert them into [[Percent]] Printable. + * Rest of the string will be wrapped in [[PString]] Printable. + * + * @example + * {{{ + * + * val w1 = 20.U // Chisel UInt type (which extends Bits) + * val f1 = 30.2 // Scala float type. + * val pable = cf"w1 = $w1%x f1 = $f1%2.2f. This is 100%% clear" + * + * // pable is as follows + * // Printables(List(PString(w1 = ), Hexadecimal(UInt<5>(20)), PString( f1 = ), PString(30.20), PString(. This is 100), Percent, PString( clear))) + * }}} + * + * @throws UnknownFormatConversionException + * if literal percent not escaped with % or if the format specifier is not supported + * for the specific type + * + * @throws StringContext.InvalidEscapeException + * if a `parts` string contains a backslash (`\`) character + * that does not start a valid escape sequence. + * + * @throws IllegalArgumentException + * if the number of `parts` in the enclosing `StringContext` does not exceed + * the number of arguments `arg` by exactly 1. + */ + def cf(args: Any*): Printable = { + + // Handle literal % + // Takes the part string - + // - this is assumed to not have any format specifiers - already handled / removed before calling this function. + // Only thing present is literal % if any which should ideally be with %%. + // If not - then flag an error. + // Return seq of Printables (either PString or Percent or both - nothing else + def percentSplitter(s: String): Seq[Printable] = { + if (s.isEmpty) Seq(PString("")) + else { + val pieces = s.split("%%").toList.flatMap { p => + if (p.contains('%')) throw new UnknownFormatConversionException("Un-escaped % found") + // Wrap in PString and intersperse the escaped percentages + Seq(Percent, PString(p)) + } + if (pieces.isEmpty) Seq(Percent) + else pieces.tail // Don't forget to drop the extra percent we put at the beginning + } } + + def extractFormatSpecifier(part: String): (Option[String], String) = { + // Check if part starts with a format specifier (with % - disambiguate with literal % checking the next character if needed to be %) + // In the case of %f specifier there is a chance that we need more information - so capture till the 1st letter (a-zA-Z). + // Example cf"This is $val%2.2f here" - parts - Seq("This is ","%2.2f here") - the format specifier here is %2.2f. + val endFmtIdx = + if (part.length > 1 && part(0) == '%' && part(1) != '%') part.indexWhere(_.isLetter) + else -1 + val (fmt, rest) = part.splitAt(endFmtIdx + 1) + + val fmtOpt = if (fmt.nonEmpty) Some(fmt) else None + (fmtOpt, rest) + + } + + sc.checkLengths(args) // Enforce sc.parts.size == pargs.size + 1 val parts = sc.parts.map(StringContext.treatEscapes) - // Zip sc.parts and pargs together ito flat Seq - // eg. Seq(sc.parts(0), pargs(0), sc.parts(1), pargs(1), ...) - val seq = for { // append None because sc.parts.size == pargs.size + 1 - (literal, arg) <- parts.zip(pargs :+ None) - optPable <- Seq(Some(PString(literal)), arg) - pable <- optPable // Remove Option[_] - } yield pable - Printables(seq) + // The 1st part is assumed never to contain a format specifier. + // If the 1st part of a string is an argument - then the 1st part will be an empty String. + // So we need to parse parts following the 1st one to get the format specifiers if any + val partsAfterFirst = parts.tail + + // Align parts to their potential specifiers + val pables = partsAfterFirst.zip(args).flatMap { + case (part, arg) => { + val (fmt, modP) = extractFormatSpecifier(part) + val fmtArg: Printable = arg match { + case d: Data => { + fmt match { + case Some("%n") => Name(d) + case Some("%N") => FullName(d) + case Some(fForm) if d.isInstanceOf[Bits] => FirrtlFormat(fForm.substring(1, 2), d) + case Some(x) => { + val msg = s"Illegal format specifier '$x' for Chisel Data type!\n" + throw new UnknownFormatConversionException(msg) + } + case None => d.toPrintable + } + } + case p: Printable => { + fmt match { + case Some(x) => { + val msg = s"Illegal format specifier '$x' for Chisel Printable type!\n" + throw new UnknownFormatConversionException(msg) + } + case None => p + } + } + + // Generic case - use String.format (for example %d,%2.2f etc on regular Scala types) + case t => PString(fmt.getOrElse("%s").format(t)) + + } + Seq(fmtArg) ++ percentSplitter(modP) + } + } + Printables(percentSplitter(parts.head) ++ pables) } } -- cgit v1.2.3 From 63f25ac4b4d7c9bd530ff1875ad38d835a82e051 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 3 Jun 2022 18:33:13 +0000 Subject: Deprecate implicit .U() and .S() syntax for literal bit extracts (backport #2534) (#2559) * Deprecate .U() and .S() syntax for literal bit extracts (#2534) (cherry picked from commit cadaf33a650ef898fdab2f81244e4ad6a07a9ea8) # Conflicts: # macros/src/main/scala/chisel3/internal/sourceinfo/SourceInfoTransform.scala * Fix backport conflict (#2560) Co-authored-by: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com>--- core/src/main/scala/chisel3/Bits.scala | 18 ++++++++++++------ core/src/main/scala/chisel3/package.scala | 12 ++++++------ 2 files changed, 18 insertions(+), 12 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/Bits.scala b/core/src/main/scala/chisel3/Bits.scala index a135a8e5..c914e88c 100644 --- a/core/src/main/scala/chisel3/Bits.scala +++ b/core/src/main/scala/chisel3/Bits.scala @@ -8,7 +8,13 @@ import chisel3.experimental.{FixedPoint, Interval} import chisel3.internal._ import chisel3.internal.Builder.pushOp import chisel3.internal.firrtl._ -import chisel3.internal.sourceinfo.{SourceInfo, SourceInfoTransform, SourceInfoWhiteboxTransform, UIntTransform} +import chisel3.internal.sourceinfo.{ + IntLiteralApplyTransform, + SourceInfo, + SourceInfoTransform, + SourceInfoWhiteboxTransform, + UIntTransform +} import chisel3.internal.firrtl.PrimOp._ import _root_.firrtl.{ir => firrtlir} import _root_.firrtl.{constraint => firrtlconstraint} @@ -94,7 +100,7 @@ sealed abstract class Bits(private[chisel3] val width: Width) extends Element wi * @param x an index * @return the specified bit */ - final def apply(x: BigInt): Bool = macro SourceInfoTransform.xArg + final def apply(x: BigInt): Bool = macro IntLiteralApplyTransform.safeApply /** @group SourceInfoTransformMacro */ final def do_apply(x: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Bool = { @@ -121,12 +127,12 @@ sealed abstract class Bits(private[chisel3] val width: Width) extends Element wi * * @param x an index * @return the specified bit - * @note convenience method allowing direct use of [[scala.Int]] without implicits */ - final def apply(x: Int): Bool = macro SourceInfoTransform.xArg + final def apply(x: Int): Bool = macro IntLiteralApplyTransform.safeApply /** @group SourceInfoTransformMacro */ - final def do_apply(x: Int)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Bool = apply(BigInt(x)) + final def do_apply(x: Int)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Bool = + do_apply(BigInt(x)) /** Returns the specified bit on this wire as a [[Bool]], dynamically addressed. * @@ -193,7 +199,7 @@ sealed abstract class Bits(private[chisel3] val width: Width) extends Element wi /** @group SourceInfoTransformMacro */ final def do_apply(x: BigInt, y: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt = - apply(castToInt(x, "High index"), castToInt(y, "Low index")) + do_apply(castToInt(x, "High index"), castToInt(y, "Low index")) private[chisel3] def unop[T <: Data](sourceInfo: SourceInfo, dest: T, op: PrimOp): T = { requireIsHardware(this, "bits operated on") diff --git a/core/src/main/scala/chisel3/package.scala b/core/src/main/scala/chisel3/package.scala index 5521c51e..87024683 100644 --- a/core/src/main/scala/chisel3/package.scala +++ b/core/src/main/scala/chisel3/package.scala @@ -7,6 +7,8 @@ import scala.collection.mutable /** This package contains the main chisel3 API. */ package object chisel3 { + import internal.chiselRuntimeDeprecated + import internal.sourceinfo.DeprecatedSourceInfo import internal.firrtl.{Port, Width} import internal.Builder @@ -39,13 +41,11 @@ package object chisel3 { case bigint => Builder.error(s"Cannot convert $bigint to Bool, must be 0 or 1"); Bool.Lit(false) } - /** Int to UInt conversion, recommended style for constants. - */ - def U: UInt = UInt.Lit(bigint, Width()) + /** Int to UInt conversion, recommended style for constants. */ + def U: UInt = UInt.Lit(bigint, Width()) // scalastyle:ignore method.name - /** Int to SInt conversion, recommended style for constants. - */ - def S: SInt = SInt.Lit(bigint, Width()) + /** Int to SInt conversion, recommended style for constants. */ + def S: SInt = SInt.Lit(bigint, Width()) // scalastyle:ignore method.name /** Int to UInt conversion with specified width, recommended style for constants. */ -- cgit v1.2.3 From 42f5d89045e7db323670964a982c59319cf9001f Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 6 Jun 2022 23:02:01 +0000 Subject: Add --warn:reflective-naming (backport #2561) (#2565) * Factor buildName into reusable function The new function is chisel3.internal.buildName. (cherry picked from commit 370ca8ac68f6d888dd99e1b9e63f0371add398cf) * Add --warn:reflective-naming This new argument (and associated annotation) will turn on a warning whenever reflective naming changes the name of a signal. This is provided to help migrate from Chisel 3.5 to 3.6 since reflective naming is removed in Chisel 3.6. (cherry picked from commit 97afd9b9a1155fa7cd5cedf19f9e0c15fbe899ec) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/RawModule.scala | 48 +++++++++++++++++++++- .../experimental/hierarchy/Definition.scala | 5 ++- core/src/main/scala/chisel3/internal/Builder.scala | 39 +++++++++--------- core/src/main/scala/chisel3/internal/Error.scala | 5 +++ 4 files changed, 76 insertions(+), 21 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala index c257f0c6..164d3880 100644 --- a/core/src/main/scala/chisel3/RawModule.scala +++ b/core/src/main/scala/chisel3/RawModule.scala @@ -43,6 +43,22 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions) extends val compileOptions = moduleCompileOptions + // This could be factored into a common utility + private def canBeNamed(id: HasId): Boolean = id match { + case d: Data => + d.binding match { + case Some(_: ConstrainedBinding) => true + case _ => false + } + case b: BaseModule => true + case m: MemBase[_] => true + // These names don't affect hardware + case _: VerificationStatement => false + // While the above should be comprehensive, since this is used in warning we want to be careful + // to never accidentally have a match error + case _ => false + } + private[chisel3] override def generateComponent(): Option[Component] = { require(!_closed, "Can't generate module more than once") _closed = true @@ -53,8 +69,24 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions) extends namePorts(names) // Then everything else gets named + val warnReflectiveNaming = Builder.warnReflectiveNaming for ((node, name) <- names) { - node.suggestName(name) + node match { + case d: HasId if warnReflectiveNaming && canBeNamed(d) => + val result = d._suggestNameCheck(name) + result match { + case None => // All good, no warning + case Some((oldName, oldPrefix)) => + val prevName = buildName(oldName, oldPrefix.reverse) + val newName = buildName(name, Nil) + val msg = s"[module ${this.name}] '$prevName' is renamed by reflection to '$newName'. " + + s"Chisel 3.6 removes reflective naming so the name will remain '$prevName'." + Builder.warningNoLoc(msg) + } + // Note that unnamable things end up here (eg. literals), this is supporting backwards + // compatibility + case _ => node.suggestName(name) + } } // All suggestions are in, force names to every node. @@ -154,6 +186,20 @@ package object internal { /** Marker trait for modules that are not true modules */ private[chisel3] trait PseudoModule extends BaseModule + /** Creates a name String from a prefix and a seed + * @param prefix The prefix associated with the seed (must be in correct order, *not* reversed) + * @param seed The seed for computing the name (if available) + */ + def buildName(seed: String, prefix: Prefix): String = { + val builder = new StringBuilder() + prefix.foreach { p => + builder ++= p + builder += '_' + } + builder ++= seed + builder.toString + } + // Private reflective version of "val io" to maintain Chisel.Module semantics without having // io as a virtual method. See https://github.com/freechipsproject/chisel3/pull/1550 for more // information about the removal of "val io" diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala b/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala index a8cfac2f..c79c26dc 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala @@ -101,7 +101,10 @@ object Definition extends SourceInfoDoc { implicit sourceInfo: SourceInfo, compileOptions: CompileOptions ): Definition[T] = { - val dynamicContext = new DynamicContext(Nil, Builder.captureContext().throwOnFirstError) + val dynamicContext = { + val context = Builder.captureContext() + new DynamicContext(Nil, context.throwOnFirstError, context.warnReflectiveNaming) + } Builder.globalNamespace.copyTo(dynamicContext.globalNamespace) dynamicContext.inDefinition = true val (ir, module) = Builder.build(Module(proto), dynamicContext, false) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 97c3bc49..6f02b57c 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -137,6 +137,17 @@ private[chisel3] trait HasId extends InstanceId { this } + // Private internal version of suggestName that tells you if the name changed + // Returns Some(old name, old prefix) if name changed, None otherwise + private[chisel3] def _suggestNameCheck(seed: => String): Option[(String, Prefix)] = { + val oldSeed = this.seedOpt + val oldPrefix = this.naming_prefix + suggestName(seed) + if (oldSeed.nonEmpty && (oldSeed != this.seedOpt || oldPrefix != this.naming_prefix)) { + Some(oldSeed.get -> oldPrefix) + } else None + } + /** Takes the first seed suggested. Multiple calls to this function will be ignored. * If the final computed name conflicts with another name, it may get uniquified by appending * a digit at the end. @@ -171,22 +182,6 @@ private[chisel3] trait HasId extends InstanceId { * @return the name, if it can be computed */ private[chisel3] def _computeName(defaultPrefix: Option[String], defaultSeed: Option[String]): Option[String] = { - - /** Computes a name of this signal, given the seed and prefix - * @param seed - * @param prefix - * @return - */ - def buildName(seed: String, prefix: Prefix): String = { - val builder = new StringBuilder() - prefix.foreach { p => - builder ++= p - builder += '_' - } - builder ++= seed - builder.toString - } - if (hasSeed) { Some(buildName(seedOpt.get, naming_prefix.reverse)) } else { @@ -374,7 +369,10 @@ private[chisel3] class ChiselContext() { val viewNamespace = Namespace.empty } -private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) { +private[chisel3] class DynamicContext( + val annotationSeq: AnnotationSeq, + val throwOnFirstError: Boolean, + val warnReflectiveNaming: Boolean) { val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a } // Ensure there are no repeated names for imported Definitions @@ -649,6 +647,8 @@ private[chisel3] object Builder extends LazyLogging { throwException("Error: No implicit reset.") ) + def warnReflectiveNaming: Boolean = dynamicContext.warnReflectiveNaming + // TODO(twigg): Ideally, binding checks and new bindings would all occur here // However, rest of frontend can't support this yet. def pushCommand[T <: Command](c: T): T = { @@ -690,8 +690,9 @@ private[chisel3] object Builder extends LazyLogging { throwException(m) } } - def warning(m: => String): Unit = if (dynamicContextVar.value.isDefined) errors.warning(m) - def deprecated(m: => String, location: Option[String] = None): Unit = + def warning(m: => String): Unit = if (dynamicContextVar.value.isDefined) errors.warning(m) + def warningNoLoc(m: => String): Unit = if (dynamicContextVar.value.isDefined) errors.warningNoLoc(m) + def deprecated(m: => String, location: Option[String] = None): Unit = if (dynamicContextVar.value.isDefined) errors.deprecated(m, location) /** Record an exception as an error, and throw it. diff --git a/core/src/main/scala/chisel3/internal/Error.scala b/core/src/main/scala/chisel3/internal/Error.scala index 62086870..3b0846eb 100644 --- a/core/src/main/scala/chisel3/internal/Error.scala +++ b/core/src/main/scala/chisel3/internal/Error.scala @@ -186,6 +186,11 @@ private[chisel3] class ErrorLog { 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) + } + /** Emit an informational message */ @deprecated("This method will be removed in 3.5", "3.4") def info(m: String): Unit = -- cgit v1.2.3 From 205d8bb34b2ac2acaef6d318a4f9e3aee181110e Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 7 Jun 2022 23:11:54 +0000 Subject: Add single argument Bits.extract (#2566) (#2568) (cherry picked from commit 255c56c3955a8c16191a6751e7d547cfcfd96705) Co-authored-by: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com>--- core/src/main/scala/chisel3/Bits.scala | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/Bits.scala b/core/src/main/scala/chisel3/Bits.scala index c914e88c..72094a65 100644 --- a/core/src/main/scala/chisel3/Bits.scala +++ b/core/src/main/scala/chisel3/Bits.scala @@ -100,10 +100,10 @@ sealed abstract class Bits(private[chisel3] val width: Width) extends Element wi * @param x an index * @return the specified bit */ - final def apply(x: BigInt): Bool = macro IntLiteralApplyTransform.safeApply + final def extract(x: BigInt): Bool = macro SourceInfoTransform.xArg /** @group SourceInfoTransformMacro */ - final def do_apply(x: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Bool = { + final def do_extract(x: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Bool = { if (x < 0) { Builder.error(s"Negative bit indices are illegal (got $x)") } @@ -123,6 +123,17 @@ sealed abstract class Bits(private[chisel3] val width: Width) extends Element wi } } + /** Returns the specified bit on this $coll as a [[Bool]], statically addressed. + * + * @param x an index + * @return the specified bit + */ + final def apply(x: BigInt): Bool = macro IntLiteralApplyTransform.safeApply + + /** @group SourceInfoTransformMacro */ + final def do_apply(x: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Bool = + do_extract(x) + /** Returns the specified bit on this $coll as a [[Bool]], statically addressed. * * @param x an index @@ -132,21 +143,32 @@ sealed abstract class Bits(private[chisel3] val width: Width) extends Element wi /** @group SourceInfoTransformMacro */ final def do_apply(x: Int)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Bool = - do_apply(BigInt(x)) + do_extract(BigInt(x)) /** Returns the specified bit on this wire as a [[Bool]], dynamically addressed. * * @param x a hardware component whose value will be used for dynamic addressing * @return the specified bit */ - final def apply(x: UInt): Bool = macro SourceInfoTransform.xArg + final def extract(x: UInt): Bool = macro SourceInfoTransform.xArg /** @group SourceInfoTransformMacro */ - final def do_apply(x: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Bool = { + final def do_extract(x: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Bool = { val theBits = this >> x theBits(0) } + /** Returns the specified bit on this wire as a [[Bool]], dynamically addressed. + * + * @param x a hardware component whose value will be used for dynamic addressing + * @return the specified bit + */ + final def apply(x: UInt): Bool = macro SourceInfoTransform.xArg + + /** @group SourceInfoTransformMacro */ + final def do_apply(x: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Bool = + do_extract(x) + /** Returns a subset of bits on this $coll from `hi` to `lo` (inclusive), statically addressed. * * @example -- cgit v1.2.3 From a689c7c0dd336fe0b9db6171786993b190a700f8 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 8 Jun 2022 20:09:45 +0000 Subject: Added migration for inferModuleReset (#2571) (#2573) Co-authored-by: Jack Koenig (cherry picked from commit 3c6c044b6bdee850ad9ba375324abaf3813c557d) Co-authored-by: Adam Izraelevitz --- core/src/main/scala/chisel3/CompileOptions.scala | 3 +++ core/src/main/scala/chisel3/Module.scala | 9 +++++++++ 2 files changed, 12 insertions(+) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/CompileOptions.scala b/core/src/main/scala/chisel3/CompileOptions.scala index db773d6e..2764b652 100644 --- a/core/src/main/scala/chisel3/CompileOptions.scala +++ b/core/src/main/scala/chisel3/CompileOptions.scala @@ -22,6 +22,9 @@ trait CompileOptions { val explicitInvalidate: Boolean // Should the reset type of Module be a Bool or a Reset val inferModuleReset: Boolean + + /** If marked true, then any Module which consumes `inferModuleReset=false` must also mix in [[RequireSyncReset]] */ + def migrateInferModuleReset: Boolean = false } object CompileOptions { diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 3382cd1b..08286ed5 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -141,6 +141,15 @@ abstract class Module(implicit moduleCompileOptions: CompileOptions) extends Raw // Top module and compatibility mode use Bool for reset // Note that a Definition elaboration will lack a parent, but still not be a Top module val inferReset = (_parent.isDefined || Builder.inDefinition) && moduleCompileOptions.inferModuleReset + if (moduleCompileOptions.migrateInferModuleReset && !moduleCompileOptions.inferModuleReset) { + this match { + case _: RequireSyncReset => // Good! It's been migrated. + case _ => // Bad! It hasn't been migrated. + Builder.error( + s"$desiredName is not inferring its module reset, but has not been marked `RequireSyncReset`. Please extend this trait." + ) + } + } if (inferReset) Reset() else Bool() } -- cgit v1.2.3 From 13641d95951b189d7f5b0d4d99ace45f8b8f6282 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 13 Jun 2022 19:11:33 +0000 Subject: Add ImplicitInvalidate, to help migrate the explicitInvalidate compiler option (#2575) (#2579) * Added ImplicitInvalidate trait with tests (cherry picked from commit 1356ced1b89ca35ae0cb1d1ab45227ec1776d5e7) Co-authored-by: Adam Izraelevitz --- core/src/main/scala/chisel3/Data.scala | 2 +- core/src/main/scala/chisel3/Module.scala | 2 +- core/src/main/scala/chisel3/RawModule.scala | 7 +++++-- 3 files changed, 7 insertions(+), 4 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index f468335e..77ad66ef 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -866,7 +866,7 @@ trait WireFactory { x.bind(WireBinding(Builder.forcedUserModule, Builder.currentWhen)) pushCommand(DefWire(sourceInfo, x)) - if (!compileOptions.explicitInvalidate) { + if (!compileOptions.explicitInvalidate || Builder.currentModule.get.isInstanceOf[ImplicitInvalidate]) { pushCommand(DefInvalid(sourceInfo, x.ref)) } diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 08286ed5..d03122f9 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -386,7 +386,7 @@ package internal { clonePorts.setAllParents(Some(cloneParent)) cloneParent._portsRecord = clonePorts // Normally handled during Module construction but ClonePorts really lives in its parent's parent - if (!compileOptions.explicitInvalidate) { + if (!compileOptions.explicitInvalidate || Builder.currentModule.get.isInstanceOf[ImplicitInvalidate]) { // FIXME This almost certainly doesn't work since clonePorts is not a real thing... pushCommand(DefInvalid(sourceInfo, clonePorts.ref)) } diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala index 164d3880..bd04fdc4 100644 --- a/core/src/main/scala/chisel3/RawModule.scala +++ b/core/src/main/scala/chisel3/RawModule.scala @@ -147,7 +147,7 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions) extends // Generate IO invalidation commands to initialize outputs as unused, // unless the client wants explicit control over their generation. val invalidateCommands = { - if (!compileOptions.explicitInvalidate) { + if (!compileOptions.explicitInvalidate || this.isInstanceOf[ImplicitInvalidate]) { getModulePorts.map { port => DefInvalid(UnlocatableSourceInfo, port.ref) } } else { Seq() @@ -161,7 +161,7 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions) extends private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = { implicit val sourceInfo = UnlocatableSourceInfo - if (!parentCompileOptions.explicitInvalidate) { + if (!parentCompileOptions.explicitInvalidate || Builder.currentModule.get.isInstanceOf[ImplicitInvalidate]) { for (port <- getModulePorts) { pushCommand(DefInvalid(sourceInfo, port.ref)) } @@ -177,6 +177,9 @@ trait RequireSyncReset extends Module { override private[chisel3] def mkReset: Bool = Bool() } +/** Mix with a [[RawModule]] to automatically connect DontCare to the module's ports, wires, and children instance IOs. */ +trait ImplicitInvalidate { self: RawModule => } + package object internal { import scala.annotation.implicitNotFound -- cgit v1.2.3 From d001b34f816f1f65d0625aebf33e5cfc5ba93e49 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 16 Jun 2022 23:15:42 +0000 Subject: Define leading '_' as API for creating temporaries (backport #2580) (#2581) * Define leading '_' as API for creating temporaries Chisel and FIRRTL have long used signals with names beginning with an underscore as an API to specify that the name does not really matter. Tools like Verilator follow a similar convention and exclude signals with underscore names from waveform dumps by default. With the introduction of compiler-plugin prefixing in Chisel 3.4, the convention remained but was hard for users to use unless the unnnamed signal existed outside of any prefix domain. Notably, unnamed signals are most useful when creating wires inside of utility methods which almost always results in the signal ending up with a prefix. With this commit, Chisel explicitly recognizes signals whos val names start with an underscore and preserve that underscore regardless of any prefixing. Chisel will also ignore such underscores when generating prefixes based on the temporary signal, preventing accidental double underscores in the names of signals that are prefixed by the temporary. (cherry picked from commit bd94366290886f3489d58f88b9768c7c11fa2cb6) * Remove unused defaultPrefix argument from _computeName (cherry picked from commit ec178aa20a830df2c8c756b9e569709a59073554) # Conflicts: # core/src/main/scala/chisel3/Module.scala # core/src/main/scala/chisel3/experimental/hierarchy/ModuleClone.scala * Resolve backport conflicts * Waive false positive binary compatibility errors Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Module.scala | 4 +- core/src/main/scala/chisel3/RawModule.scala | 57 +++++++++++++++------- .../chisel3/experimental/dataview/package.scala | 2 +- .../experimental/hierarchy/Lookupable.scala | 2 +- core/src/main/scala/chisel3/internal/Builder.scala | 30 +++++------- .../main/scala/chisel3/internal/firrtl/IR.scala | 2 +- 6 files changed, 57 insertions(+), 40 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index d03122f9..ba2d2e32 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -275,7 +275,7 @@ package internal { private[chisel3] def setRefAndPortsRef(namespace: Namespace): Unit = { val record = _portsRecord // Use .forceName to re-use default name resolving behavior - record.forceName(None, default = this.desiredName, namespace) + record.forceName(default = this.desiredName, namespace) // Now take the Ref that forceName set and convert it to the correct Arg val instName = record.getRef match { case Ref(name) => name @@ -497,7 +497,7 @@ package experimental { private[chisel3] def namePorts(names: HashMap[HasId, String]): Unit = { for (port <- getModulePorts) { - port._computeName(None, None).orElse(names.get(port)) match { + port._computeName(None).orElse(names.get(port)) match { case Some(name) => if (_namespace.contains(name)) { Builder.error( diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala index bd04fdc4..f2ce4c70 100644 --- a/core/src/main/scala/chisel3/RawModule.scala +++ b/core/src/main/scala/chisel3/RawModule.scala @@ -94,26 +94,26 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions) extends id match { case id: ModuleClone[_] => id.setRefAndPortsRef(_namespace) // special handling case id: InstanceClone[_] => id.setAsInstanceRef() - case id: BaseModule => id.forceName(None, default = id.desiredName, _namespace) - case id: MemBase[_] => id.forceName(None, default = "MEM", _namespace) - case id: stop.Stop => id.forceName(None, default = "stop", _namespace) - case id: assert.Assert => id.forceName(None, default = "assert", _namespace) - case id: assume.Assume => id.forceName(None, default = "assume", _namespace) - case id: cover.Cover => id.forceName(None, default = "cover", _namespace) - case id: printf.Printf => id.forceName(None, default = "printf", _namespace) + case id: BaseModule => id.forceName(default = id.desiredName, _namespace) + case id: MemBase[_] => id.forceName(default = "MEM", _namespace) + case id: stop.Stop => id.forceName(default = "stop", _namespace) + case id: assert.Assert => id.forceName(default = "assert", _namespace) + case id: assume.Assume => id.forceName(default = "assume", _namespace) + case id: cover.Cover => id.forceName(default = "cover", _namespace) + case id: printf.Printf => id.forceName(default = "printf", _namespace) case id: Data => if (id.isSynthesizable) { id.topBinding match { case OpBinding(_, _) => - id.forceName(Some(""), default = "T", _namespace) + id.forceName(default = "_T", _namespace) case MemoryPortBinding(_, _) => - id.forceName(None, default = "MPORT", _namespace) + id.forceName(default = "MPORT", _namespace) case PortBinding(_) => - id.forceName(None, default = "PORT", _namespace) + id.forceName(default = "PORT", _namespace) case RegBinding(_, _) => - id.forceName(None, default = "REG", _namespace) + id.forceName(default = "REG", _namespace) case WireBinding(_, _) => - id.forceName(Some(""), default = "WIRE", _namespace) + id.forceName(default = "_WIRE", _namespace) case _ => // don't name literals } } // else, don't name unbound types @@ -189,18 +189,39 @@ package object internal { /** Marker trait for modules that are not true modules */ private[chisel3] trait PseudoModule extends BaseModule + /* Check if a String name is a temporary name */ + def isTemp(name: String): Boolean = name.nonEmpty && name.head == '_' + /** Creates a name String from a prefix and a seed * @param prefix The prefix associated with the seed (must be in correct order, *not* reversed) * @param seed The seed for computing the name (if available) */ def buildName(seed: String, prefix: Prefix): String = { - val builder = new StringBuilder() - prefix.foreach { p => - builder ++= p - builder += '_' + // Don't bother copying the String if there's no prefix + if (prefix.isEmpty) { + seed + } else { + // Using Java's String builder to micro-optimize appending a String excluding 1st character + // for temporaries + val builder = new java.lang.StringBuilder() + // Starting with _ is the indicator of a temporary + val temp = isTemp(seed) + // Make sure the final result is also a temporary if this is a temporary + if (temp) { + builder.append('_') + } + prefix.foreach { p => + builder.append(p) + builder.append('_') + } + if (temp) { + // We've moved the leading _ to the front, drop it here + builder.append(seed, 1, seed.length) + } else { + builder.append(seed) + } + builder.toString } - builder ++= seed - builder.toString } // Private reflective version of "val io" to maintain Chisel.Module semantics without having diff --git a/core/src/main/scala/chisel3/experimental/dataview/package.scala b/core/src/main/scala/chisel3/experimental/dataview/package.scala index c583c516..71ae2d8f 100644 --- a/core/src/main/scala/chisel3/experimental/dataview/package.scala +++ b/core/src/main/scala/chisel3/experimental/dataview/package.scala @@ -33,7 +33,7 @@ package object dataview { // The names of views do not matter except for when a view is annotated. For Views that correspond // To a single Data, we just forward the name of the Target. For Views that correspond to more // than one Data, we return this assigned name but rename it in the Convert stage - result.forceName(None, "view", Builder.viewNamespace) + result.forceName("view", Builder.viewNamespace) result } } diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala b/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala index 46a38e7c..c83479b0 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/Lookupable.scala @@ -218,7 +218,7 @@ object Lookupable { result.bind(newBinding) result.setAllParents(Some(ViewParent)) - result.forceName(None, "view", Builder.viewNamespace) + result.forceName("view", Builder.viewNamespace) result } diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 6f02b57c..6fd9bdd5 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -177,21 +177,13 @@ private[chisel3] trait HasId extends InstanceId { } /** Computes the name of this HasId, if one exists - * @param defaultPrefix Optionally provide a default prefix for computing the name * @param defaultSeed Optionally provide default seed for computing the name * @return the name, if it can be computed */ - private[chisel3] def _computeName(defaultPrefix: Option[String], defaultSeed: Option[String]): Option[String] = { - if (hasSeed) { - Some(buildName(seedOpt.get, naming_prefix.reverse)) - } else { - defaultSeed.map { default => - defaultPrefix match { - case Some(p) => buildName(default, p :: naming_prefix.reverse) - case None => buildName(default, naming_prefix.reverse) - } - } - } + private[chisel3] def _computeName(defaultSeed: Option[String]): Option[String] = { + seedOpt + .orElse(defaultSeed) + .map(name => buildName(name, naming_prefix.reverse)) } /** This resolves the precedence of [[autoSeed]] and [[suggestName]] @@ -211,9 +203,9 @@ private[chisel3] trait HasId extends InstanceId { // Uses a namespace to convert suggestion into a true name // Will not do any naming if the reference already assigned. // (e.g. tried to suggest a name to part of a Record) - private[chisel3] def forceName(prefix: Option[String], default: => String, namespace: Namespace): Unit = + private[chisel3] def forceName(default: => String, namespace: Namespace): Unit = if (_ref.isEmpty) { - val candidate_name = _computeName(prefix, Some(default)).get + val candidate_name = _computeName(Some(default)).get val available_name = namespace.name(candidate_name) setRef(Ref(available_name)) // Clear naming prefix to free memory @@ -246,7 +238,7 @@ private[chisel3] trait HasId extends InstanceId { "This will become an error in Chisel 3.6." errors.deprecated(msg, None) errors.checkpoint(logger) - _computeName(None, None).get + _computeName(None).get } // Helper for reifying views if they map to a single Target @@ -506,7 +498,11 @@ private[chisel3] object Builder extends LazyLogging { } } buildAggName(d).map { name => - pushPrefix(name) + if (isTemp(name)) { + pushPrefix(name.tail) + } else { + pushPrefix(name) + } }.isDefined } @@ -749,7 +745,7 @@ private[chisel3] object Builder extends LazyLogging { logger.info("Elaborating design...") val mod = f if (forceModName) { // This avoids definition name index skipping with D/I - mod.forceName(None, mod.name, globalNamespace) + mod.forceName(mod.name, globalNamespace) } errors.checkpoint(logger) logger.info("Done elaborating.") diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala index 1ee8842f..9327c29e 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala @@ -94,7 +94,7 @@ object Arg { case Some(arg) => arg.name case None => id match { - case data: Data => data._computeName(None, Some("?")).get + case data: Data => data._computeName(Some("?")).get case _ => "?" } } -- cgit v1.2.3 From 7e67ca1ef93e53d4b9b6f8e13a21d69e0c5daac4 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 22 Jun 2022 02:01:31 +0000 Subject: Pass optional name in ImportDefinitionAnno (#2592) (#2594) Used for separate elaboration of Definition and Instance (cherry picked from commit 48d57cc8db6f38fdf0e23b7dce36caa404c871b8) Co-authored-by: Girish Pai --- .../experimental/hierarchy/Definition.scala | 4 +- .../chisel3/experimental/hierarchy/Instance.scala | 9 ++++- core/src/main/scala/chisel3/internal/Builder.scala | 44 ++++++++++++++++------ 3 files changed, 43 insertions(+), 14 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala b/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala index c79c26dc..36bf6f87 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala @@ -120,5 +120,7 @@ object Definition extends SourceInfoDoc { /** Stores a [[Definition]] that is imported so that its Instances can be * compiled separately. */ -case class ImportDefinitionAnnotation[T <: BaseModule with IsInstantiable](definition: Definition[T]) +case class ImportDefinitionAnnotation[T <: BaseModule with IsInstantiable]( + definition: Definition[T], + overrideDefName: Option[String] = None) extends NoTargetAnnotation diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/Instance.scala b/core/src/main/scala/chisel3/experimental/hierarchy/Instance.scala index 9f96dff1..861023a1 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/Instance.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/Instance.scala @@ -10,6 +10,7 @@ import chisel3.internal.sourceinfo.{InstanceTransform, SourceInfo} import chisel3.experimental.{BaseModule, ExtModule} import chisel3.internal.firrtl.{Component, DefBlackBox, DefModule, Port} import firrtl.annotations.IsModule +import chisel3.internal.throwException /** User-facing Instance type. * Represents a unique instance of type [[A]] which are marked as @instantiable @@ -118,8 +119,14 @@ object Instance extends SourceInfoDoc { if (existingMod.isEmpty) { // Add a Definition that will get emitted as an ExtModule so that FIRRTL // does not complain about a missing element + val extModName = Builder.importDefinitionMap.getOrElse( + definition.proto.name, + throwException( + "Imported Definition information not found - possibly forgot to add ImportDefinition annotation?" + ) + ) class EmptyExtModule extends ExtModule { - override def desiredName: String = definition.proto.name + override def desiredName: String = extModName override def generateComponent(): Option[Component] = { require(!_closed, s"Can't generate $desiredName module more than once") _closed = true diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 6fd9bdd5..35dd01ab 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -367,11 +367,30 @@ private[chisel3] class DynamicContext( val warnReflectiveNaming: Boolean) { val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a } - // Ensure there are no repeated names for imported Definitions - val importDefinitionNames = importDefinitionAnnos.map { a => a.definition.proto.name } - if (importDefinitionNames.distinct.length < importDefinitionNames.length) { - val duplicates = importDefinitionNames.diff(importDefinitionNames.distinct).mkString(", ") - throwException(s"Expected distinct imported Definition names but found duplicates for: $duplicates") + // Map holding the actual names of extModules + // Pick the definition name by default in case not passed through annotation. + val importDefinitionMap = importDefinitionAnnos + .map(a => a.definition.proto.name -> a.overrideDefName.getOrElse(a.definition.proto.name)) + .toMap + + // Helper function which does 2 things + // 1. Ensure there are no repeated names for imported Definitions - both Proto Names as well as ExtMod Names + // 2. Return the distinct definition / extMod names + private def checkAndGeDistinctProtoExtModNames() = { + val importAllDefinitionProtoNames = importDefinitionAnnos.map { a => a.definition.proto.name } + val importDistinctDefinitionProtoNames = importDefinitionMap.keys.toSeq + val importAllDefinitionExtModNames = importDefinitionMap.toSeq.map(_._2) + val importDistinctDefinitionExtModNames = importAllDefinitionExtModNames.distinct + + if (importDistinctDefinitionProtoNames.length < importAllDefinitionProtoNames.length) { + val duplicates = importAllDefinitionProtoNames.diff(importDistinctDefinitionProtoNames).mkString(", ") + throwException(s"Expected distinct imported Definition names but found duplicates for: $duplicates") + } + if (importDistinctDefinitionExtModNames.length < importAllDefinitionExtModNames.length) { + val duplicates = importAllDefinitionExtModNames.diff(importDistinctDefinitionExtModNames).mkString(", ") + throwException(s"Expected distinct overrideDef names but found duplicates for: $duplicates") + } + (importAllDefinitionProtoNames ++ importAllDefinitionExtModNames).distinct } val globalNamespace = Namespace.empty @@ -379,8 +398,8 @@ private[chisel3] class DynamicContext( // Ensure imported Definitions emit as ExtModules with the correct name so // that instantiations will also use the correct name and prevent any name // conflicts with Modules/Definitions in this elaboration - importDefinitionNames.foreach { importDefName => - globalNamespace.name(importDefName) + checkAndGeDistinctProtoExtModNames().foreach { + globalNamespace.name(_) } val components = ArrayBuffer[Component]() @@ -447,11 +466,12 @@ private[chisel3] object Builder extends LazyLogging { def idGen: IdGen = chiselContext.get.idGen - def globalNamespace: Namespace = dynamicContext.globalNamespace - def components: ArrayBuffer[Component] = dynamicContext.components - def annotations: ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations - def annotationSeq: AnnotationSeq = dynamicContext.annotationSeq - def namingStack: NamingStack = dynamicContext.namingStack + def globalNamespace: Namespace = dynamicContext.globalNamespace + def components: ArrayBuffer[Component] = dynamicContext.components + def annotations: ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations + def annotationSeq: AnnotationSeq = dynamicContext.annotationSeq + def namingStack: NamingStack = dynamicContext.namingStack + def importDefinitionMap: Map[String, String] = dynamicContext.importDefinitionMap def unnamedViews: ArrayBuffer[Data] = dynamicContext.unnamedViews def viewNamespace: Namespace = chiselContext.get.viewNamespace -- cgit v1.2.3 From 1924909e9fec213577cc74970f8cd9c2cf9780c4 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 23 Jun 2022 18:50:35 +0000 Subject: Add DataMirror isIO, isReg, isWire (#2601) (#2602) (cherry picked from commit 7fa0d8bf1cafcdf141046476a100abf021bdcac4) Co-authored-by: Zachary Yedidia --- core/src/main/scala/chisel3/Data.scala | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 77ad66ef..cb8f4683 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -12,6 +12,7 @@ import chisel3.internal.firrtl._ import chisel3.internal.sourceinfo.{DeprecatedSourceInfo, SourceInfo, SourceInfoTransform, UnlocatableSourceInfo} import scala.collection.immutable.LazyList // Needed for 2.12 alias +import scala.reflect.ClassTag import scala.util.Try /** User-specified directions. @@ -157,6 +158,31 @@ package experimental { target.direction } + private def hasBinding[B <: ConstrainedBinding: ClassTag](target: Data) = { + target.topBindingOpt match { + case Some(b: B) => true + case _ => false + } + } + + /** Check if a given `Data` is an IO port + * @param x the `Data` to check + * @return `true` if x is an IO port, `false` otherwise + */ + def isIO(x: Data): Boolean = hasBinding[PortBinding](x) + + /** Check if a given `Data` is a Wire + * @param x the `Data` to check + * @return `true` if x is a Wire, `false` otherwise + */ + def isWire(x: Data): Boolean = hasBinding[WireBinding](x) + + /** Check if a given `Data` is a Reg + * @param x the `Data` to check + * @return `true` if x is a Reg, `false` otherwise + */ + def isReg(x: Data): Boolean = hasBinding[RegBinding](x) + /** Check if two Chisel types are the same type. * Internally, this is dispatched to each Chisel type's * `typeEquivalent` function for each type to determine -- cgit v1.2.3 From 2b977a74293a49e9e2a5d960a6a9c07df22430ce Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 6 Jul 2022 00:28:02 +0000 Subject: Implement trait for Chisel compiler to name arbitrary non-Data types (#2610) (#2617) Co-authored-by: Jack Koenig Co-authored-by: Megan Wachs (cherry picked from commit 3ab34cddd8b87c22d5fc31020f10ddb2f1990d51) Co-authored-by: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com>--- .../main/scala/chisel3/experimental/package.scala | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/experimental/package.scala b/core/src/main/scala/chisel3/experimental/package.scala index 9b9c83f4..47b79099 100644 --- a/core/src/main/scala/chisel3/experimental/package.scala +++ b/core/src/main/scala/chisel3/experimental/package.scala @@ -162,6 +162,36 @@ package object experimental { */ trait NoChiselNamePrefix + /** Generate prefixes from values of this type in the Chisel compiler plugin + * + * Users can mixin this trait to tell the Chisel compiler plugin to include the names of + * vals of this type when generating prefixes for naming `Data` and `Mem` instances. + * This is generally useful whenever creating a `class` that contains `Data`, `Mem`, + * or `Module` instances but does not itself extend `Data` or `Module`. + * + * @see See [[https://www.chisel-lang.org/chisel3/docs/explanations/naming.html the compiler plugin documentation]] for more information on this process. + * + * @example {{{ + * import chisel3._ + * import chisel3.experimental.AffectsChiselPrefix + * + * class MyModule extends Module { + * // Note: This contains a Data but is not a named component itself + * class NotAData extends AffectsChiselPrefix { + * val value = Wire(Bool()) + * } + * + * // Name with AffectsChiselPrefix: "nonData_value" + * // Name without AffectsChiselPrefix: "value" + * val nonData = new NotAData + * + * // Name with AffectsChiselPrefix: "nonData2_value" + * // Name without AffectsChiselPrefix: "value_1" + * val nonData2 = new NotAData + * } + */ + trait AffectsChiselPrefix + object BundleLiterals { implicit class AddBundleLiteralConstructor[T <: Record](x: T) { def Lit(elems: (T => (Data, Data))*)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T = { -- cgit v1.2.3 From 94aeeb1a5c2fe38777a9004ba36f8b353e96b292 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 8 Jul 2022 23:44:45 +0000 Subject: CompileOptions: add and use emitStrictConnects (#2622) (#2623) (cherry picked from commit 11e8cc60d6268301cff352b8a1d7c4d672b5be11) Co-authored-by: Megan Wachs --- core/src/main/scala/chisel3/CompileOptions.scala | 35 +++++++---- core/src/main/scala/chisel3/Data.scala | 6 ++ .../main/scala/chisel3/internal/BiConnect.scala | 70 +++++++++++++--------- 3 files changed, 74 insertions(+), 37 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/CompileOptions.scala b/core/src/main/scala/chisel3/CompileOptions.scala index 2764b652..d7d30306 100644 --- a/core/src/main/scala/chisel3/CompileOptions.scala +++ b/core/src/main/scala/chisel3/CompileOptions.scala @@ -6,25 +6,37 @@ import scala.language.experimental.macros import scala.reflect.macros.blackbox.Context trait CompileOptions { - // Should Record connections require a strict match of fields. - // If true and the same fields aren't present in both source and sink, a MissingFieldException, - // MissingLeftFieldException, or MissingRightFieldException will be thrown. + + /** Should Record connections require a strict match of fields. + * + * If true and the same fields aren't present in both source and sink, a MissingFieldException, + * MissingLeftFieldException, or MissingRightFieldException will be thrown. + */ val connectFieldsMustMatch: Boolean - // When creating an object that takes a type argument, the argument must be unbound (a pure type). + + /** When creating an object that takes a type argument, the argument must be unbound (a pure type). */ val declaredTypeMustBeUnbound: Boolean - // If a connection operator fails, don't try the connection with the operands (source and sink) reversed. + + /** If a connection operator fails, don't try the connection with the operands (source and sink) reversed. */ val dontTryConnectionsSwapped: Boolean - // If connection directionality is not explicit, do not use heuristics to attempt to determine it. + + /** If connection directionality is not explicit, do not use heuristics to attempt to determine it. */ val dontAssumeDirectionality: Boolean - // Check that referenced Data have actually been declared. + + /** Check that referenced Data have actually been declared. */ val checkSynthesizable: Boolean - // Require explicit assignment of DontCare to generate "x is invalid" + + /** Require explicit assignment of DontCare to generate "x is invalid" */ val explicitInvalidate: Boolean - // Should the reset type of Module be a Bool or a Reset + + /** Should the reset type of Module be a Bool or a Reset */ val inferModuleReset: Boolean /** If marked true, then any Module which consumes `inferModuleReset=false` must also mix in [[RequireSyncReset]] */ def migrateInferModuleReset: Boolean = false + + /** Should connects emit as firrtl <= instead of <- */ + def emitStrictConnects: Boolean = true } object CompileOptions { @@ -39,6 +51,7 @@ object CompileOptions { } object ExplicitCompileOptions { + case class CompileOptionsClass( // Should Record connections require a strict match of fields. // If true and the same fields aren't present in both source and sink, a MissingFieldException, @@ -68,7 +81,9 @@ object ExplicitCompileOptions { checkSynthesizable = false, explicitInvalidate = false, inferModuleReset = false - ) + ) { + override def emitStrictConnects = false + } // Collection of "strict" connection compile options, preferred for new code. implicit val Strict = new CompileOptionsClass( diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index cb8f4683..592ebe25 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -596,6 +596,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { private[chisel3] def badConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = throwException(s"cannot connect ${this} and ${that}") + private[chisel3] def connect( that: Data )( @@ -609,6 +610,9 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { case _: ReadOnlyBinding => throwException(s"Cannot reassign to read-only $this") case _ => // fine } + } + if (connectCompileOptions.emitStrictConnects) { + try { MonoConnect.connect(sourceInfo, connectCompileOptions, this, that, Builder.referenceUserModule) } catch { @@ -636,6 +640,8 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { case (_: DontCareBinding, _) => throw BiConnect.DontCareCantBeSink case _ => // fine } + } + if (connectCompileOptions.emitStrictConnects) { try { BiConnect.connect(sourceInfo, connectCompileOptions, this, that, Builder.referenceUserModule) } catch { diff --git a/core/src/main/scala/chisel3/internal/BiConnect.scala b/core/src/main/scala/chisel3/internal/BiConnect.scala index 2d6c9e4a..e8fb2361 100644 --- a/core/src/main/scala/chisel3/internal/BiConnect.scala +++ b/core/src/main/scala/chisel3/internal/BiConnect.scala @@ -55,6 +55,24 @@ private[chisel3] object BiConnect { * There is some cleverness in the use of internal try-catch to catch exceptions * during the recursive decent and then rethrow them with extra information added. * This gives the user a 'path' to where in the connections things went wrong. + * + * == Chisel Semantics and how they emit to firrtl == + * + * 1. Strict Bi-Connect (all fields as seen by firrtl must match exactly) + * `a <= b` + * + * 2. Strict Bi-Connect (implemented as being field-blasted because we know all firrtl fields would not match exactly) + * `a.foo <= b.foo, b.bar <= a.bar` + * + * 3. Not-Strict Bi-Connect (firrtl will allow fields to not match exactly) + * `a <- b` + * + * 4. Mixed Semantic Bi-Connect (some fields need to be handled differently) + * `a.foo <= b.foo` (case 2), `b.bar <- a.bar` (case 3) + * + * - The decision on 1 vs 2 is based on structural type -- if same type once emitted to firrtl, emit 1, otherwise emit 2 + * - 1/2 vs 3 is based on CompileOptions at connection point e.g. at `<>` , emit 3 if `emitStrictConnects = false` for either side + * - 4 is a special case of 2 turning into 3 for some subfields, when either side's subfield at `extends Bundle/Record` has `emitStrictConnects = false` */ def connect( sourceInfo: SourceInfo, @@ -140,8 +158,8 @@ private[chisel3] object BiConnect { // Handle Records defined in Chisel._ code by emitting a FIRRTL bulk // connect when possible and a partial connect otherwise case pair @ (left_r: Record, right_r: Record) => - val notStrict = - Seq(left_r.compileOptions, right_r.compileOptions).contains(ExplicitCompileOptions.NotStrict) + val emitStrictConnects: Boolean = + left_r.compileOptions.emitStrictConnects && right_r.compileOptions.emitStrictConnects // chisel3 <> is commutative but FIRRTL <- is not val flipConnection = @@ -161,40 +179,38 @@ private[chisel3] object BiConnect { ) ) { pushCommand(Connect(sourceInfo, leftReified.get.lref, rightReified.get.lref)) - } else if (notStrict) { - newLeft.bulkConnect(newRight)(sourceInfo, ExplicitCompileOptions.NotStrict) + } else if (!emitStrictConnects) { + newLeft.legacyConnect(newRight)(sourceInfo) } else { recordConnect(sourceInfo, connectCompileOptions, left_r, right_r, context_mod) } - // Handle Records connected to DontCare (change to NotStrict) + // Handle Records connected to DontCare case (left_r: Record, DontCare) => - left_r.compileOptions match { - case ExplicitCompileOptions.NotStrict => - left.bulkConnect(right)(sourceInfo, ExplicitCompileOptions.NotStrict) - case _ => - // For each field in left, descend with right - for ((field, left_sub) <- left_r.elements) { - try { - connect(sourceInfo, connectCompileOptions, left_sub, right, context_mod) - } catch { - case BiConnectException(message) => throw BiConnectException(s".$field$message") - } + if (!left_r.compileOptions.emitStrictConnects) { + left.legacyConnect(right)(sourceInfo) + } else { + // For each field in left, descend with right + for ((field, left_sub) <- left_r.elements) { + try { + connect(sourceInfo, connectCompileOptions, left_sub, right, context_mod) + } catch { + case BiConnectException(message) => throw BiConnectException(s".$field$message") } + } } case (DontCare, right_r: Record) => - right_r.compileOptions match { - case ExplicitCompileOptions.NotStrict => - left.bulkConnect(right)(sourceInfo, ExplicitCompileOptions.NotStrict) - case _ => - // For each field in left, descend with right - for ((field, right_sub) <- right_r.elements) { - try { - connect(sourceInfo, connectCompileOptions, left, right_sub, context_mod) - } catch { - case BiConnectException(message) => throw BiConnectException(s".$field$message") - } + if (!right_r.compileOptions.emitStrictConnects) { + left.legacyConnect(right)(sourceInfo) + } else { + // For each field in left, descend with right + for ((field, right_sub) <- right_r.elements) { + try { + connect(sourceInfo, connectCompileOptions, left, right_sub, context_mod) + } catch { + case BiConnectException(message) => throw BiConnectException(s".$field$message") } + } } // Left and right are different subtypes of Data so fail -- cgit v1.2.3 From dbffb8779efca6bea8699ed80a04b1d47d657d93 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 13 Jul 2022 20:39:44 +0000 Subject: New enhanced API for specifying Chisel to Firrtl Annotations (#2628) (#2631) (cherry picked from commit 4b10cf7a276e90b280c1fd57070566acac3d80d3) Co-authored-by: Girish Pai --- core/src/main/scala/chisel3/Annotation.scala | 11 +++++++ core/src/main/scala/chisel3/internal/Builder.scala | 13 +++++--- .../main/scala/chisel3/internal/firrtl/IR.scala | 37 ++++++++++++++++++++-- 3 files changed, 55 insertions(+), 6 deletions(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/Annotation.scala b/core/src/main/scala/chisel3/Annotation.scala index e08557eb..c350fb30 100644 --- a/core/src/main/scala/chisel3/Annotation.scala +++ b/core/src/main/scala/chisel3/Annotation.scala @@ -20,6 +20,14 @@ trait ChiselAnnotation { def toFirrtl: Annotation } +/** Enhanced interface for Annotations in Chisel + * + * Defines a conversion to corresponding FIRRTL Annotation(s) + */ +trait ChiselMultiAnnotation { + def toFirrtl: Seq[Annotation] +} + /** Mixin for [[ChiselAnnotation]] that instantiates an associated FIRRTL Transform when this Annotation is present * during a run of * [[Driver$.execute(args:Array[String],dut:()=>chisel3\.RawModule)* Driver.execute]]. @@ -34,6 +42,9 @@ object annotate { def apply(anno: ChiselAnnotation): Unit = { Builder.annotations += anno } + def apply(annos: ChiselMultiAnnotation): Unit = { + Builder.newAnnotations += annos + } } /** Marks that a module to be ignored in Dedup Transform in Firrtl pass diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 35dd01ab..61f94f8f 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -404,6 +404,7 @@ private[chisel3] class DynamicContext( val components = ArrayBuffer[Component]() val annotations = ArrayBuffer[ChiselAnnotation]() + val newAnnotations = ArrayBuffer[ChiselMultiAnnotation]() var currentModule: Option[BaseModule] = None /** Contains a mapping from a elaborated module to their aspect @@ -466,9 +467,13 @@ private[chisel3] object Builder extends LazyLogging { def idGen: IdGen = chiselContext.get.idGen - def globalNamespace: Namespace = dynamicContext.globalNamespace - def components: ArrayBuffer[Component] = dynamicContext.components - def annotations: ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations + def globalNamespace: Namespace = dynamicContext.globalNamespace + def components: ArrayBuffer[Component] = dynamicContext.components + def annotations: ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations + + // TODO : Unify this with annotations in the future - done this way for backward compatability + def newAnnotations: ArrayBuffer[ChiselMultiAnnotation] = dynamicContext.newAnnotations + def annotationSeq: AnnotationSeq = dynamicContext.annotationSeq def namingStack: NamingStack = dynamicContext.namingStack def importDefinitionMap: Map[String, String] = dynamicContext.importDefinitionMap @@ -770,7 +775,7 @@ private[chisel3] object Builder extends LazyLogging { errors.checkpoint(logger) logger.info("Done elaborating.") - (Circuit(components.last.name, components.toSeq, annotations.toSeq, makeViewRenameMap), mod) + (Circuit(components.last.name, components.toSeq, annotations.toSeq, makeViewRenameMap, newAnnotations.toSeq), mod) } } initializeSingletons() diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala index 9327c29e..dc9ab027 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala @@ -861,7 +861,40 @@ case class DefBlackBox( params: Map[String, Param]) extends Component -case class Circuit(name: String, components: Seq[Component], annotations: Seq[ChiselAnnotation], renames: RenameMap) { - def firrtlAnnotations: Iterable[Annotation] = annotations.flatMap(_.toFirrtl.update(renames)) +case class Circuit( + name: String, + components: Seq[Component], + @deprecated("Do not use annotations val of Circuit directly - use firrtlAnnotations instead. Will be removed in a future release", + "Chisel 3.5") + annotations: Seq[ChiselAnnotation], + renames: RenameMap, + @deprecated("Do not use newAnnotations val of Circuit directly - use firrtlAnnotations instead. Will be removed in a future release", + "Chisel 3.5") + + newAnnotations: Seq[ChiselMultiAnnotation]) { + + def this(name: String, components: Seq[Component], annotations: Seq[ChiselAnnotation], renames: RenameMap) = + this(name, components, annotations, renames, Seq.empty) + + def firrtlAnnotations: Iterable[Annotation] = + annotations.flatMap(_.toFirrtl.update(renames)) ++ newAnnotations.flatMap( + _.toFirrtl.flatMap(_.update(renames)) + ) + + def copy( + name: String = name, + components: Seq[Component] = components, + annotations: Seq[ChiselAnnotation] = annotations, + renames: RenameMap = renames + ) = Circuit(name, components, annotations, renames, newAnnotations) + +} +object Circuit + extends scala.runtime.AbstractFunction4[String, Seq[Component], Seq[ChiselAnnotation], RenameMap, Circuit] { + def unapply(c: Circuit): Option[(String, Seq[Component], Seq[ChiselAnnotation], RenameMap)] = { + Some((c.name, c.components, c.annotations, c.renames)) + } + def apply(name: String, components: Seq[Component], annotations: Seq[ChiselAnnotation], renames: RenameMap): Circuit = + new Circuit(name, components, annotations, renames) } -- cgit v1.2.3 From 80035d2a7b94faf9bfef962f83f9257f57419a35 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 14 Jul 2022 08:30:13 -0700 Subject: 3.5x: Make explicit copy constructors for ExplicitCompileOptions (#2629) * Add copy constructors for compile options * Add tests for the copy constructors Co-authored-by: Jiuyang Liu --- core/src/main/scala/chisel3/CompileOptions.scala | 48 +++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/CompileOptions.scala b/core/src/main/scala/chisel3/CompileOptions.scala index d7d30306..aca00d1f 100644 --- a/core/src/main/scala/chisel3/CompileOptions.scala +++ b/core/src/main/scala/chisel3/CompileOptions.scala @@ -82,7 +82,28 @@ object ExplicitCompileOptions { explicitInvalidate = false, inferModuleReset = false ) { + override def migrateInferModuleReset = false override def emitStrictConnects = false + override def copy( + connectFieldsMustMatch: Boolean = false, + declaredTypeMustBeUnbound: Boolean = false, + dontTryConnectionsSwapped: Boolean = false, + dontAssumeDirectionality: Boolean = false, + checkSynthesizable: Boolean = false, + explicitInvalidate: Boolean = false, + inferModuleReset: Boolean = false + ) = new CompileOptionsClass( + connectFieldsMustMatch, + declaredTypeMustBeUnbound, + dontTryConnectionsSwapped, + dontAssumeDirectionality, + checkSynthesizable, + explicitInvalidate, + inferModuleReset + ) { + override def migrateInferModuleReset = false + override def emitStrictConnects = false + } } // Collection of "strict" connection compile options, preferred for new code. @@ -94,5 +115,30 @@ object ExplicitCompileOptions { checkSynthesizable = true, explicitInvalidate = true, inferModuleReset = true - ) + ) { + + override def migrateInferModuleReset = false + override def emitStrictConnects = true + + override def copy( + connectFieldsMustMatch: Boolean = true, + declaredTypeMustBeUnbound: Boolean = true, + dontTryConnectionsSwapped: Boolean = true, + dontAssumeDirectionality: Boolean = true, + checkSynthesizable: Boolean = true, + explicitInvalidate: Boolean = true, + inferModuleReset: Boolean = true + ) = new CompileOptionsClass( + connectFieldsMustMatch, + declaredTypeMustBeUnbound, + dontTryConnectionsSwapped, + dontAssumeDirectionality, + checkSynthesizable, + explicitInvalidate, + inferModuleReset + ) { + override def migrateInferModuleReset = false + override def emitStrictConnects = true + } + } } -- cgit v1.2.3 From f46d02f55bd22ffda32b20e8cc4b40aa96b03ee0 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 21 Jul 2022 14:16:11 -0700 Subject: Deprecate chiselName and NoChiselNamePrefix trait (#2627) (#2633) Also remove all non-testing uses of chiselName. (cherry picked from commit 1c5d1b5317a0c9fe7ef9d15138065a817380a1e4) Co-authored-by: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com>--- core/src/main/scala/chisel3/experimental/package.scala | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/experimental/package.scala b/core/src/main/scala/chisel3/experimental/package.scala index 47b79099..b1d9cae4 100644 --- a/core/src/main/scala/chisel3/experimental/package.scala +++ b/core/src/main/scala/chisel3/experimental/package.scala @@ -160,6 +160,10 @@ package object experimental { * } * }}} */ + @deprecated( + "@chiselName and NoChiselNamePrefix have been replaced by the compiler plugin and AffectsChiselPrefix. Use these instead", + "Chisel 3.5" + ) trait NoChiselNamePrefix /** Generate prefixes from values of this type in the Chisel compiler plugin -- cgit v1.2.3 From 0ff14a949f6781f5d76c6088202863d16cb24a18 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 22 Jul 2022 18:35:31 +0000 Subject: ChiselEnum: make factory package private (#2639) (#2640) This is required in order to support peeks in chiseltest. (cherry picked from commit 26cd15a9943ca20829630d2feedac08a069291c2) Co-authored-by: Kevin Laeufer --- core/src/main/scala/chisel3/StrongEnum.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/src') diff --git a/core/src/main/scala/chisel3/StrongEnum.scala b/core/src/main/scala/chisel3/StrongEnum.scala index 6f9cce55..cd6f11ee 100644 --- a/core/src/main/scala/chisel3/StrongEnum.scala +++ b/core/src/main/scala/chisel3/StrongEnum.scala @@ -71,7 +71,7 @@ object EnumAnnotations { } import EnumAnnotations._ -abstract class EnumType(private val factory: EnumFactory, selfAnnotating: Boolean = true) extends Element { +abstract class EnumType(private[chisel3] val factory: EnumFactory, selfAnnotating: Boolean = true) extends Element { // Use getSimpleName instead of enumTypeName because for debugging purposes // the fully qualified name isn't necessary (compared to for the -- cgit v1.2.3