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/RawModule.scala | 21 --------------------- 1 file changed, 21 deletions(-) (limited to 'core/src/main/scala/chisel3/RawModule.scala') 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 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 ++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) (limited to 'core/src/main/scala/chisel3/RawModule.scala') 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" -- 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/RawModule.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'core/src/main/scala/chisel3/RawModule.scala') 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/RawModule.scala | 57 ++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 18 deletions(-) (limited to 'core/src/main/scala/chisel3/RawModule.scala') 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 -- cgit v1.2.3