From 33bdea461c22d9941b061bab3d4a86f4b16d01f7 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Tue, 2 Jun 2020 16:40:10 -0400 Subject: [PATCH 1/8] Refactor RemoveKeywordCollisions->ManipulateNames Rewrite of RemoveKeywordCollisions into a more generic ManipulateNames. The new ManipulateNames transform is abstract in a '(String, Namespace) => String' method that can be used for arbitrary manipulation of names in a circuit. The software architecture remains mostly the same (a rename map is used as the underlying data store). However, the new ManipulateNames used Target as opposed to Named. Add the ability for naming to be selectively enabled or disabled via: - ManipulateNamesAllowlistAnnotation - ManipulateNamesBlocklistAnnotation Signed-off-by: Schuyler Eldridge --- .../firrtl/transforms/ManipulateNames.scala | 408 ++++++++++++++++++ .../transforms/RemoveKeywordCollisions.scala | 216 +--------- 2 files changed, 415 insertions(+), 209 deletions(-) create mode 100644 src/main/scala/firrtl/transforms/ManipulateNames.scala diff --git a/src/main/scala/firrtl/transforms/ManipulateNames.scala b/src/main/scala/firrtl/transforms/ManipulateNames.scala new file mode 100644 index 00000000000..51c0a06f220 --- /dev/null +++ b/src/main/scala/firrtl/transforms/ManipulateNames.scala @@ -0,0 +1,408 @@ +// See LICENSE for license details. + +package firrtl.transforms + +import firrtl._ +import firrtl.analyses.InstanceGraph +import firrtl.Mappers._ + +import firrtl.annotations.{ + CircuitTarget, + CompleteTarget, + InstanceTarget, + ModuleTarget, + MultiTargetAnnotation, + ReferenceTarget, + Target, + TargetToken +} +import firrtl.options.Dependency +import firrtl.stage.Forms +import firrtl.stage.TransformManager.TransformDependency + +import scala.collection.mutable +import scala.reflect.ClassTag + +/** Base trait for annotations that control the behavior of transforms that sub-class ManipulateNames + * @see [[ManipulateNamesBlocklistAnnotation]] + * @see [[ManipulateNamesAllowlistAnnotation]] + * @define noteLocalTargets All targets must be local. Name modification in a non-local target (e.g., a node in a + * specific instance) makes no structural modification and will be ignored during deduplication. If you want this + * behavior, use a combination of a sub-class of this annotation and a [[firrtl.transforms.NoDedupAnnotation + * NoDedupAnnotation]]. + */ +sealed trait ManipulateNamesListAnnotation[A <: ManipulateNames[_]] extends MultiTargetAnnotation { + + def transform: Dependency[A] + + /* Throw an exception if targets are non-local */ + targets.flatten.collect { + case a if !a.isLocal => a + } match { + case Nil => + case a => + val aString = a.map(_.serialize).mkString("\n - ", "\n - ", "") + throw new IllegalArgumentException(s"""'${this.getClass.getName}' given non-local targets: $aString""") + } + +} + +/** Annotation to prevent name manipulation of[[firrtl.annotations.Target Target]]s in a transform that subclasses + * [[ManipulateNames]]. All listed targets will not be modified. + * + * @param targets FIRRTL IR targets to exclude from name manipulation + * @param transform the transform that this should apply to + * @tparam A a sub-type of [[ManipulateNames]] + * @throws java.lang.IllegalArgumentException if any non-local targets are given + * @note $noteLocalTargets + */ +case class ManipulateNamesBlocklistAnnotation[A <: ManipulateNames[_]]( + targets: Seq[Seq[Target]], + transform: Dependency[A]) extends ManipulateNamesListAnnotation[A] { + + override def duplicate(a: Seq[Seq[Target]]) = this.copy(targets = a) + +} + +/** Annotation to filter name manipulation to only manipulate specific [[firrtl.annotations.Target Target]]s in a + * transform that subclasses [[ManipulateNames]]. Targets will be renamed if they are not listed in a + * [[ManipulateNamesBlocklistAnnotation]] and also listed in this annotation. + * + * Not providing a [[ManipulateNamesAllowlistAnnotation]] means that all targets in a circuit may be renamed. + * + * @param targets FIRRTL IR targets to include in name manipulation + * @param transform the transform that this should apply to + * @tparam A a sub-type of [[ManipulateNames]] + * @throws java.lang.IllegalArgumentException if any non-local targets are given + * @note $noteLocalTargets + */ +case class ManipulateNamesAllowlistAnnotation[A <: ManipulateNames[_]]( + targets: Seq[Seq[Target]], + transform: Dependency[A]) extends ManipulateNamesListAnnotation[A] { + + override def duplicate(a: Seq[Seq[Target]]) = this.copy(targets = a) + +} + +/** A datastructure used to do single-pass name manipulation + * @param circuit the [[ir.Circuit]] that will be manipulated + * @param renames a rename map + * @param block a function that returns true if a [[firrtl.annotations.Target Target]] should not be renamed + * @param allow a function that returns true if a [[firrtl.annotations.Target Target]] should be renamed + */ +private class RenameDataStructure( + circuit: ir.Circuit, + val renames: RenameMap, + val block: Target => Boolean, + val allow: Target => Boolean) { + + /** A mapping of targets to associated namespaces */ + val namespaces: mutable.HashMap[CompleteTarget, Namespace] = + mutable.HashMap(CircuitTarget(circuit.main) -> Namespace(circuit)) + + /** A mapping of a reference to either an instance or a memory (encoded as a [[ReferenceTarget]] */ + val instanceMap: mutable.HashMap[ReferenceTarget, Either[ReferenceTarget, InstanceTarget]] = + new mutable.HashMap[ReferenceTarget, Either[ReferenceTarget, InstanceTarget]] { + override def apply(a: ReferenceTarget) = try { + super.apply(a) + } catch { + case t: NoSuchElementException => throw new FirrtlUserException( + s"""|Reference target '${a.serialize}' did not exist in mapping of reference targets to insts/mems. + | This is indicative of a circuit that has not been run through LowerTypes.""".stripMargin, t) + } + } + + /** Return true if a target should be skipped based on allow and block parameters */ + def skip(a: Target): Boolean = block(a) || !allow(a) + +} + +/** Transform for manipulate all the names in a FIRRTL circuit. + * @tparam A the type of the child transform + */ +abstract class ManipulateNames[A <: ManipulateNames[_] : ClassTag] extends Transform with DependencyAPIMigration { + + /** A function used to manipulate a name in a FIRRTL circuit */ + def manipulate: (String, Namespace) => Option[String] + + override def prerequisites: Seq[TransformDependency] = Seq(Dependency(firrtl.passes.LowerTypes)) + override def optionalPrerequisites: Seq[TransformDependency] = Seq.empty + override def optionalPrerequisiteOf: Seq[TransformDependency] = Forms.LowEmitters + override def invalidates(a: Transform) = a match { + case _: analyses.GetNamespace => true + case _ => false + } + + /** Compute a new name for some target and record the rename if the new name differs. If the top module or the circuit + * is renamed, both will be renamed. + * @param name the string to rename + * @param r a data structure containing information necessary for renaming + * @param target the target associated with the name + * @return a new name (or the old name if no renaming is necessary) + */ + private def doRename(name: String, r: RenameDataStructure, target: CompleteTarget): String = { + /* Compute the new name and, if the name is a new name, a new target. */ + val (namex: String, ax: Option[CompleteTarget]) = target match { + /* Do not rename if this is designated as a skip */ + case a if r.skip(a) => + (name, None) + /* Circuit renaming */ + case a@ CircuitTarget(b) => manipulate(b, r.namespaces(a)) match { + case Some(str) => (str, Some(a.copy(circuit = str))) + case None => (b, None) + } + /* Module renaming for non-top modules */ + case a@ ModuleTarget(_, b) => manipulate(b, r.namespaces(a.circuitTarget)) match { + case Some(str) => (str, Some(a.copy(module = str))) + case None => (b, None) + } + /* Instance renaming */ + case a@ InstanceTarget(_, _, Nil, b, c) => manipulate(b, r.namespaces(a.moduleTarget)) match { + case Some(str) => (str, Some(a.copy(instance = str))) + case None => (b, None) + } + /* Rename either a module component or a memory */ + case a@ ReferenceTarget(_, _, _, b, Nil) => manipulate(b, r.namespaces(a.moduleTarget)) match { + case Some(str) => (str, Some(a.copy(ref = str))) + case None => (b, None) + } + /* Rename an instance port or a memory reader/writer/readwriter */ + case a@ ReferenceTarget(_, _, _, b, (token@ TargetToken.Field(c)) :: Nil) => + val ref = r.instanceMap(a.moduleTarget.ref(b)) match { + case Right(inst) => inst.ofModuleTarget + case Left(mem) => mem + } + manipulate(c, r.namespaces(ref)) match { + case Some(str) => (str, Some(a.copy(component = Seq(token.copy(str))))) + case None => (c, None) + } + } + /* Record the optional rename. If the circuit was renamed, also rename the top module. If the top module was + * renamed, also rename the circuit. */ + ax.foreach( + axx => target match { + case c: CircuitTarget => + r.renames.rename(target, r.renames(axx)) + r.renames.rename(c.module(c.circuit), CircuitTarget(namex).module(namex)) + /* Note: this code path is not exercised by the implementation of the [[run]] and [[onModule]] methods. Those + * only use [[doRename]] on the circuit and [[maybeRename]] on the top module. + */ + case m: ModuleTarget if m.module == m.circuit => + r.renames.rename(target, r.renames(axx)) + r.renames.rename(m.circuitTarget, axx.circuitTarget) + case _ => + r.renames.rename(target, r.renames(axx)) + } + ) + namex + } + + /** Change a name based on known renames. Do not record any new renames. + * @param name the string to rename + * @param r a data structure containing information necessary for renaming + * @param target the target associated with the name + * @return a new name (or the old name if no renaming is necessary) + */ + private def maybeRename(name: String, r: RenameDataStructure, t: CompleteTarget): String = + r.renames.underlying.get(t) match { + case Some(ax) if ax.size == 1 => + ax match { + case Seq(foo: CircuitTarget) => foo.name + case Seq(foo: ModuleTarget) => foo.module + case Seq(foo: InstanceTarget) => foo.instance + case Seq(foo: ReferenceTarget) => foo.tokens.last match { + case TargetToken.Ref(value) => value + case TargetToken.Field(value) => value + case _ => Utils.throwInternalError( + s"""|Reference target '${t.serialize}'must end in 'Ref' or 'Field' + | This is indicative of a circuit that has not been run through LowerTypes.""", + Some(new MatchError(foo.serialize))) + } + } + case s@ Some(ax) => Utils.throwInternalError( + s"""Found multiple renames '${t}' -> [${ax.map(_.serialize).mkString(",")}]. This should be impossible.""", + Some(new MatchError(s))) + case None => name + } + + /** Rename an expression + * + * This logic exploits the known structure of the output of [[LowerTypes]] such that the only possible expressions in + * a module are: (1) references to module components, (2) subfields of references are instance components, and (3) + * subfields of subfields or references are memory ports. + */ + private def onExpression(e: ir.Expression, r: RenameDataStructure, t: ModuleTarget): ir.Expression = e match { + /* A reference to something inside this module */ + case w: WRef => w.copy(name = maybeRename(w.name, r, Target.asTarget(t)(w))) + /* This is either the subfield of an instance or a subfield of a memory reader/writer/readwriter */ + case w@ WSubField(expr, ref, _, _) => expr match { + /* This is an instance */ + case we@ WRef(inst, _, _, _) => + val tx = Target.asTarget(t)(we) + val (rTarget: ReferenceTarget, iTarget: InstanceTarget) = r.instanceMap(tx) match { + case Right(a) => (a.ofModuleTarget.ref(ref), a) + case a@ Left(ref) => throw new FirrtlUserException( + s"""|Unexpected '${ref.serialize}' in instanceMap for key '${tx.serialize}' on expression '${w.serialize}'. + | This is indicative of a circuit that has not been run through LowerTypes.""", new MatchError(a)) + } + w.copy(we.copy(name=maybeRename(inst, r, iTarget)), name=maybeRename(ref, r, rTarget)) + /* This is a reader/writer/readwriter */ + case ws@ WSubField(expr, port, _, _) => expr match { + /* This is the memory. */ + case wr@ WRef(mem, _, _, _) => + w.copy( + expr=ws.copy( + expr=wr.copy(name=maybeRename(mem, r, t.ref(mem))), + name=maybeRename(port, r, t.ref(mem).field(port)))) + } + } + case e => e.map(onExpression(_: ir.Expression, r, t)) + } + + /** Rename a statement + * + * Instances will update the rename data structure. Memories are treated specially to rename their readers, writers, + * and readwriters. + */ + private def onStatement(s: ir.Statement, r: RenameDataStructure, t: ModuleTarget): ir.Statement = s match { + case decl: ir.IsDeclaration => decl match { + case decl@ WDefInstance(_, inst, mod, _) => + val modx = maybeRename(mod, r, t.circuitTarget.module(mod)) + val instx = doRename(inst, r, t.instOf(inst, mod)) + r.instanceMap(t.ref(inst)) = Right(t.instOf(inst, mod)) + decl.copy(name = instx, module = modx) + case decl: ir.DefMemory => + val namex = doRename(decl.name, r, t.ref(decl.name)) + val tx = t.ref(decl.name) + r.namespaces(tx) = Namespace(decl.readers ++ decl.writers ++ decl.readwriters) + r.instanceMap(tx) = Left(tx) + decl + .copy( + name = namex, + readers = decl.readers.map(_r => doRename(_r, r, tx.field(_r))), + writers = decl.writers.map(_w => doRename(_w, r, tx.field(_w))), + readwriters = decl.readwriters.map(_rw => doRename(_rw, r, tx.field(_rw))) + ) + .map(onExpression(_: ir.Expression, r, t)) + case decl => + decl + .map(doRename(_: String, r, t.ref(decl.name))) + .map(onExpression(_: ir.Expression, r, t)) + } + case s => + s + .map(onStatement(_: ir.Statement, r, t)) + .map(onExpression(_: ir.Expression, r, t)) + } + + /** Rename a port */ + private def onPort(p: ir.Port, r: RenameDataStructure, t: ModuleTarget): ir.Port = { + p.map(doRename(_: String, r, t.ref(p.name))) + } + + /** Rename a [[DefModule]] and it's internals (ports and statements) to fix keyword collisions and update instance + * references to respect previous renames + * @param renames a [[RenameMap]] + * @param circuit the enclosing [[CircuitName]] + * @return a [[DefModule]] without keyword conflicts + */ + private def onModule(m: ir.DefModule, r: RenameDataStructure, t: CircuitTarget): ir.DefModule = m match { + case _: ir.ExtModule => m + case ir.Module(_, main, _, _) => + val moduleTarget = t.module(m.name) + r.namespaces(moduleTarget) = Namespace(m) + + /* If top module, use [[maybeRename]]: circuit renaming already recorded a top-module rename if one should happen. + * Otherwise, use [[doRename]]: compute a new name and record it. + */ + val onName: String => String = t.circuit match { + case `main` => maybeRename(_, r, moduleTarget) + case _ => doRename(_, r, moduleTarget) + } + + m + .map(onName) + .map(onPort(_: ir.Port, r, moduleTarget)) + .map(onStatement(_: ir.Statement, r, moduleTarget)) + } + + /** Manipulate all names in a circuit + * + * @param c an input circuit + * @param renames a rename map that will be updated as names are manipulated + * @param block a function that returns true if a [[firrtl.annotations.Target Target]] should not be renamed + * @param allow a function that returns true if a [[firrtl.annotations.Target Target]] should be renamed + * @return the circuit with manipulated names + */ + def run( + c: ir.Circuit, + renames: RenameMap, + block: Target => Boolean, + allow: Target => Boolean) + : ir.Circuit = { + val t = CircuitTarget(c.main) + + /* If the circuit is a skip, return the original circuit. Otherwise, walk all the modules and rename them. Rename the + * circuit if the main module was renamed. + */ + (block(t), !allow(t)) match { + case (true, _) => + logger.info(s"Circuit '${t.serialize}' is excluded by the 'block' parameter. No renaming will occur.") + c + case (false, true) => + logger.info(s"Circuit '${t.serialize}' is not included by the 'allow' parameter. No renaming will occur.") + c + case _ => + val r = new RenameDataStructure(c, renames, block, allow) + + /* Record a rename for the circuit if the top module will be renamed. This allows all the module renames to be + * aware of the circuit rename when generating their own renames. E.g., this allows renames to be generated + * that can be resolved in a single step: + * ~foo -> FOO + * ~foo|bar -> ~FOO|BAR + * Instead of renames which require multiple steps: + * ~foo -> FOO + * ~foo|bar -> ~foo|BAR + */ + val mainx = r.skip(t.module(c.main)) match { + case true => c.main + case false => + val tx = CircuitTarget(doRename(c.main, r, t)) + logger.info(s"Main module will be renamed. Renaming circuit: '${t.serialize}' -> ['${tx.serialize}']") + renames.record(t, tx) + tx.circuit + } + + /* Rename all modules from leafs to root in one pass while updating a shared rename map. Going from leafs to + * roots ensures that the rename map is safe for parents to blindly consult. Store this in mapping of old module + * target to new module to allow the modules to be put in the old order. + */ + val modulesx: Map[ModuleTarget, ir.DefModule] = new InstanceGraph(c).moduleOrder.reverse + .map(m => t.module(m.name) -> onModule(m, r, t)) + .toMap + + /* Replace the old modules making sure that they are still in the same order */ + c.copy(modules = c.modules.map(m => modulesx(t.module(m.name))), + main = mainx) + } + } + + /** Return a circuit state with all sensitive names manipulated */ + def execute(state: CircuitState): CircuitState = { + + val block = state.annotations.collect { + case ManipulateNamesBlocklistAnnotation(targetSeq, _: Dependency[A]) => targetSeq + }.flatten.flatten.toSet + + val allow = state.annotations.collect { + case ManipulateNamesAllowlistAnnotation(targetSeq, _: Dependency[A]) => targetSeq + } match { + case Nil => (a: Target) => true + case a => a.flatten.flatten.toSet + } + + val renames = RenameMap() + state.copy(circuit = run(state.circuit, renames, block, allow), renames = Some(renames)) + } + +} diff --git a/src/main/scala/firrtl/transforms/RemoveKeywordCollisions.scala b/src/main/scala/firrtl/transforms/RemoveKeywordCollisions.scala index c5f20363afd..840a3d99060 100644 --- a/src/main/scala/firrtl/transforms/RemoveKeywordCollisions.scala +++ b/src/main/scala/firrtl/transforms/RemoveKeywordCollisions.scala @@ -4,230 +4,28 @@ package firrtl.transforms import firrtl._ -import firrtl.analyses.InstanceGraph -import firrtl.annotations.{Named, CircuitName, ModuleName, ComponentName} -import firrtl.ir -import firrtl.passes.{Uniquify, PassException} import firrtl.Utils.v_keywords -import firrtl.Mappers._ import firrtl.options.Dependency - -import scala.collection.mutable +import firrtl.passes.Uniquify /** Transform that removes collisions with reserved keywords * @param keywords a set of reserved words - * @define implicitRename @param renames the [[RenameMap]] to query when renaming - * @define implicitNamespace @param ns an encolosing [[Namespace]] with which new names must not conflict - * @define implicitScope @param scope the enclosing scope of this name. If [[None]], then this is a [[Circuit]] name */ -class RemoveKeywordCollisions(keywords: Set[String]) extends Transform with DependencyAPIMigration { - private type ModuleType = mutable.HashMap[String, ir.Type] +class RemoveKeywordCollisions(keywords: Set[String]) extends ManipulateNames { + private val inlineDelim = "_" /** Generate a new name, by appending underscores, that will not conflict with the existing namespace * @param n a name * @param ns a [[Namespace]] - * @return a conflict-free name + * @return Some name if a rename occurred, None otherwise * @note prefix uniqueness is not respected */ - private def safeName(n: String, ns: Namespace): String = - Uniquify.findValidPrefix(n + inlineDelim, Seq(""), ns.cloneUnderlying ++ keywords) - - /** Modify a name to not conflict with a Verilog keywords while respecting existing renames and a namespace - * @param n the name to rename - * @param renames the [[RenameMap]] to query when renaming - * $implicitRename - * $implicitNamespace - * $implicitScope - * @return a name without keyword conflicts - */ - private def onName(n: String)(implicit renames: RenameMap, ns: Namespace, scope: Option[Named]): String = { - - // Convert a [[String]] into [[Named]] based on the provided scope. - def wrap(name: String, scope: Option[Named]): Named = scope match { - case None => CircuitName(name) - case Some(cir: CircuitName) => ModuleName(name, cir) - case Some(mod: ModuleName) => ComponentName(name, mod) - case Some(com: ComponentName) => ComponentName(s"${com.name}.$name", com.module) - } - - val named = wrap(n, scope) - - // If this has already been renamed use that name. If it conflicts with a keyword, determine a new, safe name and - // update the renames. Otherwise, leave it alone. - val namedx: Seq[Named] = renames.get(named) match { - case Some(x) => x - case None if keywords(n) => - val sn = wrap(safeName(n, ns), scope) - renames.rename(named, sn) - Seq(sn) - case _ => Seq(wrap(n, scope)) - } - - namedx match { - case Seq(ComponentName(n, _)) => n - case Seq(ModuleName(n, _)) => n - case Seq(CircuitName(n)) => n - case x => throw new PassException( - s"Verilog renaming shouldn't result in multiple renames, but found '$named -> $namedx'") - } - } - - /** Rename the fields of a [[Type]] to match the ports of an instance - * @param t the type to rename - * $implicitRename - * $implicitNamespace - * $implicitScope - * @return a [[Type]] with updated names - * @note This is not intended for fixing arbitrary types, only [[BundleType]] in instance [[WRef]]s - */ - private def onType(t: ir.Type) - (implicit renames: RenameMap, - ns: Namespace, - scope: Option[ModuleName]): ir.Type = t match { - case b: ir.BundleType => b.copy(fields = b.fields.map(f => f.copy(name = onName(f.name)))) - case _ => t - } - - /** Rename an [[Expression]] to respect existing renames and avoid keyword collisions - * @param e the [[Expression]] to rename - * $implicitRename - * $implicitNamespace - * $implicitScope - * @return an [[Expression]] without keyword conflicts - */ - private def onExpression(e: ir.Expression) - (implicit renames: RenameMap, - ns: Namespace, - scope: Option[ModuleName], - iToM: mutable.Map[ComponentName, ModuleName], - modType: ModuleType): ir.Expression = e match { - case wsf@ WSubField(wr@ WRef(name, _, InstanceKind, _), port, _, _) => - val subInst = ComponentName(name, scope.get) - val subModule = iToM(subInst) - val subPort = ComponentName(port, subModule) - - val wrx = wr.copy( - name = renames.get(subInst).orElse(Some(Seq(subInst))).get.head.name, - tpe = modType(subModule.name)) - - wsf.copy( - expr = wrx, - name = renames.get(subPort).orElse(Some(Seq(subPort))).get.head.name) - case wr: WRef => wr.copy(name=onName(wr.name)) - case ex => ex.map(onExpression) - } - - /** Rename a [[Statement]] to respect existing renames and avoid keyword collisions - * $implicitRename - * $implicitNamespace - * $implicitScope - * @return a [[Statement]] without keyword conflicts - */ - private def onStatement(s: ir.Statement) - (implicit renames: RenameMap, - ns: Namespace, - scope: Option[ModuleName], - iToM: mutable.Map[ComponentName, ModuleName], - modType: ModuleType): ir.Statement = s match { - case wdi: WDefInstance => - val subModule = ModuleName(wdi.module, scope.get.circuit) - val modulex = renames.get(subModule).orElse(Some(Seq(subModule))).get.head.name - val wdix = wdi.copy(module = modulex, - name = onName(wdi.name), - tpe = onType(wdi.tpe)(renames, ns, Some(ModuleName(modulex, scope.get.circuit)))) - iToM(ComponentName(wdi.name, scope.get)) = ModuleName(wdix.module, scope.get.circuit) - wdix - case _ => s - .map(onStatement) - .map(onExpression) - .map(onName) - } - - /** Rename a [[Port]] to avoid keyword collisions - * $implicitRename - * $implicitNamespace - * $implicitScope - * @return a [[Port]] without keyword conflicts - */ - private def onPort(p: ir.Port)(implicit renames: RenameMap, ns: Namespace, scope: Option[ModuleName]): ir.Port = - p.copy(name = onName(p.name)) - - /** Rename a [[DefModule]] and it's internals (ports and statements) to fix keyword collisions and update instance - * references to respect previous renames - * @param renames a [[RenameMap]] - * @param circuit the enclosing [[CircuitName]] - * @return a [[DefModule]] without keyword conflicts - */ - private def onModule(renames: RenameMap, - circuit: CircuitName, - modType: ModuleType) - (m: ir.DefModule): ir.DefModule = { - implicit val moduleNamespace: Namespace = Namespace(m) - implicit val scope: Option[ModuleName] = Some(ModuleName(m.name, circuit)) - implicit val r: RenameMap = renames - implicit val mType: ModuleType = modType - - // Store local renames of refs to instances to their renamed modules. This is needed when renaming port connections - // on subfields where only the local instance name is available. - implicit val iToM: mutable.Map[ComponentName, ModuleName] = mutable.Map.empty - - val mx = m - .map(onPort) - .map(onStatement) - .map(onName(_: String)(renames, moduleNamespace, Some(circuit))) - - // Must happen after renaming the name and ports of the module itself - mType += (mx.name -> onType(Utils.module_type(mx))) - mx - } - - /** Fix any Verilog keyword collisions in a [[firrtl.ir Circuit]] - * @param c a [[firrtl.ir Circuit]] with possible name collisions - * @param renames a [[RenameMap]] to update. If you don't want to propagate renames, this can be ignored. - * @return a [[firrtl.ir Circuit]] without keyword conflicts - */ - def run(c: ir.Circuit, renames: RenameMap = RenameMap()): ir.Circuit = { - implicit val circuitNamespace: Namespace = Namespace(c) - implicit val scope: Option[CircuitName] = Some(CircuitName(c.main)) - val modType: ModuleType = new ModuleType() - - // Rename all modules from leafs to root in one pass while updating a shared rename map. Going from leafs to roots - // ensures that the rename map is safe for parents to blindly consult. - val modulesx: Map[ModuleName, Seq[ir.DefModule]] = new InstanceGraph(c).moduleOrder.reverse - .map(onModule(renames, scope.get, modType)) - .groupBy(m => ModuleName(m.name, scope.get)) - - // Reorder the renamed modules into the original circuit order. - val modulesxx: Seq[ir.DefModule] = c.modules.flatMap{ orig => - val named = ModuleName(orig.name, scope.get) - modulesx(renames.get(named).orElse(Some(Seq(named))).get.head) - } - - // Rename the circuit if the top module was renamed - val mainx = renames.get(ModuleName(c.main, CircuitName(c.main))) match { - case Some(Seq(ModuleName(m, _))) => - renames.rename(CircuitName(c.main), CircuitName(m)) - m - case x@ Some(_) => throw new PassException( - s"Verilog renaming shouldn't result in multiple renames, but found '${c.main} -> $x'") - case None => - c.main - } - - // Apply all updates - c.copy(modules = modulesxx, main = mainx) + override def manipulate = (n: String, ns: Namespace) => keywords.contains(n) match { + case true => Some(Uniquify.findValidPrefix(n + inlineDelim, Seq(""), ns.cloneUnderlying ++ keywords)) + case false => None } - /** Fix any Verilog keyword name collisions in a [[CircuitState]] while propagating renames - * @param state the [[CircuitState]] with possible name collisions - * @return a [[CircuitState]] without name collisions - */ - def execute(state: CircuitState): CircuitState = { - val renames = RenameMap() - renames.setCircuit(state.circuit.main) - state.copy(circuit = run(state.circuit, renames), renames = Some(renames)) - } } /** Transform that removes collisions with Verilog keywords */ From 4c0ab1892c683d5b96708d39a239eee214e702b8 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Tue, 16 Jun 2020 21:18:18 -0400 Subject: [PATCH 2/8] Add ManipulateNamesAllowlistResultAnnotation Add a new annotation that stores the resulting name of an allowlist name to be manipulated. Signed-off-by: Schuyler Eldridge --- .../firrtl/transforms/ManipulateNames.scala | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/src/main/scala/firrtl/transforms/ManipulateNames.scala b/src/main/scala/firrtl/transforms/ManipulateNames.scala index 51c0a06f220..956b39e6d2e 100644 --- a/src/main/scala/firrtl/transforms/ManipulateNames.scala +++ b/src/main/scala/firrtl/transforms/ManipulateNames.scala @@ -84,6 +84,47 @@ case class ManipulateNamesAllowlistAnnotation[A <: ManipulateNames[_]]( } +/** Records the result of name changes for any targets included in a [[ManipulateNamesAllowlistAnnotation]] + * + * If targets are later removed, then a target and old target will be removed from this annotation. If all targets are + * removed, then this annotation will be deleted. + * + * @param targets the new targets + * @param transform the transform that performed this rename + * @param oldTargets the old targets + */ +case class ManipulateNamesAllowlistResultAnnotation[A <: ManipulateNames[_]]( + targets: Seq[Seq[Target]], + transform: Dependency[A], + oldTargets: Seq[Seq[Target]]) extends MultiTargetAnnotation { + + override def duplicate(a: Seq[Seq[Target]]) = this.copy(targets = a) + + override def update(renames: RenameMap) = { + val (targetsx, oldTargetsx) = targets.zip(oldTargets).foldLeft((Seq.empty[Seq[Target]], Seq.empty[Seq[Target]])) { + case ((accT, accO), (t, o)) => t.flatMap(renames(_)) match { + /* If the target was deleted, delete the old target */ + case tx if tx.isEmpty => (accT, accO) + case tx => (Seq(tx) ++ accT, Seq(o) ++ accO) + } + } + targetsx match { + /* If all targets were deleted, delete the annotation */ + case Nil => Seq.empty + case _ => Seq(this.copy(targets = targetsx, oldTargets = oldTargetsx)) + } + } + + /** Return [[firrtl.RenameMap RenameMap]] from old targets to new targets */ + def toRenameMap: RenameMap = { + val m = oldTargets.zip(targets).flatMap { + case (a, b) => a.map(_ -> b) + }.toMap.asInstanceOf[Map[CompleteTarget, Seq[CompleteTarget]]] + RenameMap.create(m) + } + +} + /** A datastructure used to do single-pass name manipulation * @param circuit the [[ir.Circuit]] that will be manipulated * @param renames a rename map @@ -402,7 +443,19 @@ abstract class ManipulateNames[A <: ManipulateNames[_] : ClassTag] extends Trans } val renames = RenameMap() - state.copy(circuit = run(state.circuit, renames, block, allow), renames = Some(renames)) + val circuitx = run(state.circuit, renames, block, allow) + + val annotationsx = state.annotations.flatMap { + /* Consume blocklist annotations */ + case ManipulateNamesBlocklistAnnotation(_, _: Dependency[A]) => None + /* Convert allowlist annotations to result annotations */ + case ManipulateNamesAllowlistAnnotation(a, t: Dependency[A]) => (a, a.map(_.map(renames(_)).flatten)) match { + case (a, b) => Some(ManipulateNamesAllowlistResultAnnotation(b, t, a)) + } + case a => Some(a) + } + + state.copy(circuit = circuitx, annotations = annotationsx, renames = Some(renames)) } } From 305e9da16392f5ce8c3532469eea271c1d0b6654 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Tue, 2 Jun 2020 16:43:02 -0400 Subject: [PATCH 3/8] Test ManipulateNamesSpec Add tests for the ManipulateNames transform. Signed-off-by: Schuyler Eldridge --- .../transforms/ManipulateNamesSpec.scala | 176 ++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 src/test/scala/firrtlTests/transforms/ManipulateNamesSpec.scala diff --git a/src/test/scala/firrtlTests/transforms/ManipulateNamesSpec.scala b/src/test/scala/firrtlTests/transforms/ManipulateNamesSpec.scala new file mode 100644 index 00000000000..82cc997dde8 --- /dev/null +++ b/src/test/scala/firrtlTests/transforms/ManipulateNamesSpec.scala @@ -0,0 +1,176 @@ +// See LICENSE for license details. + +package firrtlTests.transforms + +import firrtl.{ir, CircuitState, FirrtlUserException, Namespace, Parser} +import firrtl.annotations.CircuitTarget +import firrtl.options.Dependency +import firrtl.testutils.FirrtlCheckers._ +import firrtl.transforms.{ + ManipulateNames, + ManipulateNamesBlocklistAnnotation, + ManipulateNamesAllowlistAnnotation +} + +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +object ManipulateNamesSpec { + + class AddPrefix extends ManipulateNames { + override def manipulate = (a: String, b: Namespace) => Some(b.newName("prefix_" + a)) + } + +} + +class ManipulateNamesSpec extends AnyFlatSpec with Matchers { + + import ManipulateNamesSpec._ + + class CircuitFixture { + protected val input = + """|circuit Foo: + | module Bar: + | node a = UInt<1>(0) + | module Foo: + | inst bar of Bar + | inst bar2 of Bar + |""".stripMargin + val `~Foo` = CircuitTarget("Foo") + val `~Foo|Foo` = `~Foo`.module("Foo") + val `~Foo|Foo/bar:Bar` = `~Foo|Foo`.instOf("bar", "Bar") + val `~Foo|Foo/bar2:Bar` = `~Foo|Foo`.instOf("bar2", "Bar") + val `~Foo|Bar` = `~Foo`.module("Bar") + val `~Foo|Bar>a` = `~Foo|Bar`.ref("a") + val tm = new firrtl.stage.transforms.Compiler(Seq(Dependency[AddPrefix])) + } + + behavior of "ManipulateNames" + + it should "rename everything by default" in new CircuitFixture { + val state = CircuitState(Parser.parse(input), Seq.empty) + val statex = tm.execute(state) + val expected: Seq[PartialFunction[Any, Boolean]] = Seq( + { case ir.Circuit(_, _, "prefix_Foo") => true }, + { case ir.Module(_, "prefix_Foo", _, _) => true}, + { case ir.Module(_, "prefix_Bar", _, _) => true} + ) + expected.foreach(statex should containTree (_)) + } + + it should "do nothing if the circuit is blocklisted" in new CircuitFixture { + val annotations = Seq(ManipulateNamesBlocklistAnnotation(Seq(Seq(`~Foo`)), Dependency[AddPrefix])) + val state = CircuitState(Parser.parse(input), annotations) + val statex = tm.execute(state) + state.circuit.serialize should be (statex.circuit.serialize) + } + + it should "not rename the circuit if the top module is blocklisted" in new CircuitFixture { + val annotations = Seq(ManipulateNamesBlocklistAnnotation(Seq(Seq(`~Foo|Foo`)), Dependency[AddPrefix])) + val state = CircuitState(Parser.parse(input), annotations) + val expected: Seq[PartialFunction[Any, Boolean]] = Seq( + { case ir.Circuit(_, _, "Foo") => true }, + { case ir.Module(_, "Foo", _, _) => true}, + { case ir.Module(_, "prefix_Bar", _, _) => true} + ) + val statex = tm.execute(state) + expected.foreach(statex should containTree (_)) + } + + it should "not rename instances if blocklisted" in new CircuitFixture { + val annotations = Seq(ManipulateNamesBlocklistAnnotation(Seq(Seq(`~Foo|Foo/bar:Bar`)), Dependency[AddPrefix])) + val state = CircuitState(Parser.parse(input), annotations) + val expected: Seq[PartialFunction[Any, Boolean]] = Seq( + { case ir.DefInstance(_, "bar", "prefix_Bar", _) => true}, + { case ir.Module(_, "prefix_Bar", _, _) => true} + ) + val statex = tm.execute(state) + expected.foreach(statex should containTree (_)) + } + + it should "do nothing if the circuit is not allowlisted" in new CircuitFixture { + val annotations = Seq( + ManipulateNamesAllowlistAnnotation(Seq(Seq(`~Foo|Foo`)), Dependency[AddPrefix]) + ) + val state = CircuitState(Parser.parse(input), annotations) + val statex = tm.execute(state) + state.circuit.serialize should be (statex.circuit.serialize) + } + + it should "rename only the circuit if allowlisted" in new CircuitFixture { + val annotations = Seq( + ManipulateNamesAllowlistAnnotation(Seq(Seq(`~Foo`)), Dependency[AddPrefix]), + ManipulateNamesAllowlistAnnotation(Seq(Seq(`~Foo|Foo`)), Dependency[AddPrefix]) + ) + val state = CircuitState(Parser.parse(input), annotations) + val statex = tm.execute(state) + val expected: Seq[PartialFunction[Any, Boolean]] = Seq( + { case ir.Circuit(_, _, "prefix_Foo") => true }, + { case ir.Module(_, "prefix_Foo", _, _) => true}, + { case ir.DefInstance(_, "bar", "Bar", _) => true}, + { case ir.DefInstance(_, "bar2", "Bar", _) => true}, + { case ir.Module(_, "Bar", _, _) => true}, + { case ir.DefNode(_, "a", _) => true} + ) + expected.foreach(statex should containTree (_)) + } + + it should "rename an instance via allowlisting" in new CircuitFixture { + val annotations = Seq( + ManipulateNamesAllowlistAnnotation(Seq(Seq(`~Foo`)), Dependency[AddPrefix]), + ManipulateNamesAllowlistAnnotation(Seq(Seq(`~Foo|Foo/bar:Bar`)), Dependency[AddPrefix]) + ) + val state = CircuitState(Parser.parse(input), annotations) + val statex = tm.execute(state) + val expected: Seq[PartialFunction[Any, Boolean]] = Seq( + { case ir.Circuit(_, _, "Foo") => true }, + { case ir.Module(_, "Foo", _, _) => true}, + { case ir.DefInstance(_, "prefix_bar", "Bar", _) => true}, + { case ir.DefInstance(_, "bar2", "Bar", _) => true}, + { case ir.Module(_, "Bar", _, _) => true}, + { case ir.DefNode(_, "a", _) => true} + ) + expected.foreach(statex should containTree (_)) + } + + it should "rename a node via allowlisting" in new CircuitFixture { + val annotations = Seq( + ManipulateNamesAllowlistAnnotation(Seq(Seq(`~Foo`)), Dependency[AddPrefix]), + ManipulateNamesAllowlistAnnotation(Seq(Seq(`~Foo|Bar>a`)), Dependency[AddPrefix]) + ) + val state = CircuitState(Parser.parse(input), annotations) + val statex = tm.execute(state) + val expected: Seq[PartialFunction[Any, Boolean]] = Seq( + { case ir.Circuit(_, _, "Foo") => true }, + { case ir.Module(_, "Foo", _, _) => true}, + { case ir.DefInstance(_, "bar", "Bar", _) => true}, + { case ir.DefInstance(_, "bar2", "Bar", _) => true}, + { case ir.Module(_, "Bar", _, _) => true}, + { case ir.DefNode(_, "prefix_a", _) => true} + ) + expected.foreach(statex should containTree (_)) + } + + it should "throw user errors on circuits that haven't been run through LowerTypes" in { + val input = + """|circuit Foo: + | module Foo: + | wire bar: {a: UInt<1>, b: UInt<1>} + | node baz = bar.a + |""".stripMargin + val state = CircuitState(Parser.parse(input), Seq.empty) + intercept [FirrtlUserException] { + (new AddPrefix).transform(state) + }.getMessage should include ("LowerTypes") + } + + behavior of "ManipulateNamesBlocklistAnnotation" + + it should "throw an exception if a non-local target is skipped" in new CircuitFixture { + val barA = CircuitTarget("Foo").module("Foo").instOf("bar", "Bar").ref("a") + assertThrows[java.lang.IllegalArgumentException]{ + Seq(ManipulateNamesBlocklistAnnotation(Seq(Seq(barA)), Dependency[AddPrefix])) + } + } + +} From 862ae2131a80d6a84423b17242612133f9ddec59 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Wed, 17 Jun 2020 17:15:47 -0400 Subject: [PATCH 4/8] Test ManipulateNamesAllowlistResultAnnotation Signed-off-by: Schuyler Eldridge --- .../transforms/ManipulateNamesSpec.scala | 57 ++++++++++++++++++- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/src/test/scala/firrtlTests/transforms/ManipulateNamesSpec.scala b/src/test/scala/firrtlTests/transforms/ManipulateNamesSpec.scala index 82cc997dde8..b1e2eeb94e8 100644 --- a/src/test/scala/firrtlTests/transforms/ManipulateNamesSpec.scala +++ b/src/test/scala/firrtlTests/transforms/ManipulateNamesSpec.scala @@ -2,14 +2,22 @@ package firrtlTests.transforms -import firrtl.{ir, CircuitState, FirrtlUserException, Namespace, Parser} +import firrtl.{ + ir, + CircuitState, + FirrtlUserException, + Namespace, + Parser, + RenameMap +} import firrtl.annotations.CircuitTarget import firrtl.options.Dependency import firrtl.testutils.FirrtlCheckers._ import firrtl.transforms.{ ManipulateNames, ManipulateNamesBlocklistAnnotation, - ManipulateNamesAllowlistAnnotation + ManipulateNamesAllowlistAnnotation, + ManipulateNamesAllowlistResultAnnotation } import org.scalatest.flatspec.AnyFlatSpec @@ -173,4 +181,49 @@ class ManipulateNamesSpec extends AnyFlatSpec with Matchers { } } + behavior of "ManipulateNamesAllowlistResultAnnotation" + + it should "delete itself if the new target is deleted" in { + val `~Foo|Bar` = CircuitTarget("Foo").module("Bar") + val `~Foo|prefix_Bar` = CircuitTarget("Foo").module("prefix_Bar") + + val a = ManipulateNamesAllowlistResultAnnotation( + targets = Seq(Seq(`~Foo|prefix_Bar`)), + transform = Dependency[AddPrefix], + oldTargets = Seq(Seq(`~Foo|Bar`)) + ) + + val r = RenameMap() + r.delete(`~Foo|prefix_Bar`) + + a.update(r) should be (empty) + } + + it should "drop a deleted target" in { + val `~Foo|Bar` = CircuitTarget("Foo").module("Bar") + val `~Foo|prefix_Bar` = CircuitTarget("Foo").module("prefix_Bar") + val `~Foo|Baz` = CircuitTarget("Foo").module("Baz") + val `~Foo|prefix_Baz` = CircuitTarget("Foo").module("prefix_Baz") + + val a = ManipulateNamesAllowlistResultAnnotation( + targets = Seq(Seq(`~Foo|prefix_Bar`), Seq(`~Foo|prefix_Baz`)), + transform = Dependency[AddPrefix], + oldTargets = Seq(Seq(`~Foo|Bar`), Seq(`~Foo|Baz`)) + ) + + val r = RenameMap() + r.delete(`~Foo|prefix_Bar`) + + val ax = a.update(r).collect { + case b: ManipulateNamesAllowlistResultAnnotation[_] => b + } + + ax should not be length (1) + + val keys = ax.head.toRenameMap.getUnderlying.keys + + keys should not contain (`~Foo|Bar`) + keys should contain (`~Foo|Baz`) + } + } From d93eef47c65e5d9f0b9942f9d22d754b776e42a3 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Wed, 20 May 2020 17:44:24 -0400 Subject: [PATCH 5/8] Add a second instance to Verilog keyword test Signed-off-by: Schuyler Eldridge --- .../firrtlTests/VerilogEmitterTests.scala | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/test/scala/firrtlTests/VerilogEmitterTests.scala b/src/test/scala/firrtlTests/VerilogEmitterTests.scala index ae06c3316f8..171bce7878e 100644 --- a/src/test/scala/firrtlTests/VerilogEmitterTests.scala +++ b/src/test/scala/firrtlTests/VerilogEmitterTests.scala @@ -420,27 +420,39 @@ class VerilogEmitterSpec extends FirrtlFlatSpec { | input always: UInt<1> | output always$: UInt<1> | inst assign of endmodule + | inst edge of endmodule_ | node always_ = not(always) | node always__ = and(always_, assign.fork) - | always$ <= always__ + | node always___ = and(always__, edge.fork) + | always$ <= always___ | module endmodule: | output fork: UInt<1> | node const = add(UInt<4>("h1"), UInt<3>("h2")) | fork <= const + | module endmodule_: + | output fork: UInt<1> + | node const = add(UInt<4>("h1"), UInt<3>("h1")) + | fork <= const |""".stripMargin val check_firrtl = """|circuit parameter_: | module parameter_: - | input always___: UInt<1> + | input always____: UInt<1> | output always$: UInt<1> - | inst assign_ of endmodule_ - | node always_ = not(always___) + | inst assign_ of endmodule__ + | inst edge_ of endmodule_ + | node always_ = not(always____) | node always__ = and(always_, assign_.fork_) - | always$ <= always__ - | module endmodule_: + | node always___ = and(always__, edge_.fork_) + | always$ <= always___ + | module endmodule__: | output fork_: UInt<1> | node const_ = add(UInt<4>("h1"), UInt<3>("h2")) | fork_ <= const_ + | module endmodule_: + | output fork_: UInt<1> + | node const_ = add(UInt<4>("h1"), UInt<3>("h1")) + | fork_ <= const_ |""".stripMargin val state = CircuitState(parse(input), UnknownForm, Seq.empty, None) val output = Seq( ToWorkingIR, ResolveKinds, InferTypes, new VerilogRename ) From 45835fa54d3d345428554f5459ca6776c2b9d90a Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Tue, 2 Jun 2020 16:43:37 -0400 Subject: [PATCH 6/8] Add LetterCaseTransforms This adds three new transforms: - (abstract) LetterCaseTransform parent of case manipulation - LowerCaseNames to lower case all names - UpperCaseNames to upper case all names Signed-off-by: Schuyler Eldridge --- .../firrtl/features/LetterCaseTransform.scala | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 src/main/scala/firrtl/features/LetterCaseTransform.scala diff --git a/src/main/scala/firrtl/features/LetterCaseTransform.scala b/src/main/scala/firrtl/features/LetterCaseTransform.scala new file mode 100644 index 00000000000..a6cd270af46 --- /dev/null +++ b/src/main/scala/firrtl/features/LetterCaseTransform.scala @@ -0,0 +1,29 @@ +// See LICENSE for license details. + +package firrtl.features + +import firrtl.Namespace +import firrtl.transforms.ManipulateNames + +import scala.reflect.ClassTag + +/** Parent of transforms that do change the letter case of names in a FIRRTL circuit */ +abstract class LetterCaseTransform[A <: ManipulateNames[_] : ClassTag] extends ManipulateNames[A] { + + protected def newName: String => String + + final def manipulate = (a: String, ns: Namespace) => newName(a) match { + case `a` => None + case b => Some(ns.newName(b)) + } +} + +/** Convert all FIRRTL names to lowercase */ +final class LowerCaseNames extends LetterCaseTransform[LowerCaseNames] { + override protected def newName = (a: String) => a.toLowerCase +} + +/** Convert all FIRRTL names to UPPERCASE */ +final class UpperCaseNames extends LetterCaseTransform[UpperCaseNames] { + override protected def newName = (a: String) => a.toUpperCase +} From 006df5b102df6252f004fce6800394691a1b1e74 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Tue, 2 Jun 2020 16:44:38 -0400 Subject: [PATCH 7/8] Test both LowerCaseNames and UpperCaseNames Signed-off-by: Schuyler Eldridge --- .../features/LetterCaseTransformSpec.scala | 168 ++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 src/test/scala/firrtlTests/features/LetterCaseTransformSpec.scala diff --git a/src/test/scala/firrtlTests/features/LetterCaseTransformSpec.scala b/src/test/scala/firrtlTests/features/LetterCaseTransformSpec.scala new file mode 100644 index 00000000000..e0053fa80a7 --- /dev/null +++ b/src/test/scala/firrtlTests/features/LetterCaseTransformSpec.scala @@ -0,0 +1,168 @@ +// See LICENSE for license details. + +package firrtlTests.features + +import firrtl.{ir, CircuitState, Parser, WDefInstance, WRef, WSubField} +import firrtl.annotations.{CircuitTarget, IsMember, SingleTargetAnnotation} +import firrtl.features.{LowerCaseNames, UpperCaseNames} +import firrtl.options.Dependency +import firrtl.transforms.ManipulateNamesBlocklistAnnotation +import firrtl.testutils.FirrtlCheckers._ + +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +class LetterCaseTransformSpec extends AnyFlatSpec with Matchers { + + case class TrackingAnnotation(val target: IsMember) extends SingleTargetAnnotation[IsMember] { + override def duplicate(a: IsMember) = this.copy(target=a) + } + + class CircuitFixture { + private val input = + """|circuit Foo: + | module Bar: + | output OuT: UInt<2> + | OuT <= UInt<2>(0) + | module Baz: + | output OuT: UInt<2> + | OuT <= UInt<2>(1) + | module baz: + | output OuT: UInt<2> + | OuT <= UInt<2>(2) + | extmodule Ext: + | output OuT: UInt<1> + | module Foo: + | input CLk: Clock + | input rst_P: UInt<1> + | input addr: UInt<8> + | node Bar = UInt<1>(0) + | reg baz: UInt<1>, CLk with: (reset => (rst_P, Bar)) + | wire QUX: UInt<1> + | QUX <= UInt<1>(0) + | node quuxQuux = UInt<1>(0) + | mem MeM : @[Source.scala 1:4] + | data-type => UInt<8> + | depth => 32 + | read-latency => 0 + | write-latency => 1 + | reader => Read + | writer => wRITE + | readwriter => rw + | read-under-write => undefined + | MeM.Read is invalid + | MeM.wRITE is invalid + | MeM.rw is invalid + | inst SuB1 of Bar + | inst SuB2 of Baz + | inst SuB3 of baz + | inst SuB4 of Ext + | node sub1 = UInt<1>(0) + | node corge_corge = SuB1.OuT + | node QuuzQuuz = and(SuB2.OuT, SuB3.OuT) + | node graultGrault = not(SuB4.OuT) + |""".stripMargin + + private val Foo = CircuitTarget("Foo") + private val Bar = Foo.module("Bar") + + val annotations = Seq(TrackingAnnotation(Foo.module("Foo").ref("MeM").field("wRITE")field("en")), + ManipulateNamesBlocklistAnnotation(Seq(Seq(Bar)), Dependency[LowerCaseNames])) + val state = CircuitState(Parser.parse(input), annotations) + } + + behavior of "LowerCaseNames" + + it should "change all names to lowercase" in new CircuitFixture { + val tm = new firrtl.stage.transforms.Compiler(Seq(firrtl.options.Dependency[LowerCaseNames])) + val statex = tm.execute(state) + val expected: Seq[PartialFunction[Any, Boolean]] = Seq( + { case ir.Circuit(_, _, "foo") => true }, + { case ir.Module(_, "foo", + Seq(ir.Port(_, "clk", _, _), ir.Port(_, "rst_p", _, _), ir.Port(_, "addr", _, _)), _) => true }, + /* Module "Bar" should be skipped via a ManipulateNamesBlocklistAnnotation */ + { case ir.Module(_, "Bar", Seq(ir.Port(_, "out", _, _)), _) => true }, + { case ir.Module(_, "baz_0", Seq(ir.Port(_, "out", _, _)), _) => true }, + { case ir.Module(_, "baz", Seq(ir.Port(_, "out", _, _)), _) => true }, + /* External module "Ext" is not renamed */ + { case ir.ExtModule(_, "Ext", Seq(ir.Port(_, "OuT", _, _)), _, _) => true }, + { case ir.DefNode(_, "bar", _) => true }, + { case ir.DefRegister(_, "baz", _, WRef("clk", _, _, _), WRef("rst_p", _, _, _), WRef("bar", _, _, _)) => true }, + { case ir.DefWire(_, "qux", _) => true }, + { case ir.Connect(_, WRef("qux", _, _, _), _) => true }, + { case ir.DefNode(_, "quuxquux", _) => true }, + { case ir.DefMemory(_, "mem", _, _, _, _, Seq("read"), Seq("write"), Seq("rw"), _) => true }, + /* Ports of memories should be ignored, but these are already lower case */ + { case ir.IsInvalid(_, WSubField(WSubField(WRef("mem", _, _, _), "read", _, _), "addr", _, _)) => true }, + { case ir.IsInvalid(_, WSubField(WSubField(WRef("mem", _, _, _), "write", _, _), "addr", _, _)) => true }, + { case ir.IsInvalid(_, WSubField(WSubField(WRef("mem", _, _, _), "rw", _, _), "addr", _, _)) => true }, + /* Module "Bar" was skipped via a ManipulateNamesBlocklistAnnotation. The instance "SuB1" is renamed to "sub1_0" + * because node "sub1" already exists. This differs from the upper case test. + */ + { case WDefInstance(_, "sub1_0", "Bar", _) => true }, + { case WDefInstance(_, "sub2", "baz_0", _) => true }, + { case WDefInstance(_, "sub3", "baz", _) => true }, + /* External module instance names are renamed */ + { case WDefInstance(_, "sub4", "Ext", _) => true }, + { case ir.DefNode(_, "sub1", _) => true }, + { case ir.DefNode(_, "corge_corge", WSubField(WRef("sub1_0", _, _, _), "out", _, _)) => true }, + { case ir.DefNode(_, "quuzquuz", + ir.DoPrim(_,Seq(WSubField(WRef("sub2", _, _, _), "out", _, _), + WSubField(WRef("sub3", _, _, _), "out", _, _)), _, _)) => true }, + /* References to external module ports are not renamed, e.g., OuT */ + { case ir.DefNode(_, "graultgrault", + ir.DoPrim(_, Seq(WSubField(WRef("sub4", _, _, _), "OuT", _, _)), _, _)) => true } + ) + expected.foreach( statex should containTree (_) ) + } + + behavior of "UpperCaseNames" + + it should "change all names to uppercase" in new CircuitFixture { + val tm = new firrtl.stage.transforms.Compiler(Seq(firrtl.options.Dependency[UpperCaseNames])) + val statex = tm.execute(state) + val expected: Seq[PartialFunction[Any, Boolean]] = Seq( + { case ir.Circuit(_, _, "FOO") => true }, + { case ir.Module(_, "FOO", + Seq(ir.Port(_, "CLK", _, _), ir.Port(_, "RST_P", _, _), ir.Port(_, "ADDR", _, _)), _) => true }, + /* Module "Bar" should be skipped via a ManipulateNamesBlocklistAnnotation */ + { case ir.Module(_, "Bar", Seq(ir.Port(_, "OUT", _, _)), _) => true }, + { case ir.Module(_, "BAZ", Seq(ir.Port(_, "OUT", _, _)), _) => true }, + { case ir.Module(_, "BAZ_0", Seq(ir.Port(_, "OUT", _, _)), _) => true }, + /* External module "Ext" is not renamed */ + { case ir.ExtModule(_, "Ext", Seq(ir.Port(_, "OuT", _, _)), _, _) => true }, + { case ir.DefNode(_, "BAR", _) => true }, + { case ir.DefRegister(_, "BAZ", _, WRef("CLK", _, _, _), WRef("RST_P", _, _, _), WRef("BAR", _, _, _)) => true }, + { case ir.DefWire(_, "QUX", _) => true }, + { case ir.Connect(_, WRef("QUX", _, _, _), _) => true }, + { case ir.DefNode(_, "QUUXQUUX", _) => true }, + { case ir.DefMemory(_, "MEM", _, _, _, _, Seq("READ"), Seq("WRITE"), Seq("RW"), _) => true }, + /* Ports of memories should be ignored while readers/writers are renamed, e.g., "Read" is converted to upper case + * while "addr" is not touched. + */ + { case ir.IsInvalid(_, WSubField(WSubField(WRef("MEM", _, _, _), "READ", _, _), "addr", _, _)) => true }, + { case ir.IsInvalid(_, WSubField(WSubField(WRef("MEM", _, _, _), "WRITE", _, _), "addr", _, _)) => true }, + { case ir.IsInvalid(_, WSubField(WSubField(WRef("MEM", _, _, _), "RW", _, _), "addr", _, _)) => true }, + /* Module "Bar" was skipped via a ManipulateNamesBlocklistAnnotation. The instance "SuB1" is renamed to "SUB1" + * because this statement occurs before the "sub1" node later. This differs from the lower case test. + */ + { case WDefInstance(_, "SUB1", "Bar", _) => true }, + /* Instance "SuB2" and "SuB3" switch their modules from the lower case test due to namespace behavior. */ + { case WDefInstance(_, "SUB2", "BAZ", _) => true }, + { case WDefInstance(_, "SUB3", "BAZ_0", _) => true }, + /* External module "Ext" was skipped via a ManipulateBlocklistAnnotation */ + { case WDefInstance(_, "SUB4", "Ext", _) => true }, + /* Node "sub1" becomes "SUB1_0" because instance "SuB1" already got the "SUB1" name. */ + { case ir.DefNode(_, "SUB1_0", _) => true }, + { case ir.DefNode(_, "CORGE_CORGE", WSubField(WRef("SUB1", _, _, _), "OUT", _, _)) => true }, + { case ir.DefNode(_, "QUUZQUUZ", + ir.DoPrim(_,Seq(WSubField(WRef("SUB2", _, _, _), "OUT", _, _), + WSubField(WRef("SUB3", _, _, _), "OUT", _, _)), _, _)) => true }, + /* References to external module ports are not renamed, e.g., "OuT" */ + { case ir.DefNode(_, "GRAULTGRAULT", + ir.DoPrim(_, Seq(WSubField(WRef("SUB4", _, _, _), "OuT", _, _)), _, _)) => true } + ) + expected.foreach( statex should containTree (_) ) + } + +} From 72ab5ef044dc79e12a3af29770349a33193415d4 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Mon, 11 May 2020 19:12:19 -0400 Subject: [PATCH 8/8] Add --change-name-case option Adds an options to the FIRRTL compiler command line to schedule the LowerCaseNames and UpperCaseNames transforms. Signed-off-by: Schuyler Eldridge --- src/main/scala/firrtl/stage/FirrtlAnnotations.scala | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala index 5be84bb9d42..1604e92e812 100644 --- a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala +++ b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala @@ -193,7 +193,18 @@ object RunFirrtlTransformAnnotation extends HasShellOptions { s"Unknown error when instantiating class $txName", e) }), helpText = "Run these transforms during compilation", shortOption = Some("fct"), - helpValueName = Some(".") ) ) + helpValueName = Some(".") ), + new ShellOption[String]( + longOption = "change-name-case", + toAnnotationSeq = _ match { + case "lower" => Seq(RunFirrtlTransformAnnotation(new firrtl.features.LowerCaseNames)) + case "upper" => Seq(RunFirrtlTransformAnnotation(new firrtl.features.UpperCaseNames)) + case a => throw new OptionsException(s"Unknown case '$a'. Did you misspell it?") + }, + helpText = "Convert all FIRRTL names to a specific case", + helpValueName = Some("") + ) + ) }