From 38a27869ac6be1d950b4a8fe089ed295b54c2003 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 16 Aug 2021 15:44:27 -0700 Subject: [PATCH 01/24] Multiprotobuf algorithm now pulls the top module from the protobufs --- src/main/scala/firrtl/proto/FromProto.scala | 24 +++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/main/scala/firrtl/proto/FromProto.scala b/src/main/scala/firrtl/proto/FromProto.scala index ed641eb2ae..3aa03927b8 100644 --- a/src/main/scala/firrtl/proto/FromProto.scala +++ b/src/main/scala/firrtl/proto/FromProto.scala @@ -33,6 +33,30 @@ object FromProto { proto.FromProto.convert(pb) } + /** Deserialize all the ProtoBuf representations of [[ir.Circuit]] in @directory + * + * @param dir directory containing ProtoBuf representation(s) + * @return Deserialized FIRRTL Circuit + */ + def fromDirectory(dir: String): ir.Circuit = { + val d = new File(dir) + val fileList = if (d.exists && d.isDirectory) { + d.listFiles.filter(_.isFile).toList + } else { + List[File]() + } + + val circuits = fileList.map(f => fromInputStream(new FileInputStream(f))) + val tops = circuits.map(c => c.main).distinct + + require(tops.length == 1, "Not all multi-ProtoBufs point to the same top") + + // Concatenate all modules together + val modules = circuits.flatMap(c => c.modules).distinct + + ir.Circuit(ir.NoInfo, modules, tops.head) + } + // Convert from ProtoBuf message repeated Statements to FIRRRTL Block private def compressStmts(stmts: scala.collection.Seq[ir.Statement]): ir.Statement = stmts match { case scala.collection.Seq() => ir.EmptyStmt From 4194f8684116532daf164d137210db962612d02b Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 16 Aug 2021 15:45:02 -0700 Subject: [PATCH 02/24] Remove double assignment compilation error --- .../src/main/scala/firrtl/benchmark/hot/PassBenchmark.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmark/src/main/scala/firrtl/benchmark/hot/PassBenchmark.scala b/benchmark/src/main/scala/firrtl/benchmark/hot/PassBenchmark.scala index 707c717021..cf72963a49 100644 --- a/benchmark/src/main/scala/firrtl/benchmark/hot/PassBenchmark.scala +++ b/benchmark/src/main/scala/firrtl/benchmark/hot/PassBenchmark.scala @@ -13,7 +13,7 @@ abstract class PassBenchmark(passFactory: () => Pass) extends App { val warmup = args(1).toInt val runs = args(2).toInt - val input = filenameToCircuit(inputFile) + val input = directoryToCircuit(inputFile) val inputState = CircuitState(input, ChirrtlForm) val manager = new TransformManager(passFactory().prerequisites) From 47c3a1bd0550f94d2e3a95f8b52ab245da3740ed Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 23 Aug 2021 11:35:24 -0700 Subject: [PATCH 03/24] Add compiler option to emit individual module protobufs --- src/main/scala/firrtl/Emitter.scala | 39 +++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/main/scala/firrtl/Emitter.scala b/src/main/scala/firrtl/Emitter.scala index 760e83fd54..d997620795 100644 --- a/src/main/scala/firrtl/Emitter.scala +++ b/src/main/scala/firrtl/Emitter.scala @@ -151,6 +151,45 @@ object EmitAllModulesAnnotation extends HasShellOptions { helpText = "Run the specified module emitter (one file per module)", shortOption = Some("e"), helpValueName = Some("") + ), + new ShellOption[String]( + longOption = "emit-module-protobufs", + toAnnotationSeq = (a: String) => + a match { + case "chirrtl" => + Seq( + RunFirrtlTransformAnnotation(new ProtoEmitter.Chirrtl), + EmitAllModulesAnnotation(classOf[ProtoEmitter.Chirrtl])) + case "mhigh" => + Seq( + RunFirrtlTransformAnnotation(new ProtoEmitter.MHigh), + EmitAllModulesAnnotation(classOf[ProtoEmitter.MHigh]) + ) + case "high" => + Seq( + RunFirrtlTransformAnnotation(new ProtoEmitter.High), + EmitAllModulesAnnotation(classOf[ProtoEmitter.High]) + ) + case "middle" => + Seq( + RunFirrtlTransformAnnotation(new ProtoEmitter.Middle), + EmitAllModulesAnnotation(classOf[ProtoEmitter.Middle]) + ) + case "low" => + Seq( + RunFirrtlTransformAnnotation(new ProtoEmitter.Low), + EmitAllModulesAnnotation(classOf[ProtoEmitter.Low] + )) + case "low-opt" => + Seq( + RunFirrtlTransformAnnotation(new ProtoEmitter.OptLow), + EmitAllModulesAnnotation(classOf[ProtoEmitter.Low]) + ) + case _ => throw new PhaseException(s"Unknown emitter '$a'! (Did you misspell it?)") + }, + helpText = "Run the specified module emitter (one protobuf per module)", + shortOption = Some("p"), + helpValueName = Some("") ) ) From abae2b23ab90be71507545ada9a0ee328e73d05c Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 23 Aug 2021 12:03:48 -0700 Subject: [PATCH 04/24] Scalafmt --- src/main/scala/firrtl/Emitter.scala | 10 +++--- src/main/scala/firrtl/proto/FromProto.scala | 1 + .../firrtl/stage/FirrtlAnnotations.scala | 36 ++++++++++++++++++- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/main/scala/firrtl/Emitter.scala b/src/main/scala/firrtl/Emitter.scala index d997620795..8e0572fba7 100644 --- a/src/main/scala/firrtl/Emitter.scala +++ b/src/main/scala/firrtl/Emitter.scala @@ -158,8 +158,9 @@ object EmitAllModulesAnnotation extends HasShellOptions { a match { case "chirrtl" => Seq( - RunFirrtlTransformAnnotation(new ProtoEmitter.Chirrtl), - EmitAllModulesAnnotation(classOf[ProtoEmitter.Chirrtl])) + RunFirrtlTransformAnnotation(new ProtoEmitter.Chirrtl), + EmitAllModulesAnnotation(classOf[ProtoEmitter.Chirrtl]) + ) case "mhigh" => Seq( RunFirrtlTransformAnnotation(new ProtoEmitter.MHigh), @@ -176,10 +177,7 @@ object EmitAllModulesAnnotation extends HasShellOptions { EmitAllModulesAnnotation(classOf[ProtoEmitter.Middle]) ) case "low" => - Seq( - RunFirrtlTransformAnnotation(new ProtoEmitter.Low), - EmitAllModulesAnnotation(classOf[ProtoEmitter.Low] - )) + Seq(RunFirrtlTransformAnnotation(new ProtoEmitter.Low), EmitAllModulesAnnotation(classOf[ProtoEmitter.Low])) case "low-opt" => Seq( RunFirrtlTransformAnnotation(new ProtoEmitter.OptLow), diff --git a/src/main/scala/firrtl/proto/FromProto.scala b/src/main/scala/firrtl/proto/FromProto.scala index 3aa03927b8..9966ede5e4 100644 --- a/src/main/scala/firrtl/proto/FromProto.scala +++ b/src/main/scala/firrtl/proto/FromProto.scala @@ -9,6 +9,7 @@ import collection.JavaConverters._ import FirrtlProtos._ import com.google.protobuf.CodedInputStream import Firrtl.Statement.{Formal, ReadUnderWrite} +import firrtl.ir.DefModule object FromProto { diff --git a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala index 9e6fefcae9..9afbfc7e1e 100644 --- a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala +++ b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala @@ -6,7 +6,7 @@ import firrtl._ import firrtl.ir.Circuit import firrtl.annotations.{Annotation, NoTargetAnnotation} import firrtl.options.{Dependency, HasShellOptions, OptionsException, ShellOption, Unserializable} -import java.io.FileNotFoundException +import java.io.{File, FileNotFoundException} import java.nio.file.NoSuchFileException import firrtl.stage.TransformManager.TransformDependency @@ -64,6 +64,40 @@ object FirrtlFileAnnotation extends HasShellOptions { } +/** Read a directory of FIRRTL files or ProtoBufs + * - set with `-I/--input-directory` + * - If unset, an [[FirrtlFileAnnotation]] with the default input file __will not be generated__ + * @param file input filename + */ +case class FirrtlDirectoryAnnotation(dir: String) extends NoTargetAnnotation with CircuitOption { + + def toCircuit(info: Parser.InfoMode): FirrtlCircuitAnnotation = { + val circuit = + try { + proto.FromProto.fromDirectory(dir) + } catch { + case a @ (_: FileNotFoundException | _: NoSuchFileException) => + throw new OptionsException(s"Directory '$dir' not found! (Did you misspell it?)", a) + } + FirrtlCircuitAnnotation(circuit) + } + +} + +object FirrtlDirectoryAnnotation extends HasShellOptions { + + val options = Seq( + new ShellOption[String]( + longOption = "input-directory", + toAnnotationSeq = a => Seq(FirrtlDirectoryAnnotation(a)), + helpText = "A directory of FIRRTL files", + shortOption = Some("I"), + helpValueName = Some("") + ) + ) + +} + /** An explicit output file the emitter will write to * - set with `-o/--output-file` * @param file output filename From 7b1817ab13c43b2ae6246bad4e5db70696a77867 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Tue, 24 Aug 2021 16:06:04 -0700 Subject: [PATCH 05/24] Compiler options for proto buf emission --- .../backends/proto/ProtoBufEmitter.scala | 49 +++++++++++++++++-- .../firrtl/stage/FirrtlAnnotations.scala | 1 - src/main/scala/firrtl/stage/FirrtlCli.scala | 1 + 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala b/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala index 31edb6b842..0817f55cdb 100644 --- a/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala +++ b/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala @@ -1,8 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 package firrtl.backends.proto -import firrtl.{AnnotationSeq, CircuitState, DependencyAPIMigration, Transform} -import firrtl.ir +import firrtl._ +import firrtl.ir._ import firrtl.annotations.NoTargetAnnotation import firrtl.options.CustomFileEmission import firrtl.options.Viewer.view @@ -10,6 +10,8 @@ import firrtl.proto.ToProto import firrtl.stage.{FirrtlOptions, Forms} import firrtl.stage.TransformManager.TransformDependency import java.io.{ByteArrayOutputStream, Writer} +import scala.collection.mutable.ArrayBuffer +import Utils.throwInternalError /** This object defines Annotations that are used by Protocol Buffer emission. */ @@ -59,10 +61,47 @@ sealed abstract class ProtoBufEmitter(prereqs: Seq[TransformDependency]) override def optionalPrerequisiteOf = Seq.empty override def invalidates(a: Transform) = false - override def execute(state: CircuitState) = - state.copy(annotations = state.annotations :+ Annotation.ProtoBufSerialization(state.circuit, Some(outputSuffix))) + private def emitAllModules(circuit: Circuit): Seq[Annotation.ProtoBufSerialization] = { + // For a given module, returns a Seq of all modules instantited inside of it + def collectInstantiatedModules(mod: Module, map: Map[String, DefModule]): Seq[DefModule] = { + // Use list instead of set to maintain order + val modules = ArrayBuffer.empty[DefModule] + def onStmt(stmt: Statement): Unit = stmt match { + case DefInstance(_, _, name, _) => modules += map(name) + case _: WDefInstanceConnector => throwInternalError(s"unrecognized statement: $stmt") + case other => other.foreach(onStmt) + } + onStmt(mod.body) + modules.distinct.toSeq + } + val modMap = circuit.modules.map(m => m.name -> m).toMap + // Turn each module into it's own circuit with it as the top and all instantied modules as ExtModules + circuit.modules.collect { + case m: Module => + val instModules = collectInstantiatedModules(m, modMap) + val extModules = instModules.map { + case Module(info, name, ports, _) => ExtModule(info, name, ports, name, Seq.empty) + case ext: ExtModule => ext + } + val newCircuit = Circuit(m.info, extModules :+ m, m.name) + Annotation.ProtoBufSerialization(newCircuit, Some(outputSuffix)) + } + } + + override def execute(state: CircuitState) = { + val newAnnos = state.annotations.flatMap { + case EmitCircuitAnnotation(a) if this.getClass == a => + Seq( + Annotation.ProtoBufSerialization(state.circuit, Some(outputSuffix)) + ) + case EmitAllModulesAnnotation(a) if this.getClass == a => + emitAllModules(state.circuit) + case _ => Seq() + } + state.copy(annotations = newAnnos ++ state.annotations) + } - override def emit(state: CircuitState, writer: Writer): Unit = { + def emit(state: CircuitState, writer: Writer): Unit = { val ostream = new java.io.ByteArrayOutputStream ToProto.writeToStream(ostream, state.circuit) writer.write(ostream.toString()) diff --git a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala index 9afbfc7e1e..c1c10c0f80 100644 --- a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala +++ b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala @@ -66,7 +66,6 @@ object FirrtlFileAnnotation extends HasShellOptions { /** Read a directory of FIRRTL files or ProtoBufs * - set with `-I/--input-directory` - * - If unset, an [[FirrtlFileAnnotation]] with the default input file __will not be generated__ * @param file input filename */ case class FirrtlDirectoryAnnotation(dir: String) extends NoTargetAnnotation with CircuitOption { diff --git a/src/main/scala/firrtl/stage/FirrtlCli.scala b/src/main/scala/firrtl/stage/FirrtlCli.scala index 8f84ff1813..5a7d006ea8 100644 --- a/src/main/scala/firrtl/stage/FirrtlCli.scala +++ b/src/main/scala/firrtl/stage/FirrtlCli.scala @@ -13,6 +13,7 @@ trait FirrtlCli { this: Shell => parser.note("FIRRTL Compiler Options") Seq( FirrtlFileAnnotation, + FirrtlDirectoryAnnotation, OutputFileAnnotation, InfoModeAnnotation, FirrtlSourceAnnotation, From 2362a757c5e75cce518dee27e8bd421aba521e0f Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Fri, 27 Aug 2021 13:36:38 -0700 Subject: [PATCH 06/24] Implement multi module combination when reading directory of protobufs --- src/main/scala/firrtl/Utils.scala | 31 ++++++++++++++++ .../backends/proto/ProtoBufEmitter.scala | 2 +- src/main/scala/firrtl/proto/FromProto.scala | 11 +----- .../scala/firrtl/stage/phases/Checks.scala | 37 +++++++++++-------- 4 files changed, 55 insertions(+), 26 deletions(-) diff --git a/src/main/scala/firrtl/Utils.scala b/src/main/scala/firrtl/Utils.scala index e2bb06ffa5..ac108c29e4 100644 --- a/src/main/scala/firrtl/Utils.scala +++ b/src/main/scala/firrtl/Utils.scala @@ -978,6 +978,37 @@ object Utils extends LazyLogging { map.view.map({ case (k, vs) => k -> vs.toList }).toList } + /** Combines several separate circuit modules (typically emitted by -e or -p compiler options) into a single circuit */ + def combine(circuits: Seq[Circuit]): Circuit = { + def dedup(modules: Seq[DefModule]): Seq[Either[Module, DefModule]] = { + // Left means "lone module", Right means has ExtModules + val module: Option[Module] = { + val found: Seq[Module] = modules.collect { case m: Module => m } + assert(found.size <= 1) + found.headOption + } + val extModules: Seq[ExtModule] = modules.collect { case e: ExtModule => e }.distinct + + if (extModules.isEmpty) Seq(Left(module.get)) else (module ++: extModules).map(Right(_)) + } + + // 1. Combine modules + val grouped: Seq[(String, Seq[DefModule])] = groupByIntoSeq(circuits.flatMap(_.modules))({ + case mod: Module => mod.name + case ext: ExtModule => ext.defname + }) + val deduped: Iterable[Either[Module, DefModule]] = grouped.flatMap { case (_, insts) => dedup(insts) } + + // 2. Determine top + val top = { + val found = deduped.collect { case Left(m) => m } + assert(found.size == 1) + found.head + } + val res = deduped.collect { case Right(m: Module) => m } + ir.Circuit(NoInfo, top +: res.toSeq, top.name) + } + object True { private val _True = UIntLiteral(1, IntWidth(1)) diff --git a/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala b/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala index 0817f55cdb..c3f2af4515 100644 --- a/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala +++ b/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala @@ -69,7 +69,7 @@ sealed abstract class ProtoBufEmitter(prereqs: Seq[TransformDependency]) def onStmt(stmt: Statement): Unit = stmt match { case DefInstance(_, _, name, _) => modules += map(name) case _: WDefInstanceConnector => throwInternalError(s"unrecognized statement: $stmt") - case other => other.foreach(onStmt) + case other => other.foreachStmt(onStmt) } onStmt(mod.body) modules.distinct.toSeq diff --git a/src/main/scala/firrtl/proto/FromProto.scala b/src/main/scala/firrtl/proto/FromProto.scala index 9966ede5e4..0335ef1e02 100644 --- a/src/main/scala/firrtl/proto/FromProto.scala +++ b/src/main/scala/firrtl/proto/FromProto.scala @@ -10,6 +10,7 @@ import FirrtlProtos._ import com.google.protobuf.CodedInputStream import Firrtl.Statement.{Formal, ReadUnderWrite} import firrtl.ir.DefModule +import Utils.combine object FromProto { @@ -47,15 +48,7 @@ object FromProto { List[File]() } - val circuits = fileList.map(f => fromInputStream(new FileInputStream(f))) - val tops = circuits.map(c => c.main).distinct - - require(tops.length == 1, "Not all multi-ProtoBufs point to the same top") - - // Concatenate all modules together - val modules = circuits.flatMap(c => c.modules).distinct - - ir.Circuit(ir.NoInfo, modules, tops.head) + combine(fileList.map(f => fromInputStream(new FileInputStream(f)))) } // Convert from ProtoBuf message repeated Statements to FIRRRTL Block diff --git a/src/main/scala/firrtl/stage/phases/Checks.scala b/src/main/scala/firrtl/stage/phases/Checks.scala index e906539375..edfb7d038d 100644 --- a/src/main/scala/firrtl/stage/phases/Checks.scala +++ b/src/main/scala/firrtl/stage/phases/Checks.scala @@ -31,34 +31,39 @@ class Checks extends Phase { * @throws firrtl.options.OptionsException if any checks fail */ def transform(annos: AnnotationSeq): AnnotationSeq = { - val inF, inS, eam, ec, outF, emitter, im, inC = collection.mutable.ListBuffer[Annotation]() + val inF, inS, inD, eam, ec, outF, emitter, im, inC = collection.mutable.ListBuffer[Annotation]() annos.foreach(_ match { - case a: FirrtlFileAnnotation => a +=: inF - case a: FirrtlSourceAnnotation => a +=: inS - case a: EmitAllModulesAnnotation => a +=: eam - case a: EmitCircuitAnnotation => a +=: ec - case a: OutputFileAnnotation => a +=: outF - case a: InfoModeAnnotation => a +=: im - case a: FirrtlCircuitAnnotation => a +=: inC + case a: FirrtlFileAnnotation => a +=: inF + case a: FirrtlSourceAnnotation => a +=: inS + case a: FirrtlDirectoryAnnotation => a +=: inD + case a: EmitAllModulesAnnotation => a +=: eam + case a: EmitCircuitAnnotation => a +=: ec + case a: OutputFileAnnotation => a +=: outF + case a: InfoModeAnnotation => a +=: im + case a: FirrtlCircuitAnnotation => a +=: inC case a @ RunFirrtlTransformAnnotation(_: firrtl.Emitter) => a +=: emitter case _ => }) /* At this point, only a FIRRTL Circuit should exist */ - if (inF.isEmpty && inS.isEmpty && inC.isEmpty) { - throw new OptionsException(s"""|Unable to determine FIRRTL source to read. None of the following were found: - | - an input file: -i, --input-file, FirrtlFileAnnotation - | - FIRRTL source: --firrtl-source, FirrtlSourceAnnotation - | - FIRRTL circuit: FirrtlCircuitAnnotation""".stripMargin) + if (inF.isEmpty && inS.isEmpty && inD.isEmpty && inC.isEmpty) { + throw new OptionsException( + s"""|Unable to determine FIRRTL source to read. None of the following were found: + | - an input file: -i, --input-file, FirrtlFileAnnotation + | - an input dir: -I, --input-directory, FirrtlDirectoryAnnotation + | - FIRRTL source: --firrtl-source, FirrtlSourceAnnotation + | - FIRRTL circuit: FirrtlCircuitAnnotation""".stripMargin + ) } /* Only one FIRRTL input can exist */ if (inF.size + inS.size + inC.size > 1) { throw new OptionsException( s"""|Multiply defined input FIRRTL sources. More than one of the following was found: - | - an input file (${inF.size} times): -i, --input-file, FirrtlFileAnnotation - | - FIRRTL source (${inS.size} times): --firrtl-source, FirrtlSourceAnnotation - | - FIRRTL circuit (${inC.size} times): FirrtlCircuitAnnotation""".stripMargin + | - an input file (${inF.size} times): -i, --input-file, FirrtlFileAnnotation + | - an input dir (${inD.size} times): -I, --input-directory, FirrtlDirectoryAnnotation + | - FIRRTL source (${inS.size} times): --firrtl-source, FirrtlSourceAnnotation + | - FIRRTL circuit (${inC.size} times): FirrtlCircuitAnnotation""".stripMargin ) } From e2245c0a306f1472c794201a8b90cec9b39cf1e1 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Tue, 31 Aug 2021 14:38:47 -0700 Subject: [PATCH 07/24] Use foreacher in protobuf emitter --- src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala b/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala index c3f2af4515..6610ff2592 100644 --- a/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala +++ b/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala @@ -9,6 +9,7 @@ import firrtl.options.Viewer.view import firrtl.proto.ToProto import firrtl.stage.{FirrtlOptions, Forms} import firrtl.stage.TransformManager.TransformDependency +import firrtl.traversals.Foreachers._ import java.io.{ByteArrayOutputStream, Writer} import scala.collection.mutable.ArrayBuffer import Utils.throwInternalError @@ -69,7 +70,7 @@ sealed abstract class ProtoBufEmitter(prereqs: Seq[TransformDependency]) def onStmt(stmt: Statement): Unit = stmt match { case DefInstance(_, _, name, _) => modules += map(name) case _: WDefInstanceConnector => throwInternalError(s"unrecognized statement: $stmt") - case other => other.foreachStmt(onStmt) + case other => other.foreach(onStmt) } onStmt(mod.body) modules.distinct.toSeq From 3155a0a60499a9471b17d3aae7c1ddbb0ea97b14 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Tue, 31 Aug 2021 16:07:19 -0700 Subject: [PATCH 08/24] Revert directoryToCircuit change --- .../src/main/scala/firrtl/benchmark/hot/PassBenchmark.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmark/src/main/scala/firrtl/benchmark/hot/PassBenchmark.scala b/benchmark/src/main/scala/firrtl/benchmark/hot/PassBenchmark.scala index cf72963a49..707c717021 100644 --- a/benchmark/src/main/scala/firrtl/benchmark/hot/PassBenchmark.scala +++ b/benchmark/src/main/scala/firrtl/benchmark/hot/PassBenchmark.scala @@ -13,7 +13,7 @@ abstract class PassBenchmark(passFactory: () => Pass) extends App { val warmup = args(1).toInt val runs = args(2).toInt - val input = directoryToCircuit(inputFile) + val input = filenameToCircuit(inputFile) val inputState = CircuitState(input, ChirrtlForm) val manager = new TransformManager(passFactory().prerequisites) From beeec3ef7af9e52870bb74841f17de29664af0e7 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Tue, 31 Aug 2021 16:45:03 -0700 Subject: [PATCH 09/24] Low opt option now properly uses low opt emitter --- src/main/scala/firrtl/Emitter.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/scala/firrtl/Emitter.scala b/src/main/scala/firrtl/Emitter.scala index 8e0572fba7..152ff051e9 100644 --- a/src/main/scala/firrtl/Emitter.scala +++ b/src/main/scala/firrtl/Emitter.scala @@ -99,7 +99,7 @@ object EmitCircuitAnnotation extends HasShellOptions { case "low-opt" => Seq( RunFirrtlTransformAnnotation(new ProtoEmitter.OptLow), - EmitCircuitAnnotation(classOf[ProtoEmitter.Low]) + EmitCircuitAnnotation(classOf[ProtoEmitter.OptLow]) ) case _ => throw new PhaseException(s"Unknown emitter '$a'! (Did you misspell it?)") }, @@ -181,7 +181,7 @@ object EmitAllModulesAnnotation extends HasShellOptions { case "low-opt" => Seq( RunFirrtlTransformAnnotation(new ProtoEmitter.OptLow), - EmitAllModulesAnnotation(classOf[ProtoEmitter.Low]) + EmitAllModulesAnnotation(classOf[ProtoEmitter.OptLow]) ) case _ => throw new PhaseException(s"Unknown emitter '$a'! (Did you misspell it?)") }, From 691994a7bce9b3232c4d862d5013188e20ed864d Mon Sep 17 00:00:00 2001 From: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com> Date: Wed, 1 Sep 2021 12:47:08 -0700 Subject: [PATCH 10/24] emit-modules-protobuf for long -e option Co-authored-by: Jack Koenig --- src/main/scala/firrtl/Emitter.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/firrtl/Emitter.scala b/src/main/scala/firrtl/Emitter.scala index 152ff051e9..50461f8750 100644 --- a/src/main/scala/firrtl/Emitter.scala +++ b/src/main/scala/firrtl/Emitter.scala @@ -153,7 +153,7 @@ object EmitAllModulesAnnotation extends HasShellOptions { helpValueName = Some("") ), new ShellOption[String]( - longOption = "emit-module-protobufs", + longOption = "emit-modules-protobuf", toAnnotationSeq = (a: String) => a match { case "chirrtl" => From 3c3974502c882c127a98b473aad5572084268126 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Wed, 1 Sep 2021 14:20:08 -0700 Subject: [PATCH 11/24] Directory annotation now correctly throws exceptions --- src/main/scala/firrtl/proto/FromProto.scala | 17 ++++++++++++----- .../scala/firrtl/stage/FirrtlAnnotations.scala | 4 +++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/main/scala/firrtl/proto/FromProto.scala b/src/main/scala/firrtl/proto/FromProto.scala index 0335ef1e02..047f1bf651 100644 --- a/src/main/scala/firrtl/proto/FromProto.scala +++ b/src/main/scala/firrtl/proto/FromProto.scala @@ -11,6 +11,9 @@ import com.google.protobuf.CodedInputStream import Firrtl.Statement.{Formal, ReadUnderWrite} import firrtl.ir.DefModule import Utils.combine +import java.io.FileNotFoundException +import firrtl.options.OptionsException +import java.nio.file.NotDirectoryException object FromProto { @@ -35,19 +38,23 @@ object FromProto { proto.FromProto.convert(pb) } - /** Deserialize all the ProtoBuf representations of [[ir.Circuit]] in @directory + /** Deserialize all the ProtoBuf representations of [[ir.Circuit]] in @dir * * @param dir directory containing ProtoBuf representation(s) * @return Deserialized FIRRTL Circuit + * @throws FileNotFoundException if dir does not exist + * @throws NotDirectoryException if dir exists but is not a directory */ def fromDirectory(dir: String): ir.Circuit = { val d = new File(dir) - val fileList = if (d.exists && d.isDirectory) { - d.listFiles.filter(_.isFile).toList - } else { - List[File]() + if (!d.exists) { + throw new FileNotFoundException + } + if (!d.isDirectory) { + throw new NotDirectoryException("Not a directory") } + val fileList = d.listFiles.filter(_.isFile).toList combine(fileList.map(f => fromInputStream(new FileInputStream(f)))) } diff --git a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala index c1c10c0f80..68a965d0c6 100644 --- a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala +++ b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala @@ -7,7 +7,7 @@ import firrtl.ir.Circuit import firrtl.annotations.{Annotation, NoTargetAnnotation} import firrtl.options.{Dependency, HasShellOptions, OptionsException, ShellOption, Unserializable} import java.io.{File, FileNotFoundException} -import java.nio.file.NoSuchFileException +import java.nio.file.{NoSuchFileException, NotDirectoryException} import firrtl.stage.TransformManager.TransformDependency @@ -77,6 +77,8 @@ case class FirrtlDirectoryAnnotation(dir: String) extends NoTargetAnnotation wit } catch { case a @ (_: FileNotFoundException | _: NoSuchFileException) => throw new OptionsException(s"Directory '$dir' not found! (Did you misspell it?)", a) + case _: NotDirectoryException => + throw new OptionsException(s"Directory '$dir' is not a directory") } FirrtlCircuitAnnotation(circuit) } From 0b538426ce9172fefbcb543002247868816f9c30 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Wed, 1 Sep 2021 17:04:49 -0700 Subject: [PATCH 12/24] Dedup collects extmodules if they don't have an accompanying implementation --- src/main/scala/firrtl/Utils.scala | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/scala/firrtl/Utils.scala b/src/main/scala/firrtl/Utils.scala index ac108c29e4..35503b9c13 100644 --- a/src/main/scala/firrtl/Utils.scala +++ b/src/main/scala/firrtl/Utils.scala @@ -989,7 +989,15 @@ object Utils extends LazyLogging { } val extModules: Seq[ExtModule] = modules.collect { case e: ExtModule => e }.distinct - if (extModules.isEmpty) Seq(Left(module.get)) else (module ++: extModules).map(Right(_)) + // If the module is a lone module (no extmodule references in any other file) + if (extModules.isEmpty && !module.isEmpty) + Seq(Left(module.get)) + // If a module has extmodules, but no other file contains the implementation + else if (!extModules.isEmpty && module.isEmpty) + extModules.map(Right(_)) + // Otherwise there is a module implementation with extmodule references + else + Seq(Right(module.get)) } // 1. Combine modules @@ -1005,7 +1013,7 @@ object Utils extends LazyLogging { assert(found.size == 1) found.head } - val res = deduped.collect { case Right(m: Module) => m } + val res = deduped.collect { case m: Either[Module, DefModule] => m.merge } ir.Circuit(NoInfo, top +: res.toSeq, top.name) } From e161016e40e211836841939b9a06d33868d0bec8 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Thu, 2 Sep 2021 12:44:06 -0700 Subject: [PATCH 13/24] Absolute paths for exception references in scala doc --- src/main/scala/firrtl/proto/FromProto.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/scala/firrtl/proto/FromProto.scala b/src/main/scala/firrtl/proto/FromProto.scala index 047f1bf651..21ddebacc5 100644 --- a/src/main/scala/firrtl/proto/FromProto.scala +++ b/src/main/scala/firrtl/proto/FromProto.scala @@ -42,8 +42,8 @@ object FromProto { * * @param dir directory containing ProtoBuf representation(s) * @return Deserialized FIRRTL Circuit - * @throws FileNotFoundException if dir does not exist - * @throws NotDirectoryException if dir exists but is not a directory + * @throws java.io.FileNotFoundException if dir does not exist + * @throws java.nio.file.NotDirectoryException if dir exists but is not a directory */ def fromDirectory(dir: String): ir.Circuit = { val d = new File(dir) From c6704d3731a14ab7b7dc4de914514a481a3d825d Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Thu, 2 Sep 2021 15:51:00 -0700 Subject: [PATCH 14/24] Compiler option tests --- .../firrtlTests/stage/FirrtlMainSpec.scala | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala index d51fc164eb..963042c67f 100644 --- a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala +++ b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala @@ -246,6 +246,16 @@ class FirrtlMainSpec args = Array("-X", "sverilog", "-e", "sverilog"), files = Seq("Top.sv", "Child.sv") ), + /* Test all one protobuf per module emitters */ + FirrtlMainTest( + args = Array("-X", "none", "--emit-modules-protobuf", "chirrtl"), + files = Seq("Top.pb", "Child.pb") + ), + FirrtlMainTest(args = Array("-X", "none", "-p", "mhigh"), files = Seq("Top.mhi.pb", "Child.mhi.pb")), + FirrtlMainTest(args = Array("-X", "none", "-p", "high"), files = Seq("Top.hi.pb", "Child.hi.pb")), + FirrtlMainTest(args = Array("-X", "none", "-p", "middle"), files = Seq("Top.mid.pb", "Child.mid.pb")), + FirrtlMainTest(args = Array("-X", "none", "-p", "low"), files = Seq("Top.lo.pb", "Child.lo.pb")), + FirrtlMainTest(args = Array("-X", "none", "-p", "low-opt"), files = Seq("Top.lo.pb", "Child.lo.pb")), /* Test mixing of -E with -e */ FirrtlMainTest( args = Array("-X", "middle", "-E", "high", "-e", "middle"), @@ -324,6 +334,42 @@ class FirrtlMainSpec new File(td.buildDir + "/Foo.hi.fir") should (exist) } + Scenario("User compiles to multiple Protocol Buffers") { + val stage1 = new FirrtlMainFixture + val td = new TargetDirectoryFixture("multi-protobuf") + val c = new SimpleFirrtlCircuitFixture + val protobufs = Seq("Top.hi.pb", "Child.hi.pb") + + And("some input multi-module FIRRTL IR") + val inputFile: Array[String] = { + val in = new File(td.dir, c.main) + val pw = new PrintWriter(in) + pw.write(c.input) + pw.close() + Array("-i", in.toString) + } + + When("the user tries to compile to multiple Protocol Buffers in the target directory") + stage1.stage.main( + inputFile ++ Array("-X", "none", "-p", "high", "-td", td.buildDir.toString) + ) + + protobufs.foreach { f => + Then(s"file '$f' should be emitted") + val out = new File(td.buildDir + s"/$f") + out should (exist) + } + + val stage2 = new FirrtlMainFixture + When("the user compiles the Protobufs to a single High FIRRTL IR") + stage2.stage.main( + Array("-I", td.buildDir.toString, "-X", "none", "-E", "high", "-td", td.buildDir.toString, "-o", "Foo") + ) + + Then("one single High FIRRTL file should be emitted") + new File(td.buildDir + "/Foo.hi.fir") should (exist) + } + } info("As a FIRRTL command line user") @@ -381,6 +427,12 @@ class FirrtlMainSpec circuit = None, stdout = Some("Unknown compiler name 'Verilog'! (Did you misspell it?)"), result = 1 + ), + FirrtlMainTest( + args = Array("-I", "test_run_dir/I-DO-NOT-EXIST"), + circuit = None, + stdout = Some("Directory 'test_run_dir/I-DO-NOT-EXIST' not found!"), + result = 1 ) ) .foreach(runStageExpectFiles) From c99ed1ea4cb23c41002a931f000e1fc0d455ef79 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Thu, 2 Sep 2021 15:52:03 -0700 Subject: [PATCH 15/24] Fix multiple inclusion of top module in combined circuit --- src/main/scala/firrtl/Utils.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/scala/firrtl/Utils.scala b/src/main/scala/firrtl/Utils.scala index 35503b9c13..ac41d53e8f 100644 --- a/src/main/scala/firrtl/Utils.scala +++ b/src/main/scala/firrtl/Utils.scala @@ -981,7 +981,7 @@ object Utils extends LazyLogging { /** Combines several separate circuit modules (typically emitted by -e or -p compiler options) into a single circuit */ def combine(circuits: Seq[Circuit]): Circuit = { def dedup(modules: Seq[DefModule]): Seq[Either[Module, DefModule]] = { - // Left means "lone module", Right means has ExtModules + // Left means module with no ExtModules, Right means child modules or lone ExtModules val module: Option[Module] = { val found: Seq[Module] = modules.collect { case m: Module => m } assert(found.size <= 1) @@ -1013,7 +1013,7 @@ object Utils extends LazyLogging { assert(found.size == 1) found.head } - val res = deduped.collect { case m: Either[Module, DefModule] => m.merge } + val res = deduped.collect { case Right(m) => m } ir.Circuit(NoInfo, top +: res.toSeq, top.name) } From f4200975dbe17d6f6cb8823c6ce6eef811387608 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Thu, 2 Sep 2021 16:18:38 -0700 Subject: [PATCH 16/24] Only one stage is sufficient for multiprotobuf test --- src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala index 963042c67f..a7d2b48de3 100644 --- a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala +++ b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala @@ -335,7 +335,7 @@ class FirrtlMainSpec } Scenario("User compiles to multiple Protocol Buffers") { - val stage1 = new FirrtlMainFixture + val f = new FirrtlMainFixture val td = new TargetDirectoryFixture("multi-protobuf") val c = new SimpleFirrtlCircuitFixture val protobufs = Seq("Top.hi.pb", "Child.hi.pb") @@ -350,7 +350,7 @@ class FirrtlMainSpec } When("the user tries to compile to multiple Protocol Buffers in the target directory") - stage1.stage.main( + f.stage.main( inputFile ++ Array("-X", "none", "-p", "high", "-td", td.buildDir.toString) ) @@ -360,9 +360,8 @@ class FirrtlMainSpec out should (exist) } - val stage2 = new FirrtlMainFixture When("the user compiles the Protobufs to a single High FIRRTL IR") - stage2.stage.main( + f.stage.main( Array("-I", td.buildDir.toString, "-X", "none", "-E", "high", "-td", td.buildDir.toString, "-o", "Foo") ) From f991a286eec18ac49b25e026e64cd82aa47004f0 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Fri, 3 Sep 2021 16:27:56 -0700 Subject: [PATCH 17/24] Multi-module circuit combination tests --- src/main/scala/firrtl/Utils.scala | 4 + src/test/scala/firrtlTests/UtilsSpec.scala | 171 +++++++++++++++++++++ 2 files changed, 175 insertions(+) diff --git a/src/main/scala/firrtl/Utils.scala b/src/main/scala/firrtl/Utils.scala index ac41d53e8f..4e08a27c5f 100644 --- a/src/main/scala/firrtl/Utils.scala +++ b/src/main/scala/firrtl/Utils.scala @@ -978,6 +978,10 @@ object Utils extends LazyLogging { map.view.map({ case (k, vs) => k -> vs.toList }).toList } + /** Checks if two circuits are equal regardless of their ordering of module definitions */ + def orderAgnosticEquality(a: Circuit, b: Circuit): Boolean = + a.copy(modules = a.modules.sortBy(_.name)) == b.copy(modules = b.modules.sortBy(_.name)) + /** Combines several separate circuit modules (typically emitted by -e or -p compiler options) into a single circuit */ def combine(circuits: Seq[Circuit]): Circuit = { def dedup(modules: Seq[DefModule]): Seq[Either[Module, DefModule]] = { diff --git a/src/test/scala/firrtlTests/UtilsSpec.scala b/src/test/scala/firrtlTests/UtilsSpec.scala index 99f1ffd0aa..65962749a2 100644 --- a/src/test/scala/firrtlTests/UtilsSpec.scala +++ b/src/test/scala/firrtlTests/UtilsSpec.scala @@ -46,4 +46,175 @@ class UtilsSpec extends AnyFlatSpec { (Utils.expandRef(wr)) should be(expected) } + + def combineTest(circuits: Seq[String], expected: String) = { + (Utils.orderAgnosticEquality(Utils.combine(circuits.map(c => Parser.parse(c))), Parser.parse(expected))) should be( + true + ) + } + + "combine" should "merge multiple module circuits" in { + // + val input = Seq( + """|circuit Top: + | extmodule External: + | output foo: UInt<32> + | defname = External + | + | extmodule Child1: + | output foo: UInt<32> + | defname = Child1 + | + | extmodule Child2: + | output foo: UInt<32> + | defname = Child2 + | + | module Top: + | output foo: UInt<32> + | inst c1 of Child1 + | inst c2 of Child2 + | inst e of External + | foo <= tail(add(add(c1.foo, c2.bar), e.foo), 1) + |""".stripMargin, + """|circuit Child1: + | extmodule External: + | output foo: UInt<32> + | defname = External + | + | module Child1: + | output foo: UInt<32> + | inst e of External + | foo <= e.foo + |""".stripMargin, + """|circuit Child2: + | extmodule External: + | output foo: UInt<32> + | defname = External + | + | module Child2: + | output bar: UInt<32> + | inst e of External + | bar <= e.foo + |""".stripMargin + ) + + val output = + """|circuit Top: + | module Top: + | output foo: UInt<32> + | inst c1 of Child1 + | inst c2 of Child2 + | inst e of External + | foo <= tail(add(add(c1.foo, c2.bar), e.foo), 1) + | + | module Child1: + | output foo: UInt<32> + | inst e of External + | foo <= e.foo + | + | module Child2: + | output bar: UInt<32> + | inst e of External + | bar <= e.foo + | + | extmodule External: + | output foo: UInt<32> + | defname = External + |""".stripMargin + + combineTest(input, output) + } + + "combine" should "dedup ExtModules if an implementation exists" in { + val input = Seq( + """|circuit Top: + | extmodule Child: + | output foo: UInt<32> + | defname = Child + | + | module Top: + | output foo: UInt<32> + | inst c of Child + | foo <= c.foo + |""".stripMargin, + """|circuit Child: + | module Child: + | output foo: UInt<32> + | + | skip + |""".stripMargin + ) + + val output = + """|circuit Top: + | module Top: + | output foo: UInt<32> + | inst c of Child + | foo <= c.foo + | + | module Child: + | output foo: UInt<32> + | + | skip + |""".stripMargin + + combineTest(input, output) + } + + "combine" should "not dedup lone ExtModules" in { + val input = Seq( + """|circuit Top: + | extmodule External: + | output foo: UInt<32> + | defname = External + | + | module Top: + | output foo: UInt<32> + | inst e of External + | foo <= e.foo + |""".stripMargin + ) + + val output = + """|circuit Top: + | extmodule External: + | output foo: UInt<32> + | defname = External + | + | module Top: + | output foo: UInt<32> + | inst e of External + | foo <= e.foo + |""".stripMargin + + combineTest(input, output) + } + + "combine" should "fail with multiple lone Modules" in { + val input = Seq( + """|circuit Top: + | extmodule External: + | output foo: UInt<32> + | defname = External + | + | module Top: + | output foo: UInt<32> + | inst e of External + | foo <= e.foo + |""".stripMargin, + """|circuit Top2: + | extmodule External: + | output foo: UInt<32> + | defname = External + | + | module Top2: + | output bar: UInt<32> + | inst e of External + | bar <= e.foo + |""".stripMargin + ) + + a[java.lang.AssertionError] shouldBe thrownBy { combineTest(input, "") } + } + } From e2189c4c0af26b16e20a1af7e5b327ed2ad45093 Mon Sep 17 00:00:00 2001 From: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com> Date: Fri, 3 Sep 2021 16:29:40 -0700 Subject: [PATCH 18/24] Update src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala Co-authored-by: Jack Koenig --- src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala index a7d2b48de3..4557a3f8b4 100644 --- a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala +++ b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala @@ -349,7 +349,7 @@ class FirrtlMainSpec Array("-i", in.toString) } - When("the user tries to compile to multiple Protocol Buffers in the target directory") + When("the user tries to emit a circuit to multiple Protocol Buffer files in the target directory") f.stage.main( inputFile ++ Array("-X", "none", "-p", "high", "-td", td.buildDir.toString) ) From c26f7d888639569ff3896369f1bf8b699cdf4dfc Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Fri, 3 Sep 2021 16:55:53 -0700 Subject: [PATCH 19/24] Multi protobuf test checks output circuit against input for equality --- .../firrtlTests/stage/FirrtlMainSpec.scala | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala index 4557a3f8b4..30e03b3c12 100644 --- a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala +++ b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala @@ -338,7 +338,7 @@ class FirrtlMainSpec val f = new FirrtlMainFixture val td = new TargetDirectoryFixture("multi-protobuf") val c = new SimpleFirrtlCircuitFixture - val protobufs = Seq("Top.hi.pb", "Child.hi.pb") + val protobufs = Seq("Top.pb", "Child.pb") And("some input multi-module FIRRTL IR") val inputFile: Array[String] = { @@ -351,7 +351,7 @@ class FirrtlMainSpec When("the user tries to emit a circuit to multiple Protocol Buffer files in the target directory") f.stage.main( - inputFile ++ Array("-X", "none", "-p", "high", "-td", td.buildDir.toString) + inputFile ++ Array("-X", "none", "-p", "chirrtl", "-td", td.buildDir.toString) ) protobufs.foreach { f => @@ -360,13 +360,19 @@ class FirrtlMainSpec out should (exist) } - When("the user compiles the Protobufs to a single High FIRRTL IR") + When("the user compiles the Protobufs to a single FIRRTL IR") f.stage.main( - Array("-I", td.buildDir.toString, "-X", "none", "-E", "high", "-td", td.buildDir.toString, "-o", "Foo") + Array("-I", td.buildDir.toString, "-X", "none", "-E", "chirrtl", "-td", td.buildDir.toString, "-o", "Foo") ) - Then("one single High FIRRTL file should be emitted") - new File(td.buildDir + "/Foo.hi.fir") should (exist) + Then("one single FIRRTL file should be emitted") + val outFile = new File(td.buildDir + "/Foo.fir") + outFile should (exist) + And("it should be the same as using FIRRTL input") + firrtl.Utils.orderAgnosticEquality( + firrtl.Parser.parse(c.input), + firrtl.Parser.parseFile(td.buildDir + "/Foo.fir", firrtl.Parser.IgnoreInfo) + ) should be(true) } } From 118ff7d9f2263cb5a4f1934be868102f55dd5fbb Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Fri, 3 Sep 2021 17:00:44 -0700 Subject: [PATCH 20/24] Move collectInstantiatedModules into private util function --- src/main/scala/firrtl/Utils.scala | 13 +++++++++++++ .../firrtl/backends/firrtl/FirrtlEmitter.scala | 12 ------------ .../firrtl/backends/proto/ProtoBufEmitter.scala | 14 +------------- 3 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/main/scala/firrtl/Utils.scala b/src/main/scala/firrtl/Utils.scala index 4e08a27c5f..a8c4a68254 100644 --- a/src/main/scala/firrtl/Utils.scala +++ b/src/main/scala/firrtl/Utils.scala @@ -978,6 +978,19 @@ object Utils extends LazyLogging { map.view.map({ case (k, vs) => k -> vs.toList }).toList } + // For a given module, returns a Seq of all instantiated modules inside of it + private[firrtl] def collectInstantiatedModules(mod: Module, map: Map[String, DefModule]): Seq[DefModule] = { + // Use list instead of set to maintain order + val modules = mutable.ArrayBuffer.empty[DefModule] + def onStmt(stmt: Statement): Unit = stmt match { + case DefInstance(_, _, name, _) => modules += map(name) + case _: WDefInstanceConnector => throwInternalError(s"unrecognized statement: $stmt") + case other => other.foreach(onStmt) + } + onStmt(mod.body) + modules.distinct.toSeq + } + /** Checks if two circuits are equal regardless of their ordering of module definitions */ def orderAgnosticEquality(a: Circuit, b: Circuit): Boolean = a.copy(modules = a.modules.sortBy(_.name)) == b.copy(modules = b.modules.sortBy(_.name)) diff --git a/src/main/scala/firrtl/backends/firrtl/FirrtlEmitter.scala b/src/main/scala/firrtl/backends/firrtl/FirrtlEmitter.scala index 56b63d757c..69f3da00e2 100644 --- a/src/main/scala/firrtl/backends/firrtl/FirrtlEmitter.scala +++ b/src/main/scala/firrtl/backends/firrtl/FirrtlEmitter.scala @@ -18,18 +18,6 @@ sealed abstract class FirrtlEmitter(form: Seq[TransformDependency], val outputSu override def invalidates(a: Transform) = false private def emitAllModules(circuit: Circuit): Seq[EmittedFirrtlModule] = { - // For a given module, returns a Seq of all modules instantited inside of it - def collectInstantiatedModules(mod: Module, map: Map[String, DefModule]): Seq[DefModule] = { - // Use list instead of set to maintain order - val modules = mutable.ArrayBuffer.empty[DefModule] - def onStmt(stmt: Statement): Unit = stmt match { - case DefInstance(_, _, name, _) => modules += map(name) - case _: WDefInstanceConnector => throwInternalError(s"unrecognized statement: $stmt") - case other => other.foreach(onStmt) - } - onStmt(mod.body) - modules.distinct.toSeq - } val modMap = circuit.modules.map(m => m.name -> m).toMap // Turn each module into it's own circuit with it as the top and all instantied modules as ExtModules circuit.modules.collect { diff --git a/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala b/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala index 6610ff2592..c617ea27da 100644 --- a/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala +++ b/src/main/scala/firrtl/backends/proto/ProtoBufEmitter.scala @@ -12,7 +12,7 @@ import firrtl.stage.TransformManager.TransformDependency import firrtl.traversals.Foreachers._ import java.io.{ByteArrayOutputStream, Writer} import scala.collection.mutable.ArrayBuffer -import Utils.throwInternalError +import Utils.{collectInstantiatedModules, throwInternalError} /** This object defines Annotations that are used by Protocol Buffer emission. */ @@ -63,18 +63,6 @@ sealed abstract class ProtoBufEmitter(prereqs: Seq[TransformDependency]) override def invalidates(a: Transform) = false private def emitAllModules(circuit: Circuit): Seq[Annotation.ProtoBufSerialization] = { - // For a given module, returns a Seq of all modules instantited inside of it - def collectInstantiatedModules(mod: Module, map: Map[String, DefModule]): Seq[DefModule] = { - // Use list instead of set to maintain order - val modules = ArrayBuffer.empty[DefModule] - def onStmt(stmt: Statement): Unit = stmt match { - case DefInstance(_, _, name, _) => modules += map(name) - case _: WDefInstanceConnector => throwInternalError(s"unrecognized statement: $stmt") - case other => other.foreach(onStmt) - } - onStmt(mod.body) - modules.distinct.toSeq - } val modMap = circuit.modules.map(m => m.name -> m).toMap // Turn each module into it's own circuit with it as the top and all instantied modules as ExtModules circuit.modules.collect { From 0885f7004cabe01d97fd6503e4c012721c47626f Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Tue, 7 Sep 2021 15:42:04 -0700 Subject: [PATCH 21/24] Comment cleanup --- src/main/scala/firrtl/stage/FirrtlAnnotations.scala | 4 +++- src/test/scala/firrtlTests/UtilsSpec.scala | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala index 68a965d0c6..a40277207b 100644 --- a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala +++ b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala @@ -64,8 +64,10 @@ object FirrtlFileAnnotation extends HasShellOptions { } -/** Read a directory of FIRRTL files or ProtoBufs +/** Read a directory of ProtoBufs * - set with `-I/--input-directory` + * + * TODO: Does not currently support FIRRTL files. * @param file input filename */ case class FirrtlDirectoryAnnotation(dir: String) extends NoTargetAnnotation with CircuitOption { diff --git a/src/test/scala/firrtlTests/UtilsSpec.scala b/src/test/scala/firrtlTests/UtilsSpec.scala index 65962749a2..b3e0e373a1 100644 --- a/src/test/scala/firrtlTests/UtilsSpec.scala +++ b/src/test/scala/firrtlTests/UtilsSpec.scala @@ -54,7 +54,6 @@ class UtilsSpec extends AnyFlatSpec { } "combine" should "merge multiple module circuits" in { - // val input = Seq( """|circuit Top: | extmodule External: From 4830c895922d6183bb21a9c9e58e5826eb36cf29 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Tue, 7 Sep 2021 15:44:28 -0700 Subject: [PATCH 22/24] Rename file -> dir in scaladoc --- src/main/scala/firrtl/stage/FirrtlAnnotations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala index a40277207b..8e1e200101 100644 --- a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala +++ b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala @@ -68,7 +68,7 @@ object FirrtlFileAnnotation extends HasShellOptions { * - set with `-I/--input-directory` * * TODO: Does not currently support FIRRTL files. - * @param file input filename + * @param dir input directory name */ case class FirrtlDirectoryAnnotation(dir: String) extends NoTargetAnnotation with CircuitOption { From 5942386ff60f62688d4fd0bac3df9d1529fc5699 Mon Sep 17 00:00:00 2001 From: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com> Date: Wed, 8 Sep 2021 12:38:09 -0700 Subject: [PATCH 23/24] Apply suggestions from code review Co-authored-by: Jack Koenig --- src/main/scala/firrtl/Utils.scala | 4 ++-- src/main/scala/firrtl/proto/FromProto.scala | 4 ++-- src/test/scala/firrtlTests/UtilsSpec.scala | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/scala/firrtl/Utils.scala b/src/main/scala/firrtl/Utils.scala index a8c4a68254..4ff1c108af 100644 --- a/src/main/scala/firrtl/Utils.scala +++ b/src/main/scala/firrtl/Utils.scala @@ -1001,7 +1001,7 @@ object Utils extends LazyLogging { // Left means module with no ExtModules, Right means child modules or lone ExtModules val module: Option[Module] = { val found: Seq[Module] = modules.collect { case m: Module => m } - assert(found.size <= 1) + assert(found.size <= 1, s"Module definitions should have unique names, found ${found.size} definitions named ${found.head.name}") found.headOption } val extModules: Seq[ExtModule] = modules.collect { case e: ExtModule => e }.distinct @@ -1027,7 +1027,7 @@ object Utils extends LazyLogging { // 2. Determine top val top = { val found = deduped.collect { case Left(m) => m } - assert(found.size == 1) + assert(found.size == 1, s"There should only be 1 top module, got: ${found.map(_.name).mkString(", ")}") found.head } val res = deduped.collect { case Right(m) => m } diff --git a/src/main/scala/firrtl/proto/FromProto.scala b/src/main/scala/firrtl/proto/FromProto.scala index 21ddebacc5..91b3f872d0 100644 --- a/src/main/scala/firrtl/proto/FromProto.scala +++ b/src/main/scala/firrtl/proto/FromProto.scala @@ -48,10 +48,10 @@ object FromProto { def fromDirectory(dir: String): ir.Circuit = { val d = new File(dir) if (!d.exists) { - throw new FileNotFoundException + throw new FileNotFoundException(s"Specified directory '$d' does not exist!") } if (!d.isDirectory) { - throw new NotDirectoryException("Not a directory") + throw new NotDirectoryException(s"'$d' is not a directory!") } val fileList = d.listFiles.filter(_.isFile).toList diff --git a/src/test/scala/firrtlTests/UtilsSpec.scala b/src/test/scala/firrtlTests/UtilsSpec.scala index b3e0e373a1..1048b370f9 100644 --- a/src/test/scala/firrtlTests/UtilsSpec.scala +++ b/src/test/scala/firrtlTests/UtilsSpec.scala @@ -160,7 +160,7 @@ class UtilsSpec extends AnyFlatSpec { combineTest(input, output) } - "combine" should "not dedup lone ExtModules" in { + "combine" should "support lone ExtModules" in { val input = Seq( """|circuit Top: | extmodule External: From fc136d4307d336387d4f33cfd6eb6fe0f16cd59f Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Wed, 8 Sep 2021 12:52:10 -0700 Subject: [PATCH 24/24] Scalafmt --- src/main/scala/firrtl/Utils.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/scala/firrtl/Utils.scala b/src/main/scala/firrtl/Utils.scala index 4ff1c108af..51748b68b5 100644 --- a/src/main/scala/firrtl/Utils.scala +++ b/src/main/scala/firrtl/Utils.scala @@ -1001,7 +1001,10 @@ object Utils extends LazyLogging { // Left means module with no ExtModules, Right means child modules or lone ExtModules val module: Option[Module] = { val found: Seq[Module] = modules.collect { case m: Module => m } - assert(found.size <= 1, s"Module definitions should have unique names, found ${found.size} definitions named ${found.head.name}") + assert( + found.size <= 1, + s"Module definitions should have unique names, found ${found.size} definitions named ${found.head.name}" + ) found.headOption } val extModules: Seq[ExtModule] = modules.collect { case e: ExtModule => e }.distinct