From 8bdbbac28cbec95181dfd9742b2ac614f6833d01 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Tue, 11 Aug 2020 17:45:10 -0400 Subject: File Serialization of Annotations (#1277) * Transform, not run in LegalizeAndReduction test Switch from using FirrtlStage.transform to FirrtlStage.run in one test. The latter is problematic as it doesn't include wrappers or pre/post phases which are how things will work in the future for doing file writing (via HowToSerialize ideas). Signed-off-by: Schuyler Eldridge * Use execute in FIRRTL testing infra (not run) Changes the FirrtlStage method in FIRRTL testing infrastructure from "run" (which does not include Stage-global Phases) to "execute" (which does). Signed-off-by: Schuyler Eldridge * Add HowToSerialize Annotation mix-in This adds an Annotation mix-in, HowToSerialize, that allows an annotation to declare how it should be serialized to a file. The mix-in is abstract in a baseFileName and a suffix (used to generate a filename), a howToSerialize method (defining the string contents of the file), and a howToResume method (that defines a replacement for the file-serialized annotation that allows this to be resumed) [^1]. A default implementation for generating a filename (called filename) is defined that will put the baseFileName+suffix file in the target directory. This can be overridden by the annotation if desired. [^1]: When an annotation is serialized to a file, it should be removed from the emitted JSON-serialized annotations. The howToResume method defines a way of adding replacement annotations to the JSON-serialized annotations that tell a downstream tool how to find the serialized file. E.g., if a FIRRTL circuit is written to a file, this could be used to add a FirrtlFileAnnotation defining the location of the new file. Signed-off-by: Schuyler Eldridge * Handle HowToSerialize in WriteOutputAnnotations This extends firrtl.options.phase.WriteOutputAnnotations to serialize HowToSerialize annotations to files. Signed-off-by: Schuyler Eldridge * Test HowToSerialize in WriteOutputAnnotationsSpec This adds tests of the HowToSerialize mix-in inside the WriteOutputAnnotationsSpec. Signed-off-by: Schuyler Eldridge * [skip chisel tests] Migrate to HowToSerialize This migrates EmittedAnnotations (and its children) to mixin the HowToSerialize trait. This enables this annotations to be automatically written to files via WriteOutputAnnotations Signed-off-by: Schuyler Eldridge * Deprecated firrtl.stage.phases.WriteEmitted Signed-off-by: Schuyler Eldridge * Use streams in HowToSerialize This converts the HowToSerialize trait to use a Stream[Char] when defining how an annotation should be serialized. Signed-off-by: Schuyler Eldridge * Switch from Stream[Char] to Stream[Byte] Signed-off-by: Schuyler Eldridge * Change howToSerialize method to Iterable Change the type of the HowToSerialize.howToSerialize method from a stream to an iterable. Using the latter (the superset of both lazy streams and non-lazy things like String) avoids problems with users having to choose laziness when they already have an eager object. In effect, this makes the API more general. Signed-off-by: Schuyler Eldridge * Add Scaladoc to HowToSerialize trait Signed-off-by: Schuyler Eldridge * Change HowToSerialize to CustomFileEmission Signed-off-by: Schuyler Eldridge * Add default implementation of replacements Add a default implementation of CustomFileEmission.replacements. Signed-off-by: Schuyler Eldridge * Avoid unnecessary 2x monad in CustomFileEmission Change the type of CustomFileEmission.replacements from Option[AnnotationSeq] to AnnotationSeq. The latter has all the properties of the former that I'm trying to express here: (1) can emptiness and (2) monadicity (if the AnnotationSeq is converted to a sequence first). The latter property is exploited in the WriteOutputAnnotations phase to concisely flatMap over the annotations and doing the double-monad is unnecessary. Signed-off-by: Schuyler Eldridge * Restrict CustomFileEmission filename API Change the API of CustomFileEmission to use a final def for the actual filename. The baseFileName is then made a method with an AnnotationSeq parameter to allow the filename to change as a function of other annotations, e.g., by an output circuit annotation. By restricting this API, we have more control over the default behavior of where things are written using the fixed behavior of the filename method---files will always be written using the behavior that StageOptions define. Previously, if users want customized behavior, they would need to duplicate this StageOptions functionality (and likely subtly deviate from the standard behavior and introduce problems with their build). Signed-off-by: Schuyler Eldridge * Add file conflict behavior for CustomFileEmission Set behavior of file conflicts in CustomFileEmission to be the following: No two annotations in the same annotation sequence can serialize to the same file during the WriteOutputAnnotations phase. However, if the output annotation file already exists, it will be overwritten. Signed-off-by: Schuyler Eldridge * Return relative path from getBuildFileName Change FirrtlOptions.getBuildFileName to simply serialize the underlying Java File instead of converting this to its canonical path. This should improve the relocatability of files produced by the CustomFileEmission API. Signed-off-by: Schuyler Eldridge * Normalize paths in StageOptions.getBuildFile Normalize paths inside the getBuildFileName utility of StageOptions. Add a check to prevent a null pointer dereference. Co-authored-by: Jack Koenig Co-authored-by: Schuyler Eldridge Signed-off-by: Schuyler Eldridge * Refer to CustomFIleEmission in deprecation message Co-authored-by: Jack Koenig Signed-off-by: Schuyler Eldridge * Simplify CustomFileEmission toBytes implementation Co-authored-by: Jack Koenig Co-authored-by: Schuyler Eldridge Signed-off-by: Schuyler Eldridge * Use toBytes, not getBytes, in CustomFileEmission Signed-off-by: Schuyler Eldridge Co-authored-by: Jack Koenig --- .../phases/tests/DriverCompatibilitySpec.scala | 2 +- src/test/scala/firrtl/testutils/FirrtlSpec.scala | 6 +-- .../firrtlTests/execution/VerilogExecution.scala | 3 +- .../phases/WriteOutputAnnotationsSpec.scala | 61 +++++++++++++++++++++- .../transforms/LegalizeReductions.scala | 2 +- 5 files changed, 67 insertions(+), 7 deletions(-) (limited to 'src/test/scala') diff --git a/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala b/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala index 64654175..007608ca 100644 --- a/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala +++ b/src/test/scala/firrtl/stage/phases/tests/DriverCompatibilitySpec.scala @@ -125,7 +125,7 @@ class DriverCompatibilitySpec extends AnyFlatSpec with Matchers with PrivateMeth new PhaseFixture(new AddImplicitFirrtlFile) { val annotations = Seq( TopNameAnnotation("foo") ) val expected = annotations.toSet + - FirrtlFileAnnotation(new File("foo.fir").getCanonicalPath) + FirrtlFileAnnotation(new File("foo.fir").getPath()) phase.transform(annotations).toSet should be (expected) } diff --git a/src/test/scala/firrtl/testutils/FirrtlSpec.scala b/src/test/scala/firrtl/testutils/FirrtlSpec.scala index 75739147..dfc20352 100644 --- a/src/test/scala/firrtl/testutils/FirrtlSpec.scala +++ b/src/test/scala/firrtl/testutils/FirrtlSpec.scala @@ -104,13 +104,13 @@ trait FirrtlRunners extends BackendCompilationUtilities { val customName = s"${prefix}_custom" val customAnnos = getBaseAnnos(customName) ++: toAnnos((new GetNamespace) +: customTransforms) ++: customAnnotations - val customResult = (new firrtl.stage.FirrtlStage).run(customAnnos) + val customResult = (new firrtl.stage.FirrtlStage).execute(Array.empty, customAnnos) val nsAnno = customResult.collectFirst { case m: ModuleNamespaceAnnotation => m }.get val refSuggestedName = s"${prefix}_ref" val refAnnos = getBaseAnnos(refSuggestedName) ++: Seq(RunFirrtlTransformAnnotation(new RenameModules), nsAnno) - val refResult = (new firrtl.stage.FirrtlStage).run(refAnnos) + val refResult = (new firrtl.stage.FirrtlStage).execute(Array.empty, refAnnos) val refName = refResult.collectFirst({ case stage.FirrtlCircuitAnnotation(c) => c.main }).getOrElse(refSuggestedName) assert(BackendCompilationUtilities.yosysExpectSuccess(customName, refName, testDir, timesteps)) @@ -145,7 +145,7 @@ trait FirrtlRunners extends BackendCompilationUtilities { annotations ++: (customTransforms ++ extraCheckTransforms).map(RunFirrtlTransformAnnotation(_)) - (new firrtl.stage.FirrtlStage).run(annos) + (new firrtl.stage.FirrtlStage).execute(Array.empty, annos) testDir } diff --git a/src/test/scala/firrtlTests/execution/VerilogExecution.scala b/src/test/scala/firrtlTests/execution/VerilogExecution.scala index bf3d1461..89f27609 100644 --- a/src/test/scala/firrtlTests/execution/VerilogExecution.scala +++ b/src/test/scala/firrtlTests/execution/VerilogExecution.scala @@ -21,7 +21,8 @@ trait VerilogExecution extends TestExecution { // Run FIRRTL, emit Verilog file val cAnno = FirrtlCircuitAnnotation(c) val tdAnno = TargetDirAnnotation(testDir.getAbsolutePath) - (new FirrtlStage).run(AnnotationSeq(Seq(cAnno, tdAnno) ++ customAnnotations)) + + (new FirrtlStage).execute(Array.empty, AnnotationSeq(Seq(cAnno, tdAnno)) ++ customAnnotations) // Copy harness resource to test directory val harness = new File(testDir, s"top.cpp") diff --git a/src/test/scala/firrtlTests/options/phases/WriteOutputAnnotationsSpec.scala b/src/test/scala/firrtlTests/options/phases/WriteOutputAnnotationsSpec.scala index e71eaedf..0a3cce67 100644 --- a/src/test/scala/firrtlTests/options/phases/WriteOutputAnnotationsSpec.scala +++ b/src/test/scala/firrtlTests/options/phases/WriteOutputAnnotationsSpec.scala @@ -7,7 +7,16 @@ import java.io.File import firrtl.AnnotationSeq import firrtl.annotations.{DeletedAnnotation, NoTargetAnnotation} -import firrtl.options.{InputAnnotationFileAnnotation, OutputAnnotationFileAnnotation, Phase, WriteDeletedAnnotation} +import firrtl.options.{ + CustomFileEmission, + InputAnnotationFileAnnotation, + OutputAnnotationFileAnnotation, + Phase, + PhaseException, + StageOptions, + TargetDirAnnotation, + WriteDeletedAnnotation} +import firrtl.options.Viewer.view import firrtl.options.phases.{GetIncludes, WriteOutputAnnotations} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -100,9 +109,59 @@ class WriteOutputAnnotationsSpec extends AnyFlatSpec with Matchers with firrtl.t out.toSeq should be (annotations) } + it should "write CustomFileEmission annotations" in new Fixture { + val file = new File("write-CustomFileEmission-annotations.anno.json") + val annotations = Seq( TargetDirAnnotation(dir), + OutputAnnotationFileAnnotation(file.toString), + WriteOutputAnnotationsSpec.Custom("hello!") ) + val serializedFileName = view[StageOptions](annotations).getBuildFileName("Custom", Some(".Emission")) + val expected = annotations.map { + case _: WriteOutputAnnotationsSpec.Custom => WriteOutputAnnotationsSpec.Replacement(serializedFileName) + case a => a + } + + val out = phase.transform(annotations) + + info("annotations are unmodified") + out.toSeq should be (annotations) + + fileContainsAnnotations(new File(dir, file.toString), expected) + + info(s"file '$serializedFileName' exists") + new File(serializedFileName) should (exist) + } + + it should "error if multiple annotations try to write to the same file" in new Fixture { + val file = new File("write-CustomFileEmission-annotations-error.anno.json") + val annotations = Seq( TargetDirAnnotation(dir), + OutputAnnotationFileAnnotation(file.toString), + WriteOutputAnnotationsSpec.Custom("foo"), + WriteOutputAnnotationsSpec.Custom("bar") ) + intercept[PhaseException] { + phase.transform(annotations) + }.getMessage should startWith ("Multiple CustomFileEmission annotations") + } + } private object WriteOutputAnnotationsSpec { + case object FooAnnotation extends NoTargetAnnotation + case class BarAnnotation(x: Int) extends NoTargetAnnotation + + case class Custom(value: String) extends NoTargetAnnotation with CustomFileEmission { + + override protected def baseFileName(a: AnnotationSeq): String = "Custom" + + override protected def suffix: Option[String] = Some(".Emission") + + override def getBytes: Iterable[Byte] = value.getBytes + + override def replacements(file: File): AnnotationSeq = Seq(Replacement(file.toString)) + + } + + case class Replacement(file: String) extends NoTargetAnnotation + } diff --git a/src/test/scala/firrtlTests/transforms/LegalizeReductions.scala b/src/test/scala/firrtlTests/transforms/LegalizeReductions.scala index 664701c3..5368c54c 100644 --- a/src/test/scala/firrtlTests/transforms/LegalizeReductions.scala +++ b/src/test/scala/firrtlTests/transforms/LegalizeReductions.scala @@ -65,7 +65,7 @@ circuit $name : TargetDirAnnotation(testDir.toString) :: CompilerAnnotation(new MinimumVerilogCompiler) :: Nil - val resultAnnos = (new FirrtlStage).run(annos) + val resultAnnos = (new FirrtlStage).transform(annos) val outputFilename = resultAnnos.collectFirst { case OutputFileAnnotation(f) => f } outputFilename.toRight(s"Output file not found!") // Copy Verilator harness -- cgit v1.2.3