From ea1ced34b5c9e42412cc0ac3e7431cd3194ccbc3 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 1 Feb 2022 19:56:13 +0000 Subject: Chisel plugin bundle elements handler (#2306) (#2380) Adds generation of `Bundle.elements` method to the chores done by the compiler plugin For each `Bundle` find the relevant visible Chisel field members and construct a hard-coded list of the elements and their names implemented as `_elementsImpl` For more details: See plugins/README.md - Should be no change in API - Handles inheritance and mixins - Handles Seq[Data] - Tests in BundleElementSpec Co-authored-by: chick Co-authored-by: Jack Koenig (cherry picked from commit 237200a420581519f29149cbae9b3e968c0d01fc) Co-authored-by: Chick Markley --- build.sbt | 1 + build.sc | 4 + core/src/main/scala/chisel3/Aggregate.scala | 50 +- core/src/main/scala/chisel3/Data.scala | 2 +- plugin/README.md | 99 ++++ .../chisel3/internal/plugin/BundleComponent.scala | 190 +++++-- .../chisel3/internal/plugin/ChiselPlugin.scala | 12 +- .../scala/chiselTests/BundleElementsSpec.scala | 564 +++++++++++++++++++++ src/test/scala/chiselTests/BundleSpec.scala | 10 +- 9 files changed, 887 insertions(+), 45 deletions(-) create mode 100644 plugin/README.md create mode 100644 src/test/scala/chiselTests/BundleElementsSpec.scala diff --git a/build.sbt b/build.sbt index b9406c8f..f5f178fd 100644 --- a/build.sbt +++ b/build.sbt @@ -198,6 +198,7 @@ lazy val chisel = (project in file(".")). settings( mimaPreviousArtifacts := Set("edu.berkeley.cs" %% "chisel3" % "3.5.0"), libraryDependencies += defaultVersions("treadle") % "test", + Test / scalacOptions += "-P:chiselplugin:genBundleElements", scalacOptions in Test ++= Seq("-language:reflectiveCalls"), scalacOptions in Compile in doc ++= Seq( "-diagrams", diff --git a/build.sc b/build.sc index 1e665a9f..d7dbc6d7 100644 --- a/build.sc +++ b/build.sc @@ -107,6 +107,10 @@ class chisel3CrossModule(val crossScalaVersion: String) extends CommonModule wit object test extends Tests { override def scalacPluginClasspath = m.scalacPluginClasspath + override def scalacOptions = T { + super.scalacOptions() ++ Agg("-P:chiselplugin:genBundleElements") + } + override def ivyDeps = m.ivyDeps() ++ Agg( ivy"org.scalatest::scalatest:3.2.10", ivy"org.scalatestplus::scalacheck-1-14:3.2.2.0", diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 15cdf428..75966a16 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -1208,12 +1208,58 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record { * }}} */ final lazy val elements: SeqMap[String, Data] = { + val hardwareFields = _elementsImpl.flatMap { + case (name, data: Data) => + if (data.isSynthesizable) { + Some(s"$name: $data") + } else { + None + } + case (name, Some(data: Data)) => + if (data.isSynthesizable) { + Some(s"$name: $data") + } else { + None + } + case (name, s: scala.collection.Seq[Any]) if s.nonEmpty => + s.head match { + // Ignore empty Seq() + case d: Data => + throwException( + "Public Seq members cannot be used to define Bundle elements " + + s"(found public Seq member '${name}'). " + + "Either use a Vec if all elements are of the same type, or MixedVec if the elements " + + "are of different types. If this Seq member is not intended to construct RTL, mix in the trait " + + "IgnoreSeqInBundle." + ) + case _ => // don't care about non-Data Seq + } + None + + case _ => None + } + if (hardwareFields.nonEmpty) { + throw ExpectedChiselTypeException(s"Bundle: $this contains hardware fields: " + hardwareFields.mkString(",")) + } + VectorMap(_elementsImpl.toSeq.flatMap { + case (name, data: Data) => + Some(name -> data) + case (name, Some(data: Data)) => + Some(name -> data) + case _ => None + }.sortWith { + case ((an, a), (bn, b)) => (a._id > b._id) || ((a eq b) && (an > bn)) + }: _*) + } + /* + * This method will be overwritten by the Chisel-Plugin + */ + protected def _elementsImpl: SeqMap[String, Any] = { val nameMap = LinkedHashMap[String, Data]() for (m <- getPublicFields(classOf[Bundle])) { getBundleField(m) match { case Some(d: Data) => - requireIsChiselType(d) - + // Checking for chiselType is done in elements method if (nameMap contains m.getName) { require(nameMap(m.getName) eq d) } else { diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 9e9f5dbb..ef43a3b0 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -480,7 +480,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { protected[chisel3] def binding: Option[Binding] = _binding protected def binding_=(target: Binding) { if (_binding.isDefined) { - throw RebindingException(s"Attempted reassignment of binding to $this") + throw RebindingException(s"Attempted reassignment of binding to $this, from: ${target}") } _binding = Some(target) } diff --git a/plugin/README.md b/plugin/README.md new file mode 100644 index 00000000..762f4822 --- /dev/null +++ b/plugin/README.md @@ -0,0 +1,99 @@ +# Notes on the Compiler Plug-in + +The Chisel plugin provides some operations that are too difficult, or not possbile, +to implement through regular Scala code. + +# This documentation is for developers working on chisel internals. + +## Compiler plugin operations +These are the two things that the compile plugin does. + +1. Automatically generates the `cloneType` methods of Bundle +2. Changes the underlying mechanics of the `Bundle`s `elements` method in a way +that does not require the use of **reflection** +3. Future work: Make having a Seq[Data] in a bundle be a compiler error. See "Detecting Bundles with Seq[Data]" below. + +### 1. Generating `cloneType` method +As of Mar 18, 2021, PR #1826, generating the `cloneType` method (1. above) is now the default behavior. +The cloneType method used to be a tricky thing to write for chisel developers. +For historical purposes, here is the flag was used to control that prior to full adoption. +``` +-P:chiselplugin:useBundlePlugin +``` + +### 2. Changing `Bundle#elements` method + +A `Bundle` has a default `elements` method that relies on **reflection**, which is slow and brittle, to access the list of +*fields* the bundle contains. +When enabled this second operation of the plugin examines +the `Bundle`s AST in order to determine the fields and then re-writes the underlying code of `elements`. +Technically, rewriting a lower level private method `_elementsImpl`. +It is expected that the using this feature will shortly become the default. + +>The plugin should not be enabled for the `main` chisel3 project because of internal considerations. +> It is enabled for the `Test` section. + +In the meantime, advanced users can try using the feature by adding the following flag to the scalac options in their +chisel projects. + +``` +-P:chiselplugin:buildElementAccessor +``` + +For example in an `build.sbt` file adding the line +``` +scalacOptions += "-P:chiselplugin:genBundleElements", +``` +in the appropriate place. + +## Future work +### Detecting Bundles with Seq[Data] +Trying to have a `val Seq[Data]` (as opposed to a `val Vec[Data]` in a `Bundle` is a run time error. +Here is a block of code that could be added to the plugin to detect this case at compile time (with some refinement in +the detection mechanism): +```scala + if (member.isAccessor && typeIsSeqOfData(member.tpe) && !isIgnoreSeqInBundle(bundleSymbol)) { + global.reporter.error( + member.pos, + s"Bundle.field ${bundleSymbol.name}.${member.name} cannot be a Seq[Data]. " + + "Use Vec or MixedVec or mix in trait IgnoreSeqInBundle" + ) + } +``` +### Notes about working on the `_elementsImpl` generator for the plugin in `BundleComponent.scala` +In general the easiest way to develop and debug new code in the plugin is to use `println` statements. +Naively this can result in reams of text that can be very hard to look through. + +What I found to be useful was creating some wrappers for `println` that only printed when the `Bundles` had a particular name pattern. +- Create a regular expression string in the `BundleComponent` class +- Add a printf wrapper name `show` that checks the `Bundle`'s name against the regex +- For recursive code in `getAllBundleFields` create a different wrapper `indentShow` that indents debug lines +- Sprinkle calls to these wrappers as needed for debugging + +#### Bundle Regex +```scala + val bundleNameDebugRegex = "MyBundle.*" +``` +#### Add `show` wrapper +`show` should be inside `case bundle` block of the `transform` method in order to have access to the current `Bundle` + +```scala +def show(string: => String): Unit = { + if (bundle.symbol.name.toString.matches(bundleNameDebugRegex)) { + println(string) + } +} +``` +#### Add `indentShow` wrapper +This method can be added into `BundleComponent.scala` in the `transform` method after `case Bundle` +Inside of `getAllBundleFields` I added the following code that indented for each recursion up the current +`Bundle`'s hierarchy. +```scala +def indentShow(s: => String): Unit = { + val indentString = ("-" * depth) * 2 + "> " + s.split("\n").foreach { line => + show(indentString + line) + } +} +``` + diff --git a/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala b/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala index 2d3a2cae..e92bbb23 100644 --- a/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala +++ b/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala @@ -9,7 +9,16 @@ import scala.tools.nsc.plugins.PluginComponent import scala.tools.nsc.symtab.Flags import scala.tools.nsc.transform.TypingTransformers -// TODO This component could also implement val elements in Bundles +/** Performs three operations + * 1) Records that this plugin ran on a bundle by adding a method + * `override protected def _usingPlugin: Boolean = true` + * 2) Constructs a cloneType method + * 3) Builds a `def elements` that is computed once in this plugin + * Eliminates needing reflection to discover the hardware fields of a `Bundle` + * + * @param global the environment + * @param arguments run time parameters to code + */ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPluginArguments) extends PluginComponent with TypingTransformers { @@ -32,14 +41,41 @@ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPlugi def inferType(t: Tree): Type = localTyper.typed(t, nsc.Mode.TYPEmode).tpe - val bundleTpe = inferType(tq"chisel3.Bundle") - val dataTpe = inferType(tq"chisel3.Data") + val bundleTpe: Type = inferType(tq"chisel3.Bundle") + val dataTpe: Type = inferType(tq"chisel3.Data") + val ignoreSeqTpe: Type = inferType(tq"chisel3.IgnoreSeqInBundle") + val seqOfDataTpe: Type = inferType(tq"scala.collection.Seq[chisel3.Data]") + val someOfDataTpe: Type = inferType(tq"scala.Option[chisel3.Data]") + val seqMapTpe: Type = inferType(tq"scala.collection.immutable.SeqMap[String,Any]") // Not cached because it should only be run once per class (thus once per Type) - def isBundle(sym: Symbol): Boolean = sym.tpe <:< bundleTpe + def isBundle(sym: Symbol): Boolean = { sym.tpe <:< bundleTpe } + + def isIgnoreSeqInBundle(sym: Symbol): Boolean = { sym.tpe <:< ignoreSeqTpe } + + def isSeqOfData(sym: Symbol): Boolean = { + val tpe = sym.tpe + tpe match { + case NullaryMethodType(resultType) => + resultType <:< seqOfDataTpe + case _ => + false + } + } + + def isOptionOfData(symbol: Symbol): Boolean = { + val tpe = symbol.tpe + tpe match { + case NullaryMethodType(resultType) => + resultType <:< someOfDataTpe + case _ => + false + } + } + def isExactBundle(sym: Symbol): Boolean = { sym.tpe =:= bundleTpe } - val isDataCache = new mutable.HashMap[Type, Boolean] // Cached because this is run on every argument to every Bundle + val isDataCache = new mutable.HashMap[Type, Boolean] def isData(sym: Symbol): Boolean = isDataCache.getOrElseUpdate(sym.tpe, sym.tpe <:< dataTpe) def cloneTypeFull(tree: Tree): Tree = @@ -63,7 +99,7 @@ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPlugi val msg = "Users cannot override _usingPlugin, it is for the compiler plugin's use only." global.globalError(d.pos, msg) case d: DefDef if isNullaryMethodNamed("cloneType", d) => - val msg = "Users cannot override cloneType. Let the compiler plugin generate it." + val msg = "Users cannot override cloneType. Let the compiler plugin generate it." global.globalError(d.pos, msg) case _ => } @@ -72,52 +108,136 @@ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPlugi override def transform(tree: Tree): Tree = tree match { - case bundle: ClassDef if isBundle(bundle.symbol) && !bundle.mods.hasFlag(Flag.ABSTRACT) => + case bundle: ClassDef if isBundle(bundle.symbol) => // ==================== Generate _cloneTypeImpl ==================== val (con, params) = getConstructorAndParams(bundle.impl.body) if (con.isEmpty) { global.reporter.warning(bundle.pos, "Unable to determine primary constructor!") return super.transform(tree) } - val constructor = con.get + val constructor = con.get val thiz = gen.mkAttributedThis(bundle.symbol) // The params have spaces after them (Scalac implementation detail) val paramLookup: String => Symbol = params.map(sym => sym.name.toString.trim -> sym).toMap - // Create a this. for each field matching order of constructor arguments - // List of Lists because we can have multiple parameter lists - val conArgs: List[List[Tree]] = - constructor.vparamss.map(_.map { vp => - val p = paramLookup(vp.name.toString) - // Make this. - val select = gen.mkAttributedSelect(thiz, p) - // Clone any Data parameters to avoid field aliasing, need full clone to include direction - if (isData(vp.symbol)) cloneTypeFull(select) else select - }) - - val tparamList = bundle.tparams.map { t => Ident(t.symbol) } - val ttpe = if (tparamList.nonEmpty) AppliedTypeTree(Ident(bundle.symbol), tparamList) else Ident(bundle.symbol) - val newUntyped = New(ttpe, conArgs) - val neww = localTyper.typed(newUntyped) - - // Create the symbol for the method and have it be associated with the Bundle class - val cloneTypeSym = - bundle.symbol.newMethod(TermName("_cloneTypeImpl"), bundle.symbol.pos.focus, Flag.OVERRIDE | Flag.PROTECTED) - // Handwritten cloneTypes don't have the Method flag set, unclear if it matters - cloneTypeSym.resetFlag(Flags.METHOD) - // Need to set the type to chisel3.Bundle for the override to work - cloneTypeSym.setInfo(NullaryMethodType(bundleTpe)) - - val cloneTypeImpl = localTyper.typed(DefDef(cloneTypeSym, neww)) + val cloneTypeImplOpt = if (!bundle.mods.hasFlag(Flag.ABSTRACT)) { + // Create a this. for each field matching order of constructor arguments + // List of Lists because we can have multiple parameter lists + val conArgs: List[List[Tree]] = + constructor.vparamss.map(_.map { vp => + val p = paramLookup(vp.name.toString) + // Make this. + val select = gen.mkAttributedSelect(thiz.asInstanceOf[Tree], p) + // Clone any Data parameters to avoid field aliasing, need full clone to include direction + if (isData(vp.symbol)) cloneTypeFull(select.asInstanceOf[Tree]) else select + }) + + val tparamList = bundle.tparams.map { t => Ident(t.symbol) } + val ttpe = + if (tparamList.nonEmpty) AppliedTypeTree(Ident(bundle.symbol), tparamList) else Ident(bundle.symbol) + val newUntyped = New(ttpe, conArgs) + val neww = localTyper.typed(newUntyped) + + // Create the symbol for the method and have it be associated with the Bundle class + val cloneTypeSym = + bundle.symbol.newMethod(TermName("_cloneTypeImpl"), bundle.symbol.pos.focus, Flag.OVERRIDE | Flag.PROTECTED) + // Handwritten cloneTypes don't have the Method flag set, unclear if it matters + cloneTypeSym.resetFlag(Flags.METHOD) + // Need to set the type to chisel3.Bundle for the override to work + cloneTypeSym.setInfo(NullaryMethodType(bundleTpe)) + + Some(localTyper.typed(DefDef(cloneTypeSym, neww))) + } else { + // Don't create if this Bundle is abstract + None + } + + // ==================== Generate val elements ==================== + + /* Test to see if the bundle found is amenable to having it's elements + * converted to an immediate form that will not require reflection + */ + def isSupportedBundleType: Boolean = { + arguments.genBundleElements && !bundle.mods.hasFlag(Flag.ABSTRACT) + } + + val elementsImplOpt = if (isSupportedBundleType) { + /* extract the true fields from the super classes a given bundle + * depth argument can be helpful for debugging + */ + def getAllBundleFields(bundleSymbol: Symbol, depth: Int = 0): List[(String, Tree)] = { + + def isBundleField(member: Symbol): Boolean = { + if (!member.isAccessor) { + false + } else if (isData(member.tpe.typeSymbol)) { + true + } else if (isOptionOfData(member)) { + true + } else if (isSeqOfData(member)) { + // This field is passed along, even though it is illegal + // An error for this will be generated in `Bundle.elements` + // It would be possible here to check for Seq[Data] and make a compiler error, but + // that would be a API error difference. See reference in docs/chisel-plugin.md + // If Bundle is subclass of IgnoreSeqInBundle then don't pass this field along + + !isIgnoreSeqInBundle(bundleSymbol) + } else { + // none of the above + false + } + } + + val currentFields = bundleSymbol.info.members.flatMap { + + case member if member.isPublic => + if (isBundleField(member)) { + // The params have spaces after them (Scalac implementation detail) + Some(member.name.toString.trim -> gen.mkAttributedSelect(thiz.asInstanceOf[Tree], member)) + } else { + None + } + + case _ => None + }.toList + + val allParentFields = bundleSymbol.parentSymbols.flatMap { parentSymbol => + val fieldsFromParent = if (depth < 1 && !isExactBundle(bundleSymbol)) { + val foundFields = getAllBundleFields(parentSymbol, depth + 1) + foundFields + } else { + List() + } + fieldsFromParent + } + allParentFields ++ currentFields + } + + val elementArgs = getAllBundleFields(bundle.symbol) + + val elementsImplSym = + bundle.symbol.newMethod(TermName("_elementsImpl"), bundle.symbol.pos.focus, Flag.OVERRIDE | Flag.PROTECTED) + elementsImplSym.resetFlag(Flags.METHOD) + elementsImplSym.setInfo(NullaryMethodType(seqMapTpe)) + + val elementsImpl = localTyper.typed( + DefDef(elementsImplSym, q"scala.collection.immutable.SeqMap.apply[String, Any](..$elementArgs)") + ) + + Some(elementsImpl) + } else { + // No code generated for elements accessor + None + } // ==================== Generate _usingPlugin ==================== // Unclear why quasiquotes work here but didn't for cloneTypeSym, maybe they could. - val usingPlugin = localTyper.typed(q"override protected def _usingPlugin: Boolean = true") + val usingPluginOpt = Some(localTyper.typed(q"override protected def _usingPlugin: Boolean = true")) val withMethods = deriveClassDef(bundle) { t => - deriveTemplate(t)(_ :+ cloneTypeImpl :+ usingPlugin) + deriveTemplate(t)(_ ++ cloneTypeImplOpt ++ usingPluginOpt ++ elementsImplOpt) } super.transform(localTyper.typed(withMethods)) diff --git a/plugin/src/main/scala/chisel3/internal/plugin/ChiselPlugin.scala b/plugin/src/main/scala/chisel3/internal/plugin/ChiselPlugin.scala index bd02d50c..9bf8c657 100644 --- a/plugin/src/main/scala/chisel3/internal/plugin/ChiselPlugin.scala +++ b/plugin/src/main/scala/chisel3/internal/plugin/ChiselPlugin.scala @@ -8,10 +8,13 @@ import nsc.plugins.{Plugin, PluginComponent} import scala.reflect.internal.util.NoPosition import scala.collection.mutable -private[plugin] case class ChiselPluginArguments(val skipFiles: mutable.HashSet[String] = mutable.HashSet.empty) { +private[plugin] case class ChiselPluginArguments( + val skipFiles: mutable.HashSet[String] = mutable.HashSet.empty, + var genBundleElements: Boolean = false) { def useBundlePluginOpt = "useBundlePlugin" def useBundlePluginFullOpt = s"-P:${ChiselPlugin.name}:$useBundlePluginOpt" - + def genBundleElementsOpt = "genBundleElements" + def genBundleElementsFullOpt = s"-P:${ChiselPlugin.name}:$genBundleElementsOpt" // Annoying because this shouldn't be used by users def skipFilePluginOpt = "INTERNALskipFile:" def skipFilePluginFullOpt = s"-P:${ChiselPlugin.name}:$skipFilePluginOpt" @@ -20,7 +23,7 @@ private[plugin] case class ChiselPluginArguments(val skipFiles: mutable.HashSet[ object ChiselPlugin { val name = "chiselplugin" - // Also logs why the compoennt was not run + // Also logs why the component was not run private[plugin] def runComponent( global: Global, arguments: ChiselPluginArguments @@ -67,11 +70,12 @@ class ChiselPlugin(val global: Global) extends Plugin { // Be annoying and warn because users are not supposed to use this val msg = s"Option -P:${ChiselPlugin.name}:$option should only be used for internal chisel3 compiler purposes!" global.reporter.warning(NoPosition, msg) + } else if (option == arguments.genBundleElementsOpt) { + arguments.genBundleElements = true } else { error(s"Option not understood: '$option'") } } true } - } diff --git a/src/test/scala/chiselTests/BundleElementsSpec.scala b/src/test/scala/chiselTests/BundleElementsSpec.scala new file mode 100644 index 00000000..fab2e733 --- /dev/null +++ b/src/test/scala/chiselTests/BundleElementsSpec.scala @@ -0,0 +1,564 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chiselTests + +import chisel3._ +import chisel3.experimental.{ChiselEnum, FixedPoint} +import chisel3.stage.ChiselStage +import chisel3.util.Decoupled +import org.scalatest.exceptions.TestFailedException +import org.scalatest.freespec.AnyFreeSpec +import org.scalatest.matchers.should.Matchers + +import scala.language.reflectiveCalls + +/* Rich and complicated bundle examples + */ +trait BpipSuperTraitWithField { + val bpipSuperTraitGood = SInt(17.W) + def bpipSuperTraitBad = SInt(22.W) +} + +trait BpipTraitWithField extends BpipSuperTraitWithField { + val bpipTraitGood = SInt(17.W) + def bpipTraitBad = SInt(22.W) +} + +class BpipOneField extends Bundle with BpipTraitWithField { + val bpipOneFieldOne = SInt(8.W) + val bpipOneFieldTwo = SInt(8.W) +} + +class BpipTwoField extends BpipOneField { + val bpipTwoFieldOne = SInt(8.W) + val bpipTwoFieldTwo = Vec(4, UInt(12.W)) + val myInt = 7 + val baz = Decoupled(UInt(16.W)) +} + +class BpipDecoupled extends BpipOneField { + val bpipDecoupledSInt = SInt(8.W) + val bpipDecoupledVec = Vec(4, UInt(12.W)) + val bpipDecoupledDecoupled = Decoupled(UInt(16.W)) +} + +class HasDecoupledBundleByInheritance extends Module { + val out1 = IO(Output(new BpipDecoupled)) + assertElementsMatchExpected(out1)( + "bpipDecoupledDecoupled" -> _.bpipDecoupledDecoupled, + "bpipDecoupledVec" -> _.bpipDecoupledVec, + "bpipDecoupledSInt" -> _.bpipDecoupledSInt, + "bpipOneFieldTwo" -> _.bpipOneFieldTwo, + "bpipOneFieldOne" -> _.bpipOneFieldOne, + "bpipTraitGood" -> _.bpipTraitGood, + "bpipSuperTraitGood" -> _.bpipSuperTraitGood + ) +} + +/* plugin should not affect the seq detection */ +class DebugProblem3 extends Module { + val out1 = IO(Output(new BpipTwoField)) + assertElementsMatchExpected(out1)( + "baz" -> _.baz, + "bpipTwoFieldTwo" -> _.bpipTwoFieldTwo, + "bpipTwoFieldOne" -> _.bpipTwoFieldOne, + "bpipOneFieldTwo" -> _.bpipOneFieldTwo, + "bpipOneFieldOne" -> _.bpipOneFieldOne, + "bpipTraitGood" -> _.bpipTraitGood, + "bpipSuperTraitGood" -> _.bpipSuperTraitGood + ) +} + +class BpipBadSeqBundle extends Bundle { + val bpipBadSeqBundleGood = UInt(999.W) + val bpipBadSeqBundleBad = Seq(UInt(16.W), UInt(8.W), UInt(4.W)) +} + +class HasBadSeqBundle extends Module { + val out1 = IO(Output(new BpipBadSeqBundle)) +} + +class BpipBadSeqBundleWithIgnore extends Bundle with IgnoreSeqInBundle { + val goodFieldWithIgnore = UInt(999.W) + val badSeqFieldWithIgnore = Seq(UInt(16.W), UInt(8.W), UInt(4.W)) +} + +class UsesIgnoreSeqInBundle extends Module { + val out1 = IO(Output(new BpipBadSeqBundleWithIgnore)) +} + +/* This is mostly a test of the field order */ +class BpipP8_1 extends Bundle { + val field_1_1 = UInt(11.W) + val field_1_2 = UInt(12.W) +} + +class BpipP8_2 extends BpipP8_1 { + val field_2_1 = UInt(11.W) + val field_2_2 = UInt(12.W) +} + +class BpipP8_3 extends BpipP8_2 { + val field_3_1 = UInt(11.W) + val field_3_2 = UInt(12.W) +} + +/* plugin should not affect the seq detection */ +class ForFieldOrderingTest extends Module { + val out1 = IO(Output(new BpipP8_3)) + out1 := DontCare + assertElementsMatchExpected(out1)( + "field_3_2" -> _.field_3_2, + "field_3_1" -> _.field_3_1, + "field_2_2" -> _.field_2_2, + "field_2_1" -> _.field_2_1, + "field_1_2" -> _.field_1_2, + "field_1_1" -> _.field_1_1 + ) +} + +/* plugin should allow parameter var fields */ +class HasValParamsToBundle extends Module { + // The following block does not work, suggesting that ParamIsField is not a case we need to solve + class BpipParamIsField0(val paramField0: UInt) extends Bundle + class BpipParamIsField1(val paramField1: UInt) extends BpipParamIsField0(UInt(66.W)) + + val out3 = IO(Output(new BpipParamIsField1(UInt(10.W)))) + val out4 = IO(Output(new BpipParamIsField1(UInt(10.W)))) + out3 := DontCare + assertElementsMatchExpected(out3)("paramField0" -> _.paramField0, "paramField1" -> _.paramField1) + assertElementsMatchExpected(out4)("paramField0" -> _.paramField0, "paramField1" -> _.paramField1) +} + +class HasGenParamsPassedToSuperclasses extends Module { + + class OtherBundle extends Bundle { + val otherField = UInt(55.W) + } + + class BpipWithGen[T <: Data, TT <: Data](gen: T, gen2: => TT) extends Bundle { + val superFoo = gen + val superQux = gen2 + } + +// class BpipDemoBundle[T <: Data](gen: T, gen2: => T) extends BpipTwoField with BpipVarmint { + class BpipUsesWithGen[T <: Data](gen: T, gen2: => T) extends BpipWithGen(gen, gen2) { + // val foo = gen + val bar = Bool() + val qux = gen2 + val bad = 444 + val baz = Decoupled(UInt(16.W)) + } + + val out1 = IO(Output(new BpipUsesWithGen(UInt(4.W), new OtherBundle))) + val out2 = IO(Output(new BpipUsesWithGen(UInt(4.W), FixedPoint(10.W, 4.BP)))) + + out1 := DontCare + + assertElementsMatchExpected(out1)( + "baz" -> _.baz, + "qux" -> _.qux, + "bar" -> _.bar, + "superQux" -> _.superQux, + "superFoo" -> _.superFoo + ) + assertElementsMatchExpected(out2)( + "baz" -> _.baz, + "qux" -> _.qux, + "bar" -> _.bar, + "superQux" -> _.superQux, + "superFoo" -> _.superFoo + ) +} + +/* Testing whether gen fields superFoo and superQux can be found when they are + * declared in a superclass + * + */ +class UsesGenFiedldsInSuperClass extends Module { + class BpipWithGen[T <: Data](gen: T) extends Bundle { + val superFoo = gen + val superQux = gen + } + + class BpipUsesWithGen[T <: Data](gen: T) extends BpipWithGen(gen) {} + + val out = IO(Output(new BpipUsesWithGen(UInt(4.W)))) + + out := DontCare + + assertElementsMatchExpected(out)() +} + +/* Testing whether gen fields superFoo and superQux can be found when they are + * declared in a superclass + * + */ +class BpipBadBundleWithHardware extends Bundle { + val bpipWithHardwareGood = UInt(8.W) + val bpipWithHardwareBad = 244.U(16.W) +} + +class HasHardwareFieldsInBundle extends Module { + val out = IO(Output(new BpipBadBundleWithHardware)) + assertElementsMatchExpected(out)() +} + +/* This is legal because of => + */ +class UsesBundleWithGeneratorField extends Module { + class BpipWithGen[T <: Data](gen: => T) extends Bundle { + val superFoo = gen + val superQux = gen + } + + class BpipUsesWithGen[T <: Data](gen: => T) extends BpipWithGen(gen) + + val out = IO(Output(new BpipUsesWithGen(UInt(4.W)))) + + out := DontCare + + assertElementsMatchExpected(out)("superQux" -> _.superQux, "superFoo" -> _.superFoo) +} + +/* Testing whether gen fields superFoo and superQux can be found when they are + * declared in a superclass + * + */ +class BundleElementsSpec extends AnyFreeSpec with Matchers { + + /** Tests a whole bunch of different Bundle constructions + */ + class BpipIsComplexBundle extends Module { + + trait BpipVarmint { + val varmint = Bool() + + def vermin = Bool() + + private val puppy = Bool() + } + + abstract class BpipAbstractBundle extends Bundle { + def doNothing: Unit + + val fromAbstractBundle = UInt(22.W) + } + + class BpipOneField extends Bundle { + val fieldOne = SInt(8.W) + } + + class BpipTwoField extends BpipOneField { + val fieldTwo = SInt(8.W) + val fieldThree = Vec(4, UInt(12.W)) + } + + class BpipAnimalBundle(w1: Int, w2: Int) extends Bundle { + val dog = SInt(w1.W) + val fox = UInt(w2.W) + } + + class BpipDemoBundle[T <: Data](gen: T, gen2: => T) extends BpipTwoField with BpipVarmint { + val foo = gen + val bar = Bool() + val qux = gen2 + val bad = 44 + val baz = Decoupled(UInt(16.W)) + val animals = new BpipAnimalBundle(4, 8) + } + + val out = IO(Output(new BpipDemoBundle(UInt(4.W), FixedPoint(10.W, 4.BP)))) + + val out2 = IO(Output(new BpipAbstractBundle { + override def doNothing: Unit = () + + val notAbstract: Bool = Bool() + })) + + val out4 = IO(Output(new BpipAnimalBundle(99, 100))) + val out5 = IO(Output(new BpipTwoField)) + + out := DontCare + out5 := DontCare + + assertElementsMatchExpected(out)( + "animals" -> _.animals, + "baz" -> _.baz, + "qux" -> _.qux, + "bar" -> _.bar, + "varmint" -> _.varmint, + "fieldThree" -> _.fieldThree, + "fieldTwo" -> _.fieldTwo, + "fieldOne" -> _.fieldOne, + "foo" -> _.foo + ) + assertElementsMatchExpected(out5)("fieldThree" -> _.fieldThree, "fieldTwo" -> _.fieldTwo, "fieldOne" -> _.fieldOne) + assertElementsMatchExpected(out2)("notAbstract" -> _.notAbstract, "fromAbstractBundle" -> _.fromAbstractBundle) + assertElementsMatchExpected(out4)("fox" -> _.fox, "dog" -> _.dog) + } + + "Complex Bundle with inheritance, traits and params. DebugProblem1" in { + ChiselStage.emitFirrtl(new BpipIsComplexBundle) + } + + "Decoupled Bundle with inheritance" in { + ChiselStage.emitFirrtl(new HasDecoupledBundleByInheritance) + } + + "Simple bundle inheritance. DebugProblem3" in { + ChiselStage.emitFirrtl(new DebugProblem3) + } + + "Bundles containing Seq[Data] should be compile erorr. HasBadSeqBundle" in { + intercept[ChiselException] { + ChiselStage.emitFirrtl(new HasBadSeqBundle) + } + } + + "IgnoreSeqInBundle allows Seq[Data] in bundle" in { + ChiselStage.emitFirrtl(new UsesIgnoreSeqInBundle) + } + + "Simple field ordering test." in { + ChiselStage.emitFirrtl(new ForFieldOrderingTest) + } + + "Val params to Bundle should be an Exception." in { + ChiselStage.emitFirrtl(new HasValParamsToBundle) + } + + "Should handle gen params passed to superclasses" in { + ChiselStage.emitFirrtl(new HasGenParamsPassedToSuperclasses) + } + + "Aliased fields should be detected and throw an exception, because gen: Data, with no =>" in { + intercept[AliasedAggregateFieldException] { + ChiselStage.emitFirrtl(new UsesGenFiedldsInSuperClass) + } + } + + "Error when bundle fields are hardware, such as literal values. HasHardwareFieldsInBundle" in { + val e = intercept[ExpectedChiselTypeException] { + ChiselStage.emitFirrtl(new HasHardwareFieldsInBundle) + } + e.getMessage should include( + "Bundle: BpipBadBundleWithHardware contains hardware fields: bpipWithHardwareBad: UInt<16>(244)" + ) + } + + "Aliased fields not created when using gen: => Data" in { + ChiselStage.emitFirrtl(new UsesBundleWithGeneratorField) + } + + class OptionBundle(val hasIn: Boolean) extends Bundle { + val in = if (hasIn) { + Some(Input(Bool())) + } else { + None + } + val out = Output(Bool()) + } + + class UsesBundleWithOptionFields extends Module { + val outTrue = IO(Output(new OptionBundle(hasIn = true))) + val outFalse = IO(Output(new OptionBundle(hasIn = false))) + //NOTE: The _.in.get _.in is an optional field + assertElementsMatchExpected(outTrue)("out" -> _.out, "in" -> _.in.get) + assertElementsMatchExpected(outFalse)("out" -> _.out) + } + + "plugin should work with Bundles with Option fields" in { + ChiselStage.emitFirrtl(new UsesBundleWithOptionFields) + } + + "plugin should work with enums in bundles" in { + object Enum0 extends ChiselEnum { + val s0, s1, s2 = Value + } + + ChiselStage.emitFirrtl(new Module { + val out = IO(Output(new Bundle { + val a = UInt(8.W) + val b = Bool() + val c = Enum0.Type + })) + assertElementsMatchExpected(out)("b" -> _.b, "a" -> _.a) + }) + } + + "plugin will NOT see fields that are Data but declared in some way as Any" in { + //This is not incompatible with chisel not using the plugin, but this code is considered bad practice + + ChiselStage.emitFirrtl(new Module { + val out = IO(Output(new Bundle { + val a = UInt(8.W) + val b: Any = Bool() + })) + + intercept[TestFailedException] { + assert(out.elements.keys.exists(_ == "b")) + } + }) + } + + "plugin tests should fail properly in the following cases" - { + + class BundleAB extends Bundle { + val a = Output(UInt(8.W)) + val b = Output(Bool()) + } + + def checkAssertion(checks: (BundleAB => (String, Data))*)(expectedMessage: String): Unit = { + intercept[AssertionError] { + ChiselStage.emitFirrtl(new Module { + val out = IO(new BundleAB) + assertElementsMatchExpected(out)(checks: _*) + }) + }.getMessage should include(expectedMessage) + } + + "one of the expected data values is wrong" in { + checkAssertion("b" -> _.b, "a" -> _.b)("field 'a' data field BundleElementsSpec_Anon.out.a") + } + + "one of the expected field names in wrong" in { + checkAssertion("b" -> _.b, "z" -> _.a)("field: 'a' did not match expected 'z'") + } + + "fields that are expected are not returned by the elements method" in { + checkAssertion("b" -> _.b, "a" -> _.a, "c" -> _.a)("#elements is missing the 'c' field") + } + + "fields returned by the element are not specified in the expected fields" in { + checkAssertion("b" -> _.b)("expected fields did not include 'a' field found in #elements") + } + + "multiple errors between elements method and expected fields are shown in the assertion error message" in { + checkAssertion()( + "expected fields did not include 'b' field found in #elements," + + " expected fields did not include 'a' field found in #elements" + ) + } + } + + "plugin should error correctly when bundles contain only a Option field" in { + ChiselStage.emitFirrtl(new Module { + val io = IO(new Bundle { + val foo = Input(UInt(8.W)) + val x = new Bundle { + val y = if (false) Some(Input(UInt(8.W))) else None + } + }) + assertElementsMatchExpected(io)("x" -> _.x, "foo" -> _.foo) + assertElementsMatchExpected(io.x)() + }) + } + + "plugin should handle fields using the boolean to option construct" in { + case class ALUConfig( + xLen: Int, + mul: Boolean, + b: Boolean) + + class OptionalBundle extends Bundle { + val optionBundleA = Input(UInt(3.W)) + val optionBundleB = Input(UInt(7.W)) + } + + class ALU(c: ALUConfig) extends Module { + + class BpipOptionBundle extends Bundle with IgnoreSeqInBundle { + val bpipUIntVal = Input(UInt(8.W)) + lazy val bpipUIntLazyVal = Input(UInt(8.W)) + var bpipUIntVar = Input(UInt(8.W)) + + def bpipUIntDef = Input(UInt(8.W)) + + val bpipOptionUInt = Some(Input(UInt(16.W))) + val bpipOptionOfBundle = if (c.b) Some(new OptionalBundle) else None + val bpipSeqData = Seq(UInt(8.W), UInt(8.W)) + } + + val io = IO(new BpipOptionBundle) + assertElementsMatchExpected(io)( + "bpipUIntLazyVal" -> _.bpipUIntLazyVal, + "bpipOptionUInt" -> _.bpipOptionUInt.get, + "bpipUIntVar" -> _.bpipUIntVar, + "bpipUIntVal" -> _.bpipUIntVal + ) + } + + ChiselStage.emitFirrtl(new ALU(ALUConfig(10, mul = true, b = false))) + } + + "TraceSpec test, different version found in TraceSpec.scala" in { + class Bundle0 extends Bundle { + val a = UInt(8.W) + val b = Bool() + val c = Enum0.Type + } + + class Bundle1 extends Bundle { + val a = new Bundle0 + val b = Vec(4, Vec(4, Bool())) + } + + class Module0 extends Module { + val i = IO(Input(new Bundle1)) + val o = IO(Output(new Bundle1)) + val r = Reg(new Bundle1) + o := r + r := i + + assertElementsMatchExpected(i)("b" -> _.b, "a" -> _.a) + assertElementsMatchExpected(o)("b" -> _.b, "a" -> _.a) + assertElementsMatchExpected(r)("b" -> _.b, "a" -> _.a) + } + + class Module1 extends Module { + val i = IO(Input(new Bundle1)) + val m0 = Module(new Module0) + m0.i := i + m0.o := DontCare + assertElementsMatchExpected(i)("b" -> _.b, "a" -> _.a) + } + + object Enum0 extends ChiselEnum { + val s0, s1, s2 = Value + } + + ChiselStage.emitFirrtl(new Module1) + } +} +/* Checks that the elements method of a bundle matches the testers idea of what the bundle field names and their + * associated data match and that they have the same number of fields in the same order + */ +object assertElementsMatchExpected { + def apply[T <: Bundle](bun: T)(checks: (T => (String, Data))*): Unit = { + val expected = checks.map { fn => fn(bun) } + val elements = bun.elements + val missingMsg = "missing field in #elements" + val extraMsg = "extra field in #elements" + val paired = elements.toSeq.zipAll(expected, missingMsg -> UInt(1.W), extraMsg -> UInt(1.W)) + val errorsStrings = paired.flatMap { + case (element, expected) => + val (elementName, elementData) = element + val (expectedName, expectedData) = expected + if (elementName == missingMsg) { + Some(s"#elements is missing the '$expectedName' field") + } else if (expectedName == extraMsg) { + Some(s"expected fields did not include '$elementName' field found in #elements") + } else if (elementName != expectedName) { + Some(s"field: '$elementName' did not match expected '$expectedName'") + } else if (elementData != expectedData) { + Some( + s"field '$elementName' data field ${elementData}(${elementData.hashCode}) did not match expected $expectedData(${expectedData.hashCode})" + ) + } else { + None + } + } + assert(errorsStrings.isEmpty, s"Bundle: ${bun.getClass.getName}: " + errorsStrings.mkString(", ")) + } +} diff --git a/src/test/scala/chiselTests/BundleSpec.scala b/src/test/scala/chiselTests/BundleSpec.scala index 720f877f..5dcbbefa 100644 --- a/src/test/scala/chiselTests/BundleSpec.scala +++ b/src/test/scala/chiselTests/BundleSpec.scala @@ -26,6 +26,10 @@ trait BundleSpecUtils { val bar = Seq(UInt(16.W), UInt(8.W), UInt(4.W)) } + class BadSeqBundleWithIgnoreSeqInBundle extends Bundle with IgnoreSeqInBundle { + val bar = Seq(UInt(16.W), UInt(8.W), UInt(4.W)) + } + class MyModule(output: Bundle, input: Bundle) extends Module { val io = IO(new Bundle { val in = Input(input) @@ -87,7 +91,7 @@ class BundleSpec extends ChiselFlatSpec with BundleSpecUtils with Utils { new BasicTester { val m = Module(new Module { val io = IO(new Bundle { - val b = new BadSeqBundle with IgnoreSeqInBundle + val b = new BadSeqBundleWithIgnoreSeqInBundle }) }) stop() @@ -141,7 +145,7 @@ class BundleSpec extends ChiselFlatSpec with BundleSpecUtils with Utils { out := in } } - }).getMessage should include("must be a Chisel type, not hardware") + }).getMessage should include("MyBundle contains hardware fields: foo: UInt<7>(123)") } "Bundles" should "not recursively contain aggregates with bound hardware" in { (the[ChiselException] thrownBy extractCause[ChiselException] { @@ -153,7 +157,7 @@ class BundleSpec extends ChiselFlatSpec with BundleSpecUtils with Utils { out := in } } - }).getMessage should include("must be a Chisel type, not hardware") + }).getMessage should include("Bundle: MyBundle contains hardware fields: foo: BundleSpec_Anon.out") } "Unbound bundles sharing a field" should "not error" in { ChiselStage.elaborate { -- cgit v1.2.3