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 --- plugin/README.md | 99 +++++++++++ .../chisel3/internal/plugin/BundleComponent.scala | 190 +++++++++++++++++---- .../chisel3/internal/plugin/ChiselPlugin.scala | 12 +- 3 files changed, 262 insertions(+), 39 deletions(-) create mode 100644 plugin/README.md (limited to 'plugin') 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 } - } -- cgit v1.2.3 From 8776e58ff91cd88562b957d7a09322ec16610b81 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 3 Feb 2022 04:23:48 +0000 Subject: Tweak Bundle._elementsImpl (#2390) (#2392) * Change type of Bundle._elementsImpl to Iterable It was previously SeqMap (ListMap on Scala 2.12). This change gives us more freedom to optimize the implementation without breaking binary compatibility. It is scala.collection.Iterable because it is perfectly fine to return mutable collections (like Arrays) since the only use is to Iterate on them. * Disallow users implementing Bundle._elementsImpl Currently, it would result in a runtime linkage error. This turns it into a compile-time error. Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 1b05a14ad6d5784f3b91ab510dc1095423c23ea8) Co-authored-by: Jack Koenig --- .../chisel3/internal/plugin/BundleComponent.scala | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'plugin') diff --git a/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala b/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala index e92bbb23..d768175d 100644 --- a/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala +++ b/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala @@ -41,12 +41,12 @@ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPlugi def inferType(t: Tree): Type = localTyper.typed(t, nsc.Mode.TYPEmode).tpe - 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]") + 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 itStringAnyTpe: Type = inferType(tq"scala.collection.Iterable[(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 } @@ -95,6 +95,9 @@ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPlugi case d: DefDef if isNullaryMethodNamed("_cloneTypeImpl", d) => val msg = "Users cannot override _cloneTypeImpl. Let the compiler plugin generate it." global.globalError(d.pos, msg) + case d: DefDef if isNullaryMethodNamed("_elementsImpl", d) => + val msg = "Users cannot override _elementsImpl. Let the compiler plugin generate it." + global.globalError(d.pos, msg) case d: DefDef if isNullaryMethodNamed("_usingPlugin", d) => val msg = "Users cannot override _usingPlugin, it is for the compiler plugin's use only." global.globalError(d.pos, msg) @@ -220,10 +223,10 @@ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPlugi val elementsImplSym = bundle.symbol.newMethod(TermName("_elementsImpl"), bundle.symbol.pos.focus, Flag.OVERRIDE | Flag.PROTECTED) elementsImplSym.resetFlag(Flags.METHOD) - elementsImplSym.setInfo(NullaryMethodType(seqMapTpe)) + elementsImplSym.setInfo(NullaryMethodType(itStringAnyTpe)) val elementsImpl = localTyper.typed( - DefDef(elementsImplSym, q"scala.collection.immutable.SeqMap.apply[String, Any](..$elementArgs)") + DefDef(elementsImplSym, q"scala.collection.immutable.Vector.apply[(String, Any)](..$elementArgs)") ) Some(elementsImpl) -- cgit v1.2.3