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 fb2a241ec97..83e5f6e9ecd 100644 --- a/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala +++ b/core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala @@ -103,7 +103,20 @@ object Definition extends SourceInfoDoc { ): Definition[T] = { val dynamicContext = { val context = Builder.captureContext() +<<<<<<< HEAD new DynamicContext(Nil, context.throwOnFirstError, context.warningsAsErrors, context.sourceRoots) +======= + new DynamicContext( + Nil, + context.throwOnFirstError, + context.legacyShiftRightWidth, + context.warningFilters, + context.sourceRoots, + Some(context.globalNamespace), + Builder.allDefinitions, + context.loggerOptions + ) +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) } Builder.globalNamespace.copyTo(dynamicContext.globalNamespace) dynamicContext.inDefinition = true diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index f0cbfaa5e8d..0e5660c5aeb 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -9,19 +9,24 @@ import chisel3.experimental._ import chisel3.experimental.hierarchy.core.{Clone, ImportDefinitionAnnotation, Instance} import chisel3.internal.firrtl._ 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 import _root_.firrtl.util.BackendCompilationUtilities._ 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 +<<<<<<< HEAD +======= +import chisel3.ChiselException +import logger.LoggerOptions +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) private[chisel3] class Namespace(keywords: Set[String]) { // This HashMap is compressed, not every name in the namespace is present here. @@ -425,11 +430,24 @@ private[chisel3] class ChiselContext() { } private[chisel3] class DynamicContext( +<<<<<<< HEAD val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean, val warningsAsErrors: Boolean, val sourceRoots: Seq[File]) { val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a } +======= + val annotationSeq: AnnotationSeq, + val throwOnFirstError: Boolean, + val legacyShiftRightWidth: Boolean, + val warningFilters: Seq[WarningFilter], + 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 loggerOptions: LoggerOptions) { + val importedDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a } +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) // Map holding the actual names of extModules // Pick the definition name by default in case not passed through annotation. @@ -836,6 +854,16 @@ 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) + } + } + + private def buildImpl[T <: BaseModule]( + f: => T, + dynamicContext: DynamicContext ): (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 f279d377a08..7c3d1916c96 100644 --- a/docs/src/cookbooks/cookbook.md +++ b/docs/src/cookbooks/cookbook.md @@ -784,8 +784,86 @@ If the index is too narrow you can use `.pad` to increase the width. ```scala mdoc:silent import chisel3.util.log2Up +<<<<<<< HEAD class TooNarrow(extracteeWidth: Int, indexWidth: Int) { val extractee = Wire(UInt(extracteeWidth.W)) +======= +Chisel will warn if a dynamic index is not the correctly-sized width for indexing a Vec or UInt. +"Correctly-sized" means that the width of the index should be the log2 of the size of the indexee. +If the indexee is a non-power-of-2 size, use the ceiling of the log2 result. + +```scala mdoc:invisible:reset +import chisel3._ +// 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) +} +``` + +When the index does not have enough bits to address all entries or bits in the extractee, you can `.pad` the index to increase the width. + +```scala mdoc +class TooNarrow extends RawModule { + val extractee = Wire(UInt(7.W)) + val index = Wire(UInt(2.W)) + extractee(index) +} +compile(new TooNarrow) +``` + +This can be fixed with `pad`: + +```scala mdoc +class TooNarrowFixed extends RawModule { + val extractee = Wire(UInt(7.W)) + val index = Wire(UInt(2.W)) + extractee(index.pad(3)) +} +compile(new TooNarrowFixed) +``` + +#### Use bit extraction when the index is too wide + +```scala mdoc +class TooWide extends RawModule { + val extractee = Wire(Vec(8, UInt(32.W))) + val index = Wire(UInt(4.W)) + extractee(index) +} +compile(new TooWide) +``` + +This can be fixed with bit extraction: + +```scala mdoc +class TooWideFixed extends RawModule { + val extractee = Wire(Vec(8, UInt(32.W))) + val index = Wire(UInt(4.W)) + extractee(index(2, 0)) +} +compile(new TooWideFixed) +``` + +Note that size 1 `Vecs` and `UInts` should be indexed by a zero-width `UInt`: + +```scala mdoc +class SizeOneVec extends RawModule { + val extractee = Wire(Vec(1, UInt(32.W))) + val index = Wire(UInt(0.W)) + extractee(index) +} +compile(new SizeOneVec) +``` + +Because `pad` only pads if the desired width is less than the current width of the argument, +you can use `pad` in conjunction with bit extraction when the widths may be too wide or too +narrow under different circumstances + +```scala mdoc +import chisel3.util.log2Ceil +class TooWideOrNarrow(extracteeSize: Int, indexWidth: Int) extends Module { + val extractee = Wire(Vec(extracteeSize, UInt(8.W))) +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) val index = Wire(UInt(indexWidth.W)) extractee(index.pad(log2Up(extracteeWidth))) } diff --git a/docs/src/explanations/warnings.md b/docs/src/explanations/warnings.md new file mode 100644 index 00000000000..24af2fa83b0 --- /dev/null +++ b/docs/src/explanations/warnings.md @@ -0,0 +1,145 @@ +--- +layout: docs +title: "Warnings" +section: "chisel3" +--- + +# Warnings + +Warnings in Chisel are used for deprecating old APIs or semantics for later removal. +As a matter of good software practice, Chisel users are encouraged to treat warnings as errors with `--warnings-as-errors`; +however, the coarse-grained nature of this option can be problematic when bumping Chisel which may introduce many warnings. +See [Warning Configuration](#warning-configuration) below for techniques to help deal with large numbers of warnings. + +## Warning Configuration + +Inspired by `-Wconf` [in Scala](https://www.scala-lang.org/2021/01/12/configuring-and-suppressing-warnings.html), +Chisel supports fine-grain control of warning behavior via the CLI options `--warn-conf` and `--warn-conf-file`. + +### Basic Operation + +`--warn-conf` accepts a comma-separated sequence of `:` pairs. +When a warning is hit in Chisel, the sequence of pairs are checked from left-to-right to see if the `filter` matches the warning. +The `action` associated with the first matching `filter` is the one used for the specific warning. +If no `filters` match, then the default behavior is to issue the warning. + +`--warn-conf` can be specified any number of times. +Earlier uses of `--warn-conf` take priority over later ones in the same left-to-right decreasing priority as the `filters` are checked within a single `--warn-conf`. +As a mental model, the user can pretend that all `--warn-conf` arguments concatenated together (separated by `,`) into a single argument. + +### Warning Configuration Files + +`--warn-conf-file` accepts a file which contains the same format of `:` pairs, separated by newlines. +Lines starting with `#` will be treated as comments and ignored. +`filters` are checked in decreasing priority from top-to-bottom of the file. + +A single command-line can contain any number of `--warn-conf-file` and any number of `--warn-conf` arguments. +The filters from all `--warn-conf*` arguments will be applied in the same left-to-right decreasing priority order. + +### Filters + +The supported filters are: + +* `any` - matches all warnings +* `id=` - matches warnings with the integer id +* `src=` - matches warnings when `` matches the source locator filename where the warning occurs + +`id` and `src` filters can be combined with `&`. +Any filter can have at most one `id` and at most one `src` listed. +`any` cannot be combined with any other filters. + +### Actions + +The supported actions are: + +* `:s` - suppress matching warnings +* `:w` - report matching warnings as warnings (default behavior) +* `:e` - error on matching warnings + +### Examples + +The following example issues a warning when elaborated normally + +```scala mdoc:invisible:reset +// 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) +} +``` + +```scala mdoc +import circt.stage.ChiselStage.emitSystemVerilog +import chisel3._ +class TooWideIndexModule extends RawModule { + val in = IO(Input(Vec(4, UInt(8.W)))) + val idx = IO(Input(UInt(8.W))) // This index is wider than necessary + val out = IO(Output(UInt(8.W))) + + out := in(idx) +} +compile(new TooWideIndexModule) +``` + +As shown in the warning, this warning is `W004` (which can be fixed [as described below](#w004-dynamic-index-too-wide)), we can suppress it with an `id` filter which will suppress all instances of this warning in the elaboration run. + +```scala mdoc +compile(new TooWideIndexModule, args = Array("--warn-conf", "id=4:s")) +``` + +It is generally advisable to make warning suppressions as precise as possible, so we could combine this `id` filter with a `src` glob filter for just this file: + +```scala mdoc +compile(new TooWideIndexModule, args = Array("--warn-conf", "id=4&src=**warnings.md:s")) +``` + +Finally, users are encouraged to treat warnings as errors to the extend possible, +so they should always end any warning configuration with `any:e` to elevate all unmatched warnings to errors: + +```scala mdoc +compile(new TooWideIndexModule, args = Array("--warn-conf", "id=4&src=**warnings.md:s,any:e")) +// Or +compile(new TooWideIndexModule, args = Array("--warn-conf", "id=4&src=**warnings.md:s", "--warn-conf", "any:e")) +// Or +compile(new TooWideIndexModule, args = Array("--warn-conf", "id=4&src=**warnings.md:s", "--warnings-as-errors")) +``` + +## Warning Glossary + +Chisel warnings have a unique identifier number to make them easier to lookup as well as so they can be configured as described above. + +### [W001] Unsafe UInt cast to ChiselEnum + +This warning occurs when casting a `UInt` to a `ChiselEnum` when there are values the `UInt` could take that are not legal states in the enumeration. +See the [ChiselEnum explanation](chisel-enum#casting) for more information and how to fix this warning. + +**Note:** This is the only warning that is not currently scheduled for become an error. + +### [W002] Dynamic bit select too wide + +This warning occurs when dynamically indexing a `UInt` or an `SInt` with an index that is wider than necessary to address all bits in the indexee. +It indicates that some of the high-bits of the index are ignored by the indexing operation. +It can be fixed as described in the [Cookbook](../cookbooks/cookbook#dynamic-index-too-wide-narrow). + +### [W003] Dynamic bit select too narrow + +This warning occurs when dynamically indexing a `UInt` or an `SInt` with an index that is to small to address all bits in the indexee. +It indicates that some bits of the indexee cannot be reached by the indexing operation. +It can be fixed as described in the [Cookbook](../cookbooks/cookbook#dynamic-index-too-wide-narrow). + +### [W004] Dynamic index too wide + +This warning occurs when dynamically indexing a `Vec` with an index that is wider than necessary to address all elements of the `Vec`. +It indicates that some of the high-bits of the index are ignored by the indexing operation. +It can be fixed as described in the [Cookbook](../cookbooks/cookbook#dynamic-index-too-wide-narrow). + +### [W005] Dynamic index too narrow + +This warning occurs when dynamically indexing a `Vec` with an index that is to small to address all elements in the `Vec`. +It indicates that some elements of the `Vec` cannot be reached by the indexing operation. +It can be fixed as described in the [Cookbook](../cookbooks/cookbook#dynamic-index-too-wide-narrow). + + +### [W006] Extract from Vec of size 0 + +This warning occurs when indexing a `Vec` with no elements. +It can be fixed by removing the indexing operation for the size zero `Vec` (perhaps via guarding with an `if-else` or `Option.when`). diff --git a/firrtl/src/main/scala/logger/Logger.scala b/firrtl/src/main/scala/logger/Logger.scala new file mode 100644 index 00000000000..06bd750e2d8 --- /dev/null +++ b/firrtl/src/main/scala/logger/Logger.scala @@ -0,0 +1,414 @@ +// SPDX-License-Identifier: Apache-2.0 + +package logger + +import java.io.{ByteArrayOutputStream, File, FileOutputStream, PrintStream} + +import firrtl.AnnotationSeq +import firrtl.options.Viewer.view +import logger.phases.{AddDefaults, Checks} + +import scala.util.DynamicVariable + +/** + * This provides a facility for a log4scala* type logging system. Why did we write our own? Because + * the canned ones are just too darned hard to turn on, particularly when embedded in a distribution. + * There are 4 main options: + * * A simple global option to turn on all in scope (and across threads, might want to fix this) + * * Turn on specific levels for specific fully qualified class names + * * Set a file to write things to, default is just to use stdout + * * Include the class names and level in the output. This is useful for figuring out what + * the class names that extend LazyLogger are. + * + * This is not overly optimized but does pass the string as () => String to avoid string interpolation + * occurring if the the logging level is not sufficiently high. This could be further optimized by playing + * with methods. + */ +/** + * The supported log levels, what do they mean? Whatever you want them to. + */ +object LogLevel extends Enumeration { + // None indicates "not set" + val Error, Warn, Info, Debug, Trace, None = Value + + def default = Warn + + def apply(s: String): LogLevel.Value = s.toLowerCase match { + case "error" => LogLevel.Error + case "warn" => LogLevel.Warn + case "info" => LogLevel.Info + case "debug" => LogLevel.Debug + case "trace" => LogLevel.Trace + case level => throw new Exception(s"Unknown LogLevel '$level'") + } +} + +/** + * extend this trait to enable logging in a class you are implementing + */ +trait LazyLogging { + protected val logger = new Logger(this.getClass.getName) + def getLogger: Logger = logger +} + +/** + * Mutable state of the logging system. Multiple LoggerStates may be present + * when used in multi-threaded environments + */ +private class LoggerState { + var globalLevel = LogLevel.None + val classLevels = new scala.collection.mutable.HashMap[String, LogLevel.Value] + val classToLevelCache = new scala.collection.mutable.HashMap[String, LogLevel.Value] + var logClassNames = false + var stream: PrintStream = Console.out + var fromInvoke: Boolean = false // this is used to not have invokes re-create run-state + var stringBufferOption: Option[Logger.OutputCaptor] = None + + override def toString: String = { + s"gl $globalLevel classLevels ${classLevels.mkString("\n")}" + } + + /** + * create a new state object copying the basic values of this state + * @return new state object + */ + def copy: LoggerState = { + val newState = new LoggerState + newState.globalLevel = this.globalLevel + newState.classLevels ++= this.classLevels + newState.stream = this.stream + newState.logClassNames = this.logClassNames + newState + } +} + +/** + * Singleton in control of what is supposed to get logged, how it's to be logged and where it is to be logged + * We uses a dynamic variable in case multiple threads are used as can be in scalatests + */ +object Logger { + private val updatableLoggerState = new DynamicVariable[Option[LoggerState]](Some(new LoggerState)) + private def state: LoggerState = { + updatableLoggerState.value.get + } + + /** + * a class for managing capturing logging output in a string buffer + */ + class OutputCaptor { + val byteArrayOutputStream = new ByteArrayOutputStream() + val printStream = new PrintStream(byteArrayOutputStream) + + /** + * Get logged messages to this captor as a string + * @return + */ + def getOutputAsString: String = { + byteArrayOutputStream.toString + } + + /** + * Clear the string buffer + */ + def clear(): Unit = { + byteArrayOutputStream.reset() + } + } + + /** Set a scope for this logger based on available annotations + * @param options a sequence annotations + * @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: 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) { + newRunState + } else { + val forcedNewRunState = new LoggerState + forcedNewRunState.fromInvoke = true + forcedNewRunState + } + } + updatableLoggerState.withValue(Some(runState)) { + doSetOptions() + codeBlock + } + } + + /** + * Used to test whether a given log statement should generate some logging output. + * It breaks up a class name into a list of packages. From this list generate progressively + * broader names (lopping off from right side) checking for a match + * @param className class name that the logging statement came from + * @param level the level of the log statement being evaluated + * @return + */ + private def testPackageNameMatch(className: String, level: LogLevel.Value): Option[Boolean] = { + val classLevels = state.classLevels + if (classLevels.isEmpty) return None + + // If this class name in cache just use that value + val levelForThisClassName = state.classToLevelCache.getOrElse( + className, { + // otherwise break up the class name in to full package path as list and find most specific entry you can + val packageNameList = className.split("""\.""").toList + /* + * start with full class path, lopping off from the tail until nothing left + */ + def matchPathToFindLevel(packageList: List[String]): LogLevel.Value = { + if (packageList.isEmpty) { + LogLevel.None + } else { + val partialName = packageList.mkString(".") + val level = classLevels.getOrElse( + partialName, { + matchPathToFindLevel(packageList.reverse.tail.reverse) + } + ) + level + } + } + + val levelSpecified = matchPathToFindLevel(packageNameList) + if (levelSpecified != LogLevel.None) { + state.classToLevelCache(className) = levelSpecified + } + levelSpecified + } + ) + + if (levelForThisClassName != LogLevel.None) { + Some(levelForThisClassName >= level) + } else { + None + } + } + + /** + * Used as the common log routine, for warn, debug etc. Only calls message if log should be generated + * Allows lazy evaluation of any string interpolation or function that generates the message itself + * @note package level supercedes global, which allows one to turn on debug everywhere except for specific classes + * @param level level of the called statement + * @param className class name of statement + * @param message a function returning a string with the message + */ + private def showMessage(level: LogLevel.Value, className: String, message: => String): Unit = { + def logIt(): Unit = { + if (state.logClassNames) { + state.stream.println(s"[$level:$className] $message") + } else { + state.stream.println(message) + } + } + testPackageNameMatch(className, level) match { + case Some(true) => logIt() + case Some(false) => + case None => + if (getGlobalLevel >= level) { + logIt() + } + } + } + + def getGlobalLevel: LogLevel.Value = state.globalLevel match { + // None means "not set" so use default in that case + case LogLevel.None => LogLevel.default + case other => other + } + + /** + * This resets everything in the current Logger environment, including the destination + * use this with caution. Unexpected things can happen + */ + def reset(): Unit = { + state.classLevels.clear() + clearCache() + state.logClassNames = false + state.globalLevel = LogLevel.Error + state.stream = System.out + } + + /** + * clears the cache of class names top class specific log levels + */ + private def clearCache(): Unit = { + state.classToLevelCache.clear() + } + + /** + * This sets the global logging level + * @param level The desired global logging level + */ + def setLevel(level: LogLevel.Value): Unit = { + state.globalLevel = level + } + + /** + * This sets the logging level for a particular class or package + * The package name must be general to specific. I.e. + * package1.package2.class + * package1.package2 + * package1 + * Will work. + * package2.class will not work if package2 is within package1 + * @param classOrPackageName The string based class name or + * @param level The desired global logging level + */ + def setLevel(classOrPackageName: String, level: LogLevel.Value): Unit = { + clearCache() + state.classLevels(classOrPackageName) = level + } + + /** + * Set the log level based on a class type + * @example {{{ setLevel(classOf[SomeClass], LogLevel.Debug) }}} + * @param classType Kind of class + * @param level log level to set + */ + def setLevel(classType: Class[_ <: LazyLogging], level: LogLevel.Value): Unit = { + clearCache() + val name = classType.getCanonicalName + state.classLevels(name) = level + } + + /** + * Clears the logging data in the string capture buffer if it exists + * @return The logging data if it exists + */ + def clearStringBuffer(): Unit = { + state.stringBufferOption match { + case Some(x) => x.byteArrayOutputStream.reset() + case None => + } + } + + /** + * Set the logging destination to a file name + * @param fileName destination name + */ + def setOutput(fileName: String): Unit = { + state.stream = new PrintStream(new FileOutputStream(new File(fileName))) + } + + /** + * Set the logging destination to a print stream + * @param stream destination stream + */ + def setOutput(stream: PrintStream): Unit = { + state.stream = stream + } + + /** + * Sets the logging destination to Console.out + */ + def setConsole(): Unit = { + state.stream = Console.out + } + + /** + * Adds a list of of className, loglevel tuples to the global (dynamicVar) + * See testPackageNameMatch for a description of how class name matching works + * @param namesToLevel a list of tuples (class name, log level) + */ + def setClassLogLevels(namesToLevel: Map[String, LogLevel.Value]): Unit = { + clearCache() + state.classLevels ++= namesToLevel + } + + /** Set logger options based on the content of an [[firrtl.AnnotationSeq AnnotationSeq]] + * @param inputAnnotations annotation sequence containing logger options + */ + def setOptions(inputAnnotations: AnnotationSeq): Unit = { + val annotations = + Seq(new AddDefaults, Checks) + .foldLeft(inputAnnotations)((a, p) => p.transform(a)) + + 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(options.classLogLevels) + + if (options.logFileName.nonEmpty) { + setOutput(options.logFileName.get) + } + + state.logClassNames = options.logClassNames + } +} + +/** + * Classes implementing [[LazyLogging]] will have logger of this type + * @param containerClass passed in from the LazyLogging trait in order to provide class level logging granularity + */ +class Logger(containerClass: String) { + + /** + * Log message at Error level + * @param message message generator to be invoked if level is right + */ + def error(message: => String): Unit = { + Logger.showMessage(LogLevel.Error, containerClass, message) + } + + /** + * Log message at Warn level + * @param message message generator to be invoked if level is right + */ + def warn(message: => String): Unit = { + Logger.showMessage(LogLevel.Warn, containerClass, message) + } + + /** + * Log message at Inof level + * @param message message generator to be invoked if level is right + */ + def info(message: => String): Unit = { + Logger.showMessage(LogLevel.Info, containerClass, message) + } + + /** + * Log message at Debug level + * @param message message generator to be invoked if level is right + */ + def debug(message: => String): Unit = { + Logger.showMessage(LogLevel.Debug, containerClass, message) + } + + /** + * Log message at Trace level + * @param message message generator to be invoked if level is right + */ + def trace(message: => String): Unit = { + Logger.showMessage(LogLevel.Trace, containerClass, message) + } +} + +/** An exception originating from the Logger + * @param str an exception message + * @param cause a reason for the exception + */ +class LoggerException(val str: String, cause: Throwable = null) extends RuntimeException(str, cause) diff --git a/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala b/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala index 310f876344a..fc179eef364 100644 --- a/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala +++ b/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala @@ -13,6 +13,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 * @@ -58,12 +59,22 @@ 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, chiselOptions.throwOnFirstError, +<<<<<<< HEAD chiselOptions.warningsAsErrors, chiselOptions.sourceRoots +======= + chiselOptions.useLegacyShiftRightWidth, + chiselOptions.warningFilters, + chiselOptions.sourceRoots, + None, + Nil, // FIXME this maybe should somehow grab definitions from earlier elaboration + loggerOptions +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) ) // 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 7d72e7e2439..58635686803 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,13 +32,23 @@ 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( annotations, chiselOptions.throwOnFirstError, +<<<<<<< HEAD chiselOptions.warningsAsErrors, chiselOptions.sourceRoots +======= + chiselOptions.useLegacyShiftRightWidth, + chiselOptions.warningFilters, + chiselOptions.sourceRoots, + None, + Nil, + loggerOptions +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) ) 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 29d68652582..0bfb1b78af1 100644 --- a/src/main/scala/circt/stage/ChiselStage.scala +++ b/src/main/scala/circt/stage/ChiselStage.scala @@ -6,8 +6,14 @@ import chisel3.RawModule import chisel3.stage.{ChiselCircuitAnnotation, ChiselGeneratorAnnotation, CircuitSerializationAnnotation} import chisel3.stage.CircuitSerializationAnnotation.FirrtlFileFormat import firrtl.{AnnotationSeq, EmittedVerilogCircuitAnnotation} +<<<<<<< HEAD import firrtl.options.{Dependency, Phase, PhaseManager, Shell, Stage, StageMain} import firrtl.stage.FirrtlCircuitAnnotation +======= +import firrtl.options.{CustomFileEmission, Dependency, Phase, PhaseManager, Stage, StageMain, Unserializable} +import firrtl.stage.FirrtlCircuitAnnotation +import logger.LogLevelAnnotation +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) /** Entry point for running Chisel with the CIRCT compiler. * @@ -22,21 +28,27 @@ 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], Dependency[chisel3.stage.phases.AddSerializationAnnotations], Dependency[chisel3.stage.phases.Convert], Dependency[chisel3.stage.phases.MaybeInjectingPhase], +<<<<<<< HEAD Dependency[firrtl.stage.phases.AddImplicitOutputFile], Dependency[circt.stage.phases.Checks], +======= + Dependency[circt.stage.phases.AddImplicitOutputFile], +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) Dependency[circt.stage.phases.CIRCT] ), currentState = Seq( @@ -55,7 +67,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], @@ -74,7 +85,11 @@ object ChiselStage { val annos = Seq( ChiselGeneratorAnnotation(() => gen), CIRCTTargetAnnotation(CIRCTTarget.CHIRRTL) +<<<<<<< HEAD ) ++ (new ChiselStage).shell.parse(args) +======= + ) ++ (new Shell("circt")).parse(args) +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) phase .transform(annos) @@ -97,7 +112,11 @@ object ChiselStage { val annos = Seq( ChiselGeneratorAnnotation(() => gen), CIRCTTargetAnnotation(CIRCTTarget.CHIRRTL) +<<<<<<< HEAD ) ++ (new ChiselStage).shell.parse(args) +======= + ) ++ (new Shell("circt")).parse(args) +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) phase .transform(annos) @@ -116,7 +135,11 @@ object ChiselStage { val annos = Seq( ChiselGeneratorAnnotation(() => gen), CIRCTTargetAnnotation(CIRCTTarget.FIRRTL) +<<<<<<< HEAD ) ++ (new ChiselStage).shell.parse(args) ++ firtoolOpts.map(FirtoolOption(_)) +======= + ) ++ (new Shell("circt")).parse(args) ++ firtoolOpts.map(FirtoolOption(_)) +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) phase .transform(annos) @@ -135,7 +158,11 @@ object ChiselStage { val annos = Seq( ChiselGeneratorAnnotation(() => gen), CIRCTTargetAnnotation(CIRCTTarget.HW) +<<<<<<< HEAD ) ++ (new ChiselStage).shell.parse(args) ++ firtoolOpts.map(FirtoolOption(_)) +======= + ) ++ (new Shell("circt")).parse(args) ++ firtoolOpts.map(FirtoolOption(_)) +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) phase .transform(annos) @@ -160,7 +187,11 @@ object ChiselStage { val annos = Seq( ChiselGeneratorAnnotation(() => gen), CIRCTTargetAnnotation(CIRCTTarget.SystemVerilog) +<<<<<<< HEAD ) ++ (new circt.stage.ChiselStage).shell.parse(args) ++ firtoolOpts.map(FirtoolOption(_)) +======= + ) ++ (new Shell("circt")).parse(args) ++ firtoolOpts.map(FirtoolOption(_)) +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) 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..f2295b86f7c --- /dev/null +++ b/src/main/scala/circt/stage/Shell.scala @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: Apache-2.0 + +package circt.stage + +import chisel3.stage.{ + ChiselCircuitAnnotation, + ChiselGeneratorAnnotation, + CircuitSerializationAnnotation, + PrintFullStackTraceAnnotation, + SourceRootAnnotation, + ThrowOnFirstErrorAnnotation, + UseLegacyShiftRightWidthBehavior, + 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, + UseLegacyShiftRightWidthBehavior, + 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 f8b48836b3e..38a6662aae2 100644 --- a/src/test/scala/circtTests/stage/ChiselStageSpec.scala +++ b/src/test/scala/circtTests/stage/ChiselStageSpec.scala @@ -282,6 +282,85 @@ class ChiselStageSpec extends AnyFunSpec with Matchers with chiselTests.Utils { file should exist } } +<<<<<<< HEAD +======= + + it("should emit Annotations inline in emitted CHIRRTL") { + val targetDir = os.pwd / "ChiselStageSpec" / "should-inline-Annotations-in-emitted-CHIRRTL" + + val args: Array[String] = Array( + "--target", + "chirrtl", + "--target-dir", + targetDir.toString + ) + + (new ChiselStage) + .execute( + args, + Seq(ChiselGeneratorAnnotation(() => new ChiselStageSpec.Foo(hasDontTouch = true))) + ) + + info("output file included an Annotation") + os.read(targetDir / "Foo.fir") should include("firrtl.transforms.DontTouchAnnotation") + } + + it("should NOT emit Unserializable Annotations inline in emitted CHIRRTL") { + val targetDir = baseDir / "should-not-inline-Unserializable-Annotations-in-emitted-CHIRRTL" + + val args: Array[String] = Array( + "--target", + "chirrtl", + "--target-dir", + targetDir.toString + ) + + (new ChiselStage) + .execute( + args, + Seq(ChiselGeneratorAnnotation(() => new ChiselStageSpec.HasUnserializableAnnotation)) + ) + + os.read(targetDir / "HasUnserializableAnnotation.fir") shouldNot include("DummyAnnotation") + } + + 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 { + (new ChiselStage).execute(args, annos) + } + log1 shouldNot include(msg) + + val (log2, _) = grabLog { + (new ChiselStage).execute(args ++ Array("--log-level", "debug"), annos) + } + 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.") + } +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) } describe("ChiselStage exception handling") { @@ -965,6 +1044,47 @@ class ChiselStageSpec extends AnyFunSpec with Matchers with chiselTests.Utils { info(s"'$expectedOutput' exists") } +<<<<<<< HEAD +======= + it("""should error if give a "--target-directory" option""") { + + val exception = intercept[firrtl.options.OptionsException] { + ChiselStage.emitCHIRRTL(new ChiselStageSpec.Foo, Array("--target-directory")) should include("circuit Foo") + } + + val message = exception.getMessage + info("""The exception includes "Unknown option"""") + message should include("Unknown option --target-directory") + + } + + 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.") + } +>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895)) } describe("ChiselStage$ exception handling") {