From cbe90114f7693d40aaf76151f7b132c09aa32859 Mon Sep 17 00:00:00 2001 From: Jack Date: Mon, 18 Dec 2017 00:06:10 -0800 Subject: Add support for multiple annotation files Change loadAnnotations to return annotations instead of mutating firrtlOptions Deprecate implicit annotation file (top.anno) and annotation file override --- src/main/scala/firrtl/Driver.scala | 68 ++++++++++++++-------- .../scala/firrtl/ExecutionOptionsManager.scala | 12 ++-- src/test/scala/firrtlTests/DriverSpec.scala | 46 ++++++++++++--- 3 files changed, 90 insertions(+), 36 deletions(-) (limited to 'src') diff --git a/src/main/scala/firrtl/Driver.scala b/src/main/scala/firrtl/Driver.scala index b5eb1531..bc5102d2 100644 --- a/src/main/scala/firrtl/Driver.scala +++ b/src/main/scala/firrtl/Driver.scala @@ -89,34 +89,58 @@ object Driver { println("-"*78 + Console.RESET) } - /** - * Load annotation file based on options - * @param optionsManager use optionsManager config to load annotation file if it exists - * update the firrtlOptions with new annotations if it does + /** Load annotations from specified files and options + * + * @param optionsManager use optionsManager config to load annotation files + * @return Annotations read from files */ - def loadAnnotations(optionsManager: ExecutionOptionsManager with HasFirrtlOptions): Unit = { + //scalastyle:off cyclomatic.complexity method.length + def loadAnnotations( + optionsManager: ExecutionOptionsManager with HasFirrtlOptions + ): Seq[Annotation] = { + val firrtlConfig = optionsManager.firrtlOptions + + //noinspection ScalaDeprecation + val oldAnnoFileName = firrtlConfig.getAnnotationFileName(optionsManager) + val oldAnnoFile = new File(oldAnnoFileName).getCanonicalFile + + val (annoFiles, usingImplicitAnnoFile) = { + val afs = firrtlConfig.annotationFileNames.map { x => + new File(x).getCanonicalFile + } + // Implicit anno file could be included explicitly, only include it and + // warn if it's not also explicit + val use = oldAnnoFile.exists && !afs.contains(oldAnnoFile) + if (use) (oldAnnoFile +: afs, true) else (afs, false) + } - def firrtlConfig = optionsManager.firrtlOptions + // Warnings to get people to change to drop old API + if (firrtlConfig.annotationFileNameOverride.nonEmpty) { + val msg = "annotationFileNameOverride is deprecated! " + + "Use annotationFileNames" + Driver.dramaticWarning(msg) + } else if (usingImplicitAnnoFile) { + val msg = "Implicit .anno file from top-name is deprecated!\n" + + (" "*9) + "Use explicit -faf option or annotationFileNames" + Driver.dramaticWarning(msg) + } + + val loadedAnnos = annoFiles.flatMap { file => + if (!file.exists) { + throw new FileNotFoundException(s"Annotation file $file not found!") + } + val yaml = io.Source.fromFile(file).getLines().mkString("\n").parseYaml + yaml.convertTo[List[Annotation]] - val annotationFileName = firrtlConfig.getAnnotationFileName(optionsManager) - val annotationFile = new File(annotationFileName) - if (annotationFile.exists) { - val annotationsYaml = io.Source.fromFile(annotationFile).getLines().mkString("\n").parseYaml - val annotationArray = annotationsYaml.convertTo[Array[Annotation]] - optionsManager.firrtlOptions = firrtlConfig.copy(annotations = firrtlConfig.annotations ++ annotationArray) } - if(firrtlConfig.annotations.nonEmpty) { - val targetDirAnno = List(Annotation( + val targetDirAnno = + List(Annotation( CircuitName("All"), classOf[BlackBoxSourceHelper], BlackBoxTargetDir(optionsManager.targetDirName).serialize )) - optionsManager.firrtlOptions = optionsManager.firrtlOptions.copy( - annotations = firrtlConfig.annotations ++ targetDirAnno) - } - // Output Annotations val outputAnnos = firrtlConfig.getEmitterAnnos(optionsManager) @@ -124,9 +148,7 @@ object Driver { (if (firrtlConfig.dontCheckCombLoops) Seq(DontCheckCombLoopsAnnotation()) else Seq()) ++ (if (firrtlConfig.noDCE) Seq(NoDCEAnnotation()) else Seq()) - optionsManager.firrtlOptions = optionsManager.firrtlOptions.copy( - annotations = firrtlConfig.annotations ++ outputAnnos ++ globalAnnos) - + targetDirAnno ++ outputAnnos ++ globalAnnos ++ firrtlConfig.annotations ++ loadedAnnos } /** @@ -169,7 +191,7 @@ object Driver { } } - loadAnnotations(optionsManager) + val annos = loadAnnotations(optionsManager) val parsedInput = Parser.parse(firrtlSource, firrtlConfig.infoMode) @@ -179,7 +201,7 @@ object Driver { val finalState = firrtlConfig.compiler.compile( CircuitState(parsedInput, ChirrtlForm, - Some(AnnotationMap(firrtlConfig.annotations))), + Some(AnnotationMap(annos))), firrtlConfig.customTransforms ) diff --git a/src/main/scala/firrtl/ExecutionOptionsManager.scala b/src/main/scala/firrtl/ExecutionOptionsManager.scala index 8bbb8dcd..e5bd95ef 100644 --- a/src/main/scala/firrtl/ExecutionOptionsManager.scala +++ b/src/main/scala/firrtl/ExecutionOptionsManager.scala @@ -177,6 +177,7 @@ case class FirrtlExecutionOptions( firrtlSource: Option[String] = None, customTransforms: Seq[Transform] = List.empty, annotations: List[Annotation] = List.empty, + annotationFileNames: List[String] = List.empty, annotationFileNameOverride: String = "", outputAnnotationFileName: String = "", emitOneFilePerModule: Boolean = false, @@ -273,6 +274,7 @@ case class FirrtlExecutionOptions( * @param optionsManager this is needed to access build function and its common options * @return */ + @deprecated("Use FirrtlOptions.annotationFileNames instead", "1.1") def getAnnotationFileName(optionsManager: ExecutionOptionsManager): String = { optionsManager.getBuildFileName("anno", annotationFileNameOverride) } @@ -309,12 +311,12 @@ trait HasFirrtlOptions { parser.opt[String]("annotation-file") .abbr("faf") - .valueName ("") + .unbounded() + .valueName("") .foreach { x => - firrtlOptions = firrtlOptions.copy(annotationFileNameOverride = x) - }.text { - "use this to override the default annotation file name, default is empty" - } + val annoFiles = x +: firrtlOptions.annotationFileNames + firrtlOptions = firrtlOptions.copy(annotationFileNames = annoFiles) + }.text("Used to specify annotation files (can appear multiple times)") parser.opt[Unit]("force-append-anno-file") .abbr("ffaaf") diff --git a/src/test/scala/firrtlTests/DriverSpec.scala b/src/test/scala/firrtlTests/DriverSpec.scala index b589f69a..3e3b7570 100644 --- a/src/test/scala/firrtlTests/DriverSpec.scala +++ b/src/test/scala/firrtlTests/DriverSpec.scala @@ -2,7 +2,7 @@ package firrtlTests -import java.io.{File, FileNotFoundException, FileOutputStream} +import java.io.File import org.scalatest.{FreeSpec, Matchers} import firrtl.passes.InlineInstances @@ -150,7 +150,23 @@ class DriverSpec extends FreeSpec with Matchers with BackendCompilationUtilities } } - "Annotations can be read from a file" in { + // Deprecated + "Annotations can be read implicitly from the name of the circuit" in { + val top = "foo" + val optionsManager = new ExecutionOptionsManager("test") with HasFirrtlOptions { + commonOptions = commonOptions.copy(topName = top) + } + val annoFile = new File(optionsManager.commonOptions.targetDirName, top + ".anno") + copyResourceToFile("/annotations/SampleAnnotations.anno", annoFile) + optionsManager.firrtlOptions.annotations.length should be (0) + val annos = Driver.loadAnnotations(optionsManager) + annos.length should be (12) // 9 from circuit plus 3 general purpose + annos.count(_.transformClass == "firrtl.passes.InlineInstances") should be (9) + annoFile.delete() + } + + // Deprecated + "Annotations can be read using annotationFileNameOverride" in { val optionsManager = new ExecutionOptionsManager("test") with HasFirrtlOptions { commonOptions = commonOptions.copy(topName = "a.fir") firrtlOptions = firrtlOptions.copy( @@ -160,10 +176,26 @@ class DriverSpec extends FreeSpec with Matchers with BackendCompilationUtilities val annotationsTestFile = new File(optionsManager.commonOptions.targetDirName, optionsManager.firrtlOptions.annotationFileNameOverride + ".anno") copyResourceToFile("/annotations/SampleAnnotations.anno", annotationsTestFile) optionsManager.firrtlOptions.annotations.length should be (0) - Driver.loadAnnotations(optionsManager) - optionsManager.firrtlOptions.annotations.length should be (12) // 9 from circuit plus 3 general purpose + val annos = Driver.loadAnnotations(optionsManager) + annos.length should be (12) // 9 from circuit plus 3 general purpose + annos.count(_.transformClass == "firrtl.passes.InlineInstances") should be (9) + annotationsTestFile.delete() + } - optionsManager.firrtlOptions.annotations.head.transformClass should be ("firrtl.passes.InlineInstances") + "Annotations can be read from multiple files" in { + val filename = "SampleAnnotations.anno" + val optionsManager = new ExecutionOptionsManager("test") with HasFirrtlOptions { + commonOptions = commonOptions.copy(topName = "a.fir") + firrtlOptions = firrtlOptions.copy( + annotationFileNames = List.fill(2)(filename) // just read the safe file twice + ) + } + val annotationsTestFile = new File(optionsManager.commonOptions.targetDirName, filename) + copyResourceToFile(s"/annotations/$filename", annotationsTestFile) + optionsManager.firrtlOptions.annotations.length should be (0) + val annos = Driver.loadAnnotations(optionsManager) + annos.length should be (21) // 18 from files plus 3 general purpose + annos.count(_.transformClass == "firrtl.passes.InlineInstances") should be (18) annotationsTestFile.delete() } @@ -181,9 +213,7 @@ class DriverSpec extends FreeSpec with Matchers with BackendCompilationUtilities val firrtlOptions = optionsManager.firrtlOptions firrtlOptions.annotations.length should be (1) // infer-rw - Driver.loadAnnotations(optionsManager) - - val anns = optionsManager.firrtlOptions.annotations.groupBy(_.transform) + val anns = Driver.loadAnnotations(optionsManager).groupBy(_.transform) anns(classOf[BlackBoxSourceHelper]).length should be (1) // built in to loadAnnotations anns(classOf[InferReadWrite]).length should be (1) // --infer-rw anns(classOf[InlineInstances]).length should be (9) // annotations file -- cgit v1.2.3