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