diff --git a/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala b/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala index 7c24e3452f6..41d7f2fbc34 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala @@ -107,7 +107,8 @@ object Definition extends SourceInfoDoc { context.warningFilters, context.sourceRoots, Some(context.globalNamespace), - Builder.allDefinitions + Builder.allDefinitions, + context.loggerOptions ) } dynamicContext.inDefinition = true diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 2e31c82bb7d..51c057271f1 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -11,7 +11,7 @@ import chisel3.properties.Class import chisel3.internal.firrtl.ir._ import chisel3.internal.firrtl.Converter import chisel3.internal.naming._ -import _root_.firrtl.annotations.{CircuitName, ComponentName, IsMember, ModuleName, Named, ReferenceTarget} +import _root_.firrtl.annotations.{Annotation, CircuitName, ComponentName, IsMember, ModuleName, Named, ReferenceTarget} import _root_.firrtl.annotations.AnnotationUtils.validComponentName import _root_.firrtl.AnnotationSeq import _root_.firrtl.renamemap.MutableRenameMap @@ -19,13 +19,14 @@ import _root_.firrtl.util.BackendCompilationUtilities._ import _root_.firrtl.{ir => fir} import chisel3.experimental.dataview.{reify, reifySingleData} import chisel3.internal.Builder.Prefix -import logger.LazyLogging +import logger.{LazyLogging, LoggerOption} import scala.collection.mutable import scala.annotation.tailrec import java.io.File import scala.util.control.NonFatal import chisel3.ChiselException +import logger.LoggerOptions private[chisel3] class Namespace(keywords: Set[String], separator: Char = '_') { // This HashMap is compressed, not every name in the namespace is present here. @@ -452,7 +453,8 @@ private[chisel3] class DynamicContext( val sourceRoots: Seq[File], val defaultNamespace: Option[Namespace], // Definitions from other scopes in the same elaboration, use allDefinitions below - val outerScopeDefinitions: List[Iterable[Definition[_]]]) { + val outerScopeDefinitions: List[Iterable[Definition[_]]], + val loggerOptions: LoggerOptions) { val importedDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a } // Map from proto module name to ext-module name @@ -1000,6 +1002,17 @@ private[chisel3] object Builder extends LazyLogging { f: => T, dynamicContext: DynamicContext, forceModName: Boolean = true + ): (Circuit, T) = { + // Logger has its own context separate from Chisel's dynamic context + _root_.logger.Logger.makeScope(dynamicContext.loggerOptions) { + buildImpl(f, dynamicContext, forceModName) + } + } + + private def buildImpl[T <: BaseModule]( + f: => T, + dynamicContext: DynamicContext, + forceModName: Boolean ): (Circuit, T) = { dynamicContextVar.withValue(Some(dynamicContext)) { ViewParent: Unit // Must initialize the singleton in a Builder context or weird things can happen diff --git a/docs/src/cookbooks/cookbook.md b/docs/src/cookbooks/cookbook.md index ec03a334fd2..312636f2976 100644 --- a/docs/src/cookbooks/cookbook.md +++ b/docs/src/cookbooks/cookbook.md @@ -832,9 +832,6 @@ If the indexee is a non-power-of-2 size, use the ceiling of the log2 result. ```scala mdoc:invisible:reset import chisel3._ -// Some other test is clobbering the global Logger which breaks the warnings below -// Setting the output stream to the Console fixes the issue -logger.Logger.setConsole() // Helper to throw away return value so it doesn't show up in mdoc def compile(gen: => chisel3.RawModule): Unit = { circt.stage.ChiselStage.emitCHIRRTL(gen) diff --git a/docs/src/explanations/warnings.md b/docs/src/explanations/warnings.md index b6223ac748c..24af2fa83b0 100644 --- a/docs/src/explanations/warnings.md +++ b/docs/src/explanations/warnings.md @@ -61,9 +61,6 @@ The supported actions are: The following example issues a warning when elaborated normally ```scala mdoc:invisible:reset -// Some other test is clobbering the global Logger which breaks the warnings below -// Setting the output stream to the Console fixes the issue -logger.Logger.setConsole() // Helper to throw away return value so it doesn't show up in mdoc def compile(gen: => chisel3.RawModule, args: Array[String] = Array()): Unit = { circt.stage.ChiselStage.emitCHIRRTL(gen, args = args) diff --git a/firrtl/src/main/scala/logger/Logger.scala b/firrtl/src/main/scala/logger/Logger.scala index f14eec7178e..06bd750e2d8 100644 --- a/firrtl/src/main/scala/logger/Logger.scala +++ b/firrtl/src/main/scala/logger/Logger.scala @@ -121,7 +121,18 @@ object Logger { * @tparam A return type of the code block * @return the original return of the code block */ - def makeScope[A](options: AnnotationSeq = Seq.empty)(codeBlock: => A): A = { + def makeScope[A](options: AnnotationSeq = Seq.empty)(codeBlock: => A): A = + makeScope(() => setOptions(options))(codeBlock) + + /** Set a scope for this logger based on available annotations + * @param options LoggerOptions to use + * @param codeBlock some Scala code over which to define this scope + * @tparam A return type of the code block + * @return the original return of the code block + */ + def makeScope[A](options: LoggerOptions)(codeBlock: => A): A = makeScope(() => setOptions(options))(codeBlock) + + private def makeScope[A](doSetOptions: () => Unit)(codeBlock: => A): A = { val runState: LoggerState = { val newRunState = updatableLoggerState.value.getOrElse(new LoggerState) if (newRunState.fromInvoke) { @@ -133,7 +144,7 @@ object Logger { } } updatableLoggerState.withValue(Some(runState)) { - setOptions(options) + doSetOptions() codeBlock } } @@ -326,20 +337,26 @@ object Logger { Seq(new AddDefaults, Checks) .foldLeft(inputAnnotations)((a, p) => p.transform(a)) - val lopts = view[LoggerOptions](annotations) - state.globalLevel = (state.globalLevel, lopts.globalLogLevel) match { + setOptions(view[LoggerOptions](annotations)) + } + + /** Set logger options + * @param options options to set + */ + def setOptions(options: LoggerOptions): Unit = { + state.globalLevel = (state.globalLevel, options.globalLogLevel) match { case (LogLevel.None, LogLevel.None) => LogLevel.None case (x, LogLevel.None) => x case (LogLevel.None, x) => x case (_, x) => x } - setClassLogLevels(lopts.classLogLevels) + setClassLogLevels(options.classLogLevels) - if (lopts.logFileName.nonEmpty) { - setOutput(lopts.logFileName.get) + if (options.logFileName.nonEmpty) { + setOutput(options.logFileName.get) } - state.logClassNames = lopts.logClassNames + state.logClassNames = options.logClassNames } } diff --git a/firrtl/src/main/scala/logger/LoggerAnnotations.scala b/firrtl/src/main/scala/logger/LoggerAnnotations.scala index 0185492e363..1f11ce42179 100644 --- a/firrtl/src/main/scala/logger/LoggerAnnotations.scala +++ b/firrtl/src/main/scala/logger/LoggerAnnotations.scala @@ -3,7 +3,7 @@ package logger import firrtl.annotations.{Annotation, NoTargetAnnotation} -import firrtl.options.{HasShellOptions, ShellOption} +import firrtl.options.{HasShellOptions, ShellOption, Unserializable} /** An annotation associated with a Logger command line option */ sealed trait LoggerOption { this: Annotation => } @@ -16,6 +16,7 @@ sealed trait LoggerOption { this: Annotation => } case class LogLevelAnnotation(globalLogLevel: LogLevel.Value = LogLevel.None) extends NoTargetAnnotation with LoggerOption + with Unserializable object LogLevelAnnotation extends HasShellOptions { @@ -39,6 +40,7 @@ object LogLevelAnnotation extends HasShellOptions { case class ClassLogLevelAnnotation(className: String, level: LogLevel.Value) extends NoTargetAnnotation with LoggerOption + with Unserializable object ClassLogLevelAnnotation extends HasShellOptions { @@ -63,7 +65,7 @@ object ClassLogLevelAnnotation extends HasShellOptions { * - maps to [[LoggerOptions.logFileName]] * - enabled with `--log-file` */ -case class LogFileAnnotation(file: Option[String]) extends NoTargetAnnotation with LoggerOption +case class LogFileAnnotation(file: Option[String]) extends NoTargetAnnotation with LoggerOption with Unserializable object LogFileAnnotation extends HasShellOptions { @@ -81,7 +83,11 @@ object LogFileAnnotation extends HasShellOptions { /** Enables class names in log output * - enabled with `-lcn/--log-class-names` */ -case object LogClassNamesAnnotation extends NoTargetAnnotation with LoggerOption with HasShellOptions { +case object LogClassNamesAnnotation + extends NoTargetAnnotation + with LoggerOption + with HasShellOptions + with Unserializable { val options = Seq( new ShellOption[Unit]( diff --git a/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala b/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala index dde1a39411f..24b7fc0d2f4 100644 --- a/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala +++ b/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala @@ -12,6 +12,7 @@ import firrtl.options.Viewer.view import firrtl.{ir, _} import scala.collection.mutable +import logger.LoggerOptions /** Aspect to inject Chisel code into a module of type M * @@ -57,6 +58,7 @@ abstract class InjectorAspect[T <: RawModule, M <: RawModule]( final def toAnnotation(modules: Iterable[M], circuit: String, moduleNames: Seq[String]): AnnotationSeq = { modules.map { module => val chiselOptions = view[ChiselOptions](annotationsInAspect) + val loggerOptions = view[LoggerOptions](annotationsInAspect) val dynamicContext = new DynamicContext( annotationsInAspect, @@ -64,7 +66,8 @@ abstract class InjectorAspect[T <: RawModule, M <: RawModule]( chiselOptions.warningFilters, chiselOptions.sourceRoots, None, - Nil // FIXME this maybe should somehow grab definitions from earlier elaboration + Nil, // FIXME this maybe should somehow grab definitions from earlier elaboration + loggerOptions ) // Add existing module names into the namespace. If injection logic instantiates new modules // which would share the same name, they will get uniquified accordingly diff --git a/src/main/scala/chisel3/stage/phases/Elaborate.scala b/src/main/scala/chisel3/stage/phases/Elaborate.scala index 0d0f343f0a4..06b19197d3e 100644 --- a/src/main/scala/chisel3/stage/phases/Elaborate.scala +++ b/src/main/scala/chisel3/stage/phases/Elaborate.scala @@ -13,14 +13,18 @@ import chisel3.stage.{ ThrowOnFirstErrorAnnotation } import firrtl.AnnotationSeq -import firrtl.options.Phase +import firrtl.options.{Dependency, Phase} import firrtl.options.Viewer.view +import logger.LoggerOptions /** Elaborate all [[chisel3.stage.ChiselGeneratorAnnotation]]s into [[chisel3.stage.ChiselCircuitAnnotation]]s. */ class Elaborate extends Phase { - override def prerequisites = Seq.empty + override def prerequisites: Seq[Dependency[Phase]] = Seq( + Dependency[chisel3.stage.phases.Checks], + Dependency(_root_.logger.phases.Checks) + ) override def optionalPrerequisites = Seq.empty override def optionalPrerequisiteOf = Seq.empty override def invalidates(a: Phase) = false @@ -28,6 +32,7 @@ class Elaborate extends Phase { def transform(annotations: AnnotationSeq): AnnotationSeq = annotations.flatMap { case ChiselGeneratorAnnotation(gen) => val chiselOptions = view[ChiselOptions](annotations) + val loggerOptions = view[LoggerOptions](annotations) try { val context = new DynamicContext( @@ -36,7 +41,8 @@ class Elaborate extends Phase { chiselOptions.warningFilters, chiselOptions.sourceRoots, None, - Nil + Nil, + loggerOptions ) val (circuit, dut) = Builder.build(Module(gen()), context) diff --git a/src/main/scala/circt/stage/ChiselStage.scala b/src/main/scala/circt/stage/ChiselStage.scala index 7b9084a0a99..1662544d87e 100644 --- a/src/main/scala/circt/stage/ChiselStage.scala +++ b/src/main/scala/circt/stage/ChiselStage.scala @@ -3,49 +3,13 @@ package circt.stage import chisel3.RawModule -import chisel3.stage.{ - ChiselCircuitAnnotation, - ChiselGeneratorAnnotation, - CircuitSerializationAnnotation, - PrintFullStackTraceAnnotation, - SourceRootAnnotation, - ThrowOnFirstErrorAnnotation, - WarningConfigurationAnnotation, - WarningConfigurationFileAnnotation, - WarningsAsErrorsAnnotation -} +import chisel3.stage.{ChiselCircuitAnnotation, ChiselGeneratorAnnotation, CircuitSerializationAnnotation} import chisel3.stage.CircuitSerializationAnnotation.FirrtlFileFormat import firrtl.{AnnotationSeq, EmittedVerilogCircuitAnnotation} -import firrtl.options.{ - BareShell, - CustomFileEmission, - Dependency, - Phase, - PhaseManager, - Shell, - Stage, - StageMain, - Unserializable -} +import firrtl.options.{CustomFileEmission, Dependency, Phase, PhaseManager, Stage, StageMain, Unserializable} import firrtl.stage.FirrtlCircuitAnnotation -trait CLI { this: BareShell => - parser.note("CIRCT (MLIR FIRRTL Compiler) options") - Seq( - CIRCTTargetAnnotation, - PreserveAggregate, - ChiselGeneratorAnnotation, - PrintFullStackTraceAnnotation, - ThrowOnFirstErrorAnnotation, - WarningsAsErrorsAnnotation, - WarningConfigurationAnnotation, - WarningConfigurationFileAnnotation, - SourceRootAnnotation, - SplitVerilog, - FirtoolBinaryPath, - DumpFir - ).foreach(_.addOptions(parser)) -} +import logger.LogLevelAnnotation /** Entry point for running Chisel with the CIRCT compiler. * @@ -58,13 +22,15 @@ class ChiselStage extends Stage { override def optionalPrerequisiteOf = Seq.empty override def invalidates(a: Phase) = false - override val shell = new Shell("circt") with CLI + override val shell = new firrtl.options.Shell("circt") with CLI { + // These are added by firrtl.options.Shell (which we must extend because we are a Stage) + override protected def includeLoggerOptions = false + } override def run(annotations: AnnotationSeq): AnnotationSeq = { val pm = new PhaseManager( targets = Seq( - Dependency[chisel3.stage.phases.Checks], Dependency[chisel3.stage.phases.AddImplicitOutputFile], Dependency[chisel3.stage.phases.AddImplicitOutputAnnotationFile], Dependency[chisel3.stage.phases.MaybeAspectPhase], @@ -73,7 +39,6 @@ class ChiselStage extends Stage { Dependency[chisel3.stage.phases.AddDedupGroupAnnotations], Dependency[chisel3.stage.phases.MaybeInjectingPhase], Dependency[circt.stage.phases.AddImplicitOutputFile], - Dependency[circt.stage.phases.Checks], Dependency[circt.stage.phases.CIRCT] ), currentState = Seq( @@ -92,7 +57,6 @@ object ChiselStage { /** A phase shared by all the CIRCT backends */ private def phase = new PhaseManager( Seq( - Dependency[chisel3.stage.phases.Checks], Dependency[chisel3.aop.injecting.InjectingPhase], Dependency[chisel3.stage.phases.Elaborate], Dependency[chisel3.stage.phases.Convert], @@ -111,7 +75,7 @@ object ChiselStage { val annos = Seq( ChiselGeneratorAnnotation(() => gen), CIRCTTargetAnnotation(CIRCTTarget.CHIRRTL) - ) ++ (new BareShell("circt") with CLI).parse(args) + ) ++ (new Shell("circt")).parse(args) val resultAnnos = phase.transform(annos) @@ -138,7 +102,7 @@ object ChiselStage { val annos = Seq( ChiselGeneratorAnnotation(() => gen), CIRCTTargetAnnotation(CIRCTTarget.CHIRRTL) - ) ++ (new BareShell("circt") with CLI).parse(args) + ) ++ (new Shell("circt")).parse(args) phase .transform(annos) @@ -157,7 +121,7 @@ object ChiselStage { val annos = Seq( ChiselGeneratorAnnotation(() => gen), CIRCTTargetAnnotation(CIRCTTarget.FIRRTL) - ) ++ (new BareShell("circt") with CLI).parse(args) ++ firtoolOpts.map(FirtoolOption(_)) + ) ++ (new Shell("circt")).parse(args) ++ firtoolOpts.map(FirtoolOption(_)) phase .transform(annos) @@ -176,7 +140,7 @@ object ChiselStage { val annos = Seq( ChiselGeneratorAnnotation(() => gen), CIRCTTargetAnnotation(CIRCTTarget.HW) - ) ++ (new BareShell("circt") with CLI).parse(args) ++ firtoolOpts.map(FirtoolOption(_)) + ) ++ (new Shell("circt")).parse(args) ++ firtoolOpts.map(FirtoolOption(_)) phase .transform(annos) @@ -201,7 +165,7 @@ object ChiselStage { val annos = Seq( ChiselGeneratorAnnotation(() => gen), CIRCTTargetAnnotation(CIRCTTarget.SystemVerilog) - ) ++ (new BareShell("circt") with CLI).parse(args) ++ firtoolOpts.map(FirtoolOption(_)) + ) ++ (new Shell("circt")).parse(args) ++ firtoolOpts.map(FirtoolOption(_)) phase .transform(annos) .collectFirst { diff --git a/src/main/scala/circt/stage/Shell.scala b/src/main/scala/circt/stage/Shell.scala new file mode 100644 index 00000000000..1385ddd3151 --- /dev/null +++ b/src/main/scala/circt/stage/Shell.scala @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: Apache-2.0 + +package circt.stage + +import chisel3.stage.{ + ChiselCircuitAnnotation, + ChiselGeneratorAnnotation, + CircuitSerializationAnnotation, + PrintFullStackTraceAnnotation, + SourceRootAnnotation, + ThrowOnFirstErrorAnnotation, + WarningConfigurationAnnotation, + WarningConfigurationFileAnnotation, + WarningsAsErrorsAnnotation +} +import firrtl.options.BareShell +import firrtl.options.TargetDirAnnotation +import logger.{ClassLogLevelAnnotation, LogClassNamesAnnotation, LogFileAnnotation, LogLevelAnnotation} + +trait CLI { this: BareShell => + + /** Include options for logging + * + * Defaults to true, override to false when mixing in to [[firrtl.options.Shell]] for use in a [[firrtl.options.Phase]] + */ + protected def includeLoggerOptions: Boolean = true + + if (includeLoggerOptions) { + parser.note("Logger options") + Seq(LogLevelAnnotation, ClassLogLevelAnnotation, LogClassNamesAnnotation).foreach(_.addOptions(parser)) + } + + parser.note("Chisel options") + Seq( + ChiselGeneratorAnnotation, + PrintFullStackTraceAnnotation, + ThrowOnFirstErrorAnnotation, + WarningsAsErrorsAnnotation, + WarningConfigurationAnnotation, + WarningConfigurationFileAnnotation, + SourceRootAnnotation, + DumpFir + ).foreach(_.addOptions(parser)) + + parser.note("CIRCT (MLIR FIRRTL Compiler) options") + Seq( + CIRCTTargetAnnotation, + PreserveAggregate, + SplitVerilog, + FirtoolBinaryPath + ).foreach(_.addOptions(parser)) +} + +private trait LoggerOptions {} + +/** Default Shell for [[ChiselStage]] + */ +private class Shell(applicationName: String) extends BareShell(applicationName) with CLI diff --git a/src/test/scala/circtTests/stage/ChiselStageSpec.scala b/src/test/scala/circtTests/stage/ChiselStageSpec.scala index 51657c3f619..f0d24d48721 100644 --- a/src/test/scala/circtTests/stage/ChiselStageSpec.scala +++ b/src/test/scala/circtTests/stage/ChiselStageSpec.scala @@ -365,18 +365,40 @@ class ChiselStageSpec extends AnyFunSpec with Matchers with chiselTests.Utils { } it("should forward firtool-resolver logging under log-level debug") { + val targetDir = baseDir / "should-forward-firtool-resolver-logging-under-log-level-debug" + + val args: Array[String] = Array( + "--target", + "systemverilog", + "--target-dir", + targetDir.toString + ) + val annos = Seq(ChiselGeneratorAnnotation(() => new ChiselStageSpec.Foo)) + val msg = "Checking CHISEL_FIRTOOL_PATH for firtool" // By default it should NOT show anything val (log1, _) = grabLog { - ChiselStage.emitSystemVerilog(new ChiselStageSpec.Foo) + (new ChiselStage).execute(args, annos) } - log1 shouldNot include("Checking CHISEL_FIRTOOL_PATH for firtool") + log1 shouldNot include(msg) - // circt.stage.ChiselStage does not currently accept --log-level so we have to use testing - // APIs to set the level - val (log2, _) = grabLogLevel(LogLevel.Debug) { - ChiselStage.emitSystemVerilog(new ChiselStageSpec.Foo) + val (log2, _) = grabLog { + (new ChiselStage).execute(args ++ Array("--log-level", "debug"), annos) } - log2 should include("Checking CHISEL_FIRTOOL_PATH for firtool") + log2 should include(msg) + } + + it("should support --log-level") { + def args(level: String): Array[String] = Array("--target", "chirrtl", "--log-level", level) + val annos = Seq(ChiselGeneratorAnnotation(() => new ChiselStageSpec.Foo)) + val (log1, _) = grabLog { + (new ChiselStage).execute(args("info"), annos) + } + log1 should include("Done elaborating.") + + val (log2, _) = grabLog { + (new ChiselStage).execute(args("warn"), annos) + } + log2 shouldNot include("Done elaborating.") } } @@ -1085,6 +1107,38 @@ class ChiselStageSpec extends AnyFunSpec with Matchers with chiselTests.Utils { } + it("should forward firtool-resolver logging under log-level debug") { + // By default it should NOT show anything + val (log1, _) = grabLog { + ChiselStage.emitSystemVerilog(new ChiselStageSpec.Foo) + } + log1 shouldNot include("Checking CHISEL_FIRTOOL_PATH for firtool") + + // circt.stage.ChiselStage does not currently accept --log-level so we have to use testing + // APIs to set the level + val (log2, _) = grabLog { + ChiselStage.emitSystemVerilog(new ChiselStageSpec.Foo, args = Array("--log-level", "debug")) + } + log2 should include("Checking CHISEL_FIRTOOL_PATH for firtool") + } + + it("should support --log-level") { + val (log1, _) = grabLog { + ChiselStage.emitCHIRRTL(new ChiselStageSpec.Foo, Array("--log-level", "info")) + } + log1 should include("Done elaborating.") + + val (log2, _) = grabLog { + ChiselStage.emitCHIRRTL(new ChiselStageSpec.Foo, Array("--log-level", "warn")) + } + log2 shouldNot include("Done elaborating.") + } + + it("should not emit logger annotations") { + import chisel3.RawModule + (ChiselStage.emitCHIRRTL(new RawModule {}) should not).include("LogLevelAnnotation") + (ChiselStage.emitCHIRRTL(new RawModule {}, Array("-ll", "info")) should not).include("LogLevelAnnotation") + } } describe("ChiselStage$ exception handling") {