diff options
| -rw-r--r-- | core/src/main/scala/chisel3/StrongEnum.scala | 34 | ||||
| -rw-r--r-- | core/src/main/scala/chisel3/internal/Builder.scala | 2 | ||||
| -rw-r--r-- | core/src/main/scala/chisel3/internal/Error.scala | 25 | ||||
| -rw-r--r-- | docs/src/explanations/chisel-enum.md | 170 | ||||
| -rw-r--r-- | src/test/scala/chiselTests/ChiselSpec.scala | 17 | ||||
| -rw-r--r-- | src/test/scala/chiselTests/StrongEnum.scala | 97 |
6 files changed, 275 insertions, 70 deletions
diff --git a/core/src/main/scala/chisel3/StrongEnum.scala b/core/src/main/scala/chisel3/StrongEnum.scala index 7d328eb7..1d0e04d3 100644 --- a/core/src/main/scala/chisel3/StrongEnum.scala +++ b/core/src/main/scala/chisel3/StrongEnum.scala @@ -131,7 +131,7 @@ abstract class EnumType(private val factory: EnumFactory, selfAnnotating: Boolea if (litOption.isDefined) { true.B } else { - factory.all.map(this === _).reduce(_ || _) + if (factory.isTotal) true.B else factory.all.map(this === _).reduce(_ || _) } } @@ -233,6 +233,12 @@ abstract class EnumFactory { private[chisel3] val enumTypeName = getClass.getName.init + // Do all bitvectors of this Enum's width represent legal states? + private[chisel3] def isTotal: Boolean = { + (this.getWidth < 31) && // guard against Integer overflow + (enumRecords.size == (1 << this.getWidth)) + } + private[chisel3] def globalAnnotation: EnumDefChiselAnnotation = EnumDefChiselAnnotation(enumTypeName, (enumNames, enumValues).zipped.toMap) @@ -277,7 +283,7 @@ abstract class EnumFactory { def apply(): Type = new Type - def apply(n: UInt)(implicit sourceInfo: SourceInfo, connectionCompileOptions: CompileOptions): Type = { + private def castImpl(n: UInt, warn: Boolean)(implicit sourceInfo: SourceInfo, connectionCompileOptions: CompileOptions): Type = { if (n.litOption.isDefined) { enumInstances.find(_.litValue == n.litValue) match { case Some(result) => result @@ -288,8 +294,9 @@ abstract class EnumFactory { } else if (n.getWidth > this.getWidth) { throwException(s"The UInt being cast to $enumTypeName is wider than $enumTypeName's width ($getWidth)") } else { - Builder.warning(s"Casting non-literal UInt to $enumTypeName. You can check that its value is legal by calling isValid") - + if (warn && !this.isTotal) { + Builder.warning(s"Casting non-literal UInt to $enumTypeName. You can use $enumTypeName.safe to cast without this warning.") + } val glue = Wire(new UnsafeEnum(width)) glue := n val result = Wire(new Type) @@ -297,6 +304,25 @@ abstract class EnumFactory { result } } + + /** Cast an [[UInt]] to the type of this Enum + * + * @note will give a Chisel elaboration time warning if the argument could hit invalid states + * @param n the UInt to cast + * @return the equivalent Enum to the value of the cast UInt + */ + def apply(n: UInt)(implicit sourceInfo: SourceInfo, connectionCompileOptions: CompileOptions): Type = castImpl(n, warn = true) + + /** Safely cast an [[UInt]] to the type of this Enum + * + * @param n the UInt to cast + * @return the equivalent Enum to the value of the cast UInt and a Bool indicating if the + * Enum is valid + */ + def safe(n: UInt)(implicit sourceInfo: SourceInfo, connectionCompileOptions: CompileOptions): (Type, Bool) = { + val t = castImpl(n, warn = false) + (t, t.isValid) + } } diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 35c4bdf9..fef12093 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -652,7 +652,7 @@ private[chisel3] object Builder extends LazyLogging { logger.warn("Elaborating design...") val mod = f mod.forceName(None, mod.name, globalNamespace) - errors.checkpoint() + errors.checkpoint(logger) logger.warn("Done elaborating.") (Circuit(components.last.name, components.toSeq, annotations.toSeq), mod) diff --git a/core/src/main/scala/chisel3/internal/Error.scala b/core/src/main/scala/chisel3/internal/Error.scala index 454be360..d6e0c0e6 100644 --- a/core/src/main/scala/chisel3/internal/Error.scala +++ b/core/src/main/scala/chisel3/internal/Error.scala @@ -4,6 +4,7 @@ package chisel3.internal import scala.annotation.tailrec import scala.collection.mutable.{ArrayBuffer, LinkedHashMap} +import _root_.logger.Logger object ExceptionHelpers { @@ -184,31 +185,31 @@ private[chisel3] class ErrorLog { } /** Throw an exception if any errors have yet occurred. */ - def checkpoint(): Unit = { + def checkpoint(logger: Logger): Unit = { deprecations.foreach { case ((message, sourceLoc), count) => - println(s"${ErrorLog.depTag} $sourceLoc ($count calls): $message") + logger.warn(s"${ErrorLog.depTag} $sourceLoc ($count calls): $message") } - errors foreach println + errors.foreach(e => logger.error(e.toString)) if (!deprecations.isEmpty) { - println(s"${ErrorLog.warnTag} ${Console.YELLOW}There were ${deprecations.size} deprecated function(s) used." + + logger.warn(s"${ErrorLog.warnTag} ${Console.YELLOW}There were ${deprecations.size} deprecated function(s) used." + s" These may stop compiling in a future release - you are encouraged to fix these issues.${Console.RESET}") - println(s"${ErrorLog.warnTag} Line numbers for deprecations reported by Chisel may be inaccurate; enable scalac compiler deprecation warnings via either of the following methods:") - println(s"${ErrorLog.warnTag} In the sbt interactive console, enter:") - println(s"""${ErrorLog.warnTag} set scalacOptions in ThisBuild ++= Seq("-unchecked", "-deprecation")""") - println(s"${ErrorLog.warnTag} or, in your build.sbt, add the line:") - println(s"""${ErrorLog.warnTag} scalacOptions := Seq("-unchecked", "-deprecation")""") + logger.warn(s"${ErrorLog.warnTag} Line numbers for deprecations reported by Chisel may be inaccurate; enable scalac compiler deprecation warnings via either of the following methods:") + logger.warn(s"${ErrorLog.warnTag} In the sbt interactive console, enter:") + logger.warn(s"""${ErrorLog.warnTag} set scalacOptions in ThisBuild ++= Seq("-unchecked", "-deprecation")""") + logger.warn(s"${ErrorLog.warnTag} or, in your build.sbt, add the line:") + logger.warn(s"""${ErrorLog.warnTag} scalacOptions := Seq("-unchecked", "-deprecation")""") } val allErrors = errors.filter(_.isFatal) val allWarnings = errors.filter(!_.isFatal) if (!allWarnings.isEmpty && !allErrors.isEmpty) { - println(s"${ErrorLog.errTag} There were ${Console.RED}${allErrors.size} error(s)${Console.RESET} and ${Console.YELLOW}${allWarnings.size} warning(s)${Console.RESET} during hardware elaboration.") + logger.warn(s"${ErrorLog.errTag} There were ${Console.RED}${allErrors.size} error(s)${Console.RESET} and ${Console.YELLOW}${allWarnings.size} warning(s)${Console.RESET} during hardware elaboration.") } else if (!allWarnings.isEmpty) { - println(s"${ErrorLog.warnTag} There were ${Console.YELLOW}${allWarnings.size} warning(s)${Console.RESET} during hardware elaboration.") + logger.warn(s"${ErrorLog.warnTag} There were ${Console.YELLOW}${allWarnings.size} warning(s)${Console.RESET} during hardware elaboration.") } else if (!allErrors.isEmpty) { - println(s"${ErrorLog.errTag} There were ${Console.RED}${allErrors.size} error(s)${Console.RESET} during hardware elaboration.") + logger.warn(s"${ErrorLog.errTag} There were ${Console.RED}${allErrors.size} error(s)${Console.RESET} during hardware elaboration.") } if (!allErrors.isEmpty) { diff --git a/docs/src/explanations/chisel-enum.md b/docs/src/explanations/chisel-enum.md index cb017239..3f0f3b18 100644 --- a/docs/src/explanations/chisel-enum.md +++ b/docs/src/explanations/chisel-enum.md @@ -1,4 +1,4 @@ ---- +--- layout: docs title: "Enumerations" section: "chisel3" @@ -6,8 +6,8 @@ section: "chisel3" # ChiselEnum -The ChiselEnum type can be used to reduce the chance of error when encoding mux selectors, opcodes, and functional unit operations. In contrast with`Chisel.util.Enum`, `ChiselEnum` are subclasses of `Data`, which means that they can be used to define fields in `Bundle`s, including in `IO`s. - +The ChiselEnum type can be used to reduce the chance of error when encoding mux selectors, opcodes, and functional unit operations. +In contrast with `Chisel.util.Enum`, `ChiselEnum` are subclasses of `Data`, which means that they can be used to define fields in `Bundle`s, including in `IO`s. ## Functionality and Examples @@ -19,57 +19,68 @@ import chisel3.stage.ChiselStage import chisel3.experimental.ChiselEnum ``` -Below we see ChiselEnum being used as mux select for a RISC-V core. While wrapping the object in a package is not required, it is highly recommended as it allows for the type to be used in multiple files more easily. +```scala mdoc:invisible +// Helper to print stdout from Chisel elab +// May be related to: https://github.com/scalameta/mdoc/issues/517 +import java.io._ +import _root_.logger.Logger +def grabLog[T](thunk: => T): (String, T) = { + val baos = new ByteArrayOutputStream() + val stream = new PrintStream(baos, true, "utf-8") + val ret = Logger.makeScope(Nil) { + Logger.setOutput(stream) + thunk + } + (baos.toString, ret) +} +``` + +Below we see ChiselEnum being used as mux select for a RISC-V core. While wrapping the object in a package is not required, it is highly recommended as it allows for the type to be used in multiple files more easily. ```scala mdoc // package CPUTypes { - object AluMux1Sel extends ChiselEnum { - val selectRS1, selectPC = Value - /** How the values will be mapped - "selectRS1" -> 0.U, - "selectPC" -> 1.U - */ - } -// } +object AluMux1Sel extends ChiselEnum { + val selectRS1, selectPC = Value +} +// We can see the mapping by printing each Value +AluMux1Sel.all.foreach(println) ``` -Here we see a mux using the AluMux1Sel to select between different inputs. +Here we see a mux using the AluMux1Sel to select between different inputs. ```scala mdoc import AluMux1Sel._ class AluMux1Bundle extends Bundle { - val aluMux1Sel = Input( AluMux1Sel() ) - val rs1Out = Input(Bits(32.W)) - val pcOut = Input(Bits(32.W)) - val aluMux1Out = Output(Bits(32.W)) + val aluMux1Sel = Input(AluMux1Sel()) + val rs1Out = Input(Bits(32.W)) + val pcOut = Input(Bits(32.W)) + val aluMux1Out = Output(Bits(32.W)) } class AluMux1File extends Module { - val io = IO(new AluMux1Bundle) - - // Default value for aluMux1Out - io.aluMux1Out := 0.U - - switch (io.aluMux1Sel) { - is (selectRS1) { - io.aluMux1Out := io.rs1Out - } - is (selectPC) { - io.aluMux1Out := io.pcOut - } + val io = IO(new AluMux1Bundle) + + // Default value for aluMux1Out + io.aluMux1Out := 0.U + + switch (io.aluMux1Sel) { + is (selectRS1) { + io.aluMux1Out := io.rs1Out } + is (selectPC) { + io.aluMux1Out := io.pcOut + } + } } ``` ```scala mdoc:verilog -ChiselStage.emitVerilog(new AluMux1File ) +ChiselStage.emitVerilog(new AluMux1File) ``` -ChiselEnum also allows for the user to define variables by passing in the value shown below. Note that the value must be increasing or else - - > chisel3.internal.ChiselException: Exception thrown when elaborating ChiselGeneratorAnnotation - -is thrown during Verilog generation. +ChiselEnum also allows for the user to directly set the Values by passing an `UInt` to `Value(...)` +as shown below. Note that the magnitude of each `Value` must be strictly greater than the one before +it. ```scala mdoc object Opcode extends ChiselEnum { @@ -85,27 +96,81 @@ object Opcode extends ChiselEnum { } ``` -The user can 'jump' to a value and continue incrementing by passing a start point then using a regular Value assignment. +The user can 'jump' to a value and continue incrementing by passing a start point then using a regular Value definition. ```scala mdoc object BranchFunct3 extends ChiselEnum { val beq, bne = Value val blt = Value(4.U) val bge, bltu, bgeu = Value - /** How the values will be mapped - "beq" -> 0.U, - "bne" -> 1.U, - "blt" -> 4.U, - "bge" -> 5.U, - "bltu" -> 6.U, - "bgeu" -> 7.U - */ } +// We can see the mapping by printing each Value +BranchFunct3.all.foreach(println) +``` + +## Casting + +You can cast an enum to a `UInt` using `.asUInt`: + +```scala mdoc +class ToUInt extends RawModule { + val in = IO(Input(Opcode())) + val out = IO(Output(UInt())) + out := in.asUInt +} +``` + +```scala mdoc:invisible +// Always need to run Chisel to see if there are elaboration errors +ChiselStage.emitVerilog(new ToUInt) +``` + +You can cast from a `UInt` to an enum by passing the `UInt` to the apply method of the `ChiselEnum` object: + +```scala mdoc +class FromUInt extends Module { + val in = IO(Input(UInt(7.W))) + val out = IO(Output(Opcode())) + out := Opcode(in) +} +``` + +However, if you cast from a `UInt` to an Enum type when there are undefined states in the Enum values +that the `UInt` could hit, you will see a warning like the following: + +```scala mdoc:passthrough +val (log, _) = grabLog(ChiselStage.emitChirrtl(new FromUInt)) +println(s"```$log```") +``` +(Note that the name of the Enum is ugly as an artifact of our documentation generation flow, it will +be cleaner in normal use). + +You can avoid this warning by using the `.safe` factory method which returns the cast Enum in addition +to a `Bool` indicating if the Enum is in a valid state: + +```scala mdoc +class SafeFromUInt extends Module { + val in = IO(Input(UInt(7.W))) + val out = IO(Output(Opcode())) + val (value, valid) = Opcode.safe(in) + assert(valid, "Enum state must be valid, got %d!", in) + out := value +} +``` + +Now there will be no warning: + +```scala mdoc:passthrough +val (log2, _) = grabLog(ChiselStage.emitChirrtl(new SafeFromUInt)) +println(s"```$log2```") ``` ## Testing -When testing your modules, the `.Type` and `.litValue` attributes allow for the the objects to be passed as parameters and for the value to be converted to BigInt type. Note that BigInts cannot be casted to Int with `.asInstanceOf[Int]`, they use their own methods like `toInt`. Please review the [scala.math.BigInt](https://www.scala-lang.org/api/2.12.5/scala/math/BigInt.html) page for more details! +The _Type_ of the enums values is `<ChiselEnum Object>.Type` which can be useful for passing the values +as parameters to a function (or any other time a type annotation is needed). +Calling `.litValue` on an enum value will return the integer value of that object as a +[`BigInt`](https://www.scala-lang.org/api/2.12.13/scala/math/BigInt.html). ```scala mdoc def expectedSel(sel: AluMux1Sel.Type): Boolean = sel match { @@ -115,22 +180,23 @@ def expectedSel(sel: AluMux1Sel.Type): Boolean = sel match { } ``` -The ChiselEnum type also has methods `.all` and `.getWidth` where `all` returns all of the enum instances and `getWidth` returns the width of the hardware type. +Some additional useful methods defined on the `ChiselEnum` object are: +* `.all`: returns the enum values within the enumeration +* `.getWidth`: returns the width of the hardware type ## Workarounds -As of 2/26/2021, the width of the values is always inferred. To work around this, you can add an extra `Value` that forces the width that is desired. This is shown in the example below, where we add a field `ukn` to force the width to be 3 bits wide: +As of Chisel v3.4.3 (1 July 2020), the width of the values is always inferred. +To work around this, you can add an extra `Value` that forces the width that is desired. +This is shown in the example below, where we add a field `ukn` to force the width to be 3 bits wide: ```scala mdoc object StoreFunct3 extends ChiselEnum { val sb, sh, sw = Value val ukn = Value(7.U) - /** How the values will be mapped - "sb" -> 0.U, - "sh" -> 1.U, - "sw" -> 2.U - */ } +// We can see the mapping by printing each Value +StoreFunct3.all.foreach(println) ``` Signed values are not supported so if you want the value signed, you must cast the UInt with `.asSInt`. diff --git a/src/test/scala/chiselTests/ChiselSpec.scala b/src/test/scala/chiselTests/ChiselSpec.scala index a4192c5e..e513189e 100644 --- a/src/test/scala/chiselTests/ChiselSpec.scala +++ b/src/test/scala/chiselTests/ChiselSpec.scala @@ -9,6 +9,7 @@ import chisel3.testers._ import firrtl.annotations.Annotation import firrtl.util.BackendCompilationUtilities import firrtl.{AnnotationSeq, EmittedVerilogCircuitAnnotation} +import _root_.logger.Logger import org.scalacheck._ import org.scalatest._ import org.scalatest.flatspec.AnyFlatSpec @@ -17,7 +18,7 @@ import org.scalatest.propspec.AnyPropSpec import org.scalatest.matchers.should.Matchers import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks -import java.io.ByteArrayOutputStream +import java.io.{ByteArrayOutputStream, PrintStream} import java.security.Permission import scala.reflect.ClassTag @@ -172,6 +173,20 @@ trait Utils { (stdout.toString, stderr.toString, ret) } + /** Run some Scala thunk and return all logged messages as Strings + * @param thunk some Scala code + * @return a tuple containing LOGGED, and what the thunk returns + */ + def grabLog[T](thunk: => T): (String, T) = { + val baos = new ByteArrayOutputStream() + val stream = new PrintStream(baos, true, "utf-8") + val ret = Logger.makeScope(Nil) { + Logger.setOutput(stream) + thunk + } + (baos.toString, ret) + } + /** Encodes a System.exit exit code * @param status the exit code */ diff --git a/src/test/scala/chiselTests/StrongEnum.scala b/src/test/scala/chiselTests/StrongEnum.scala index bf0eb2fe..e59a5398 100644 --- a/src/test/scala/chiselTests/StrongEnum.scala +++ b/src/test/scala/chiselTests/StrongEnum.scala @@ -74,6 +74,18 @@ class CastFromNonLit extends Module { io.valid := io.out.isValid } +class SafeCastFromNonLit extends Module { + val io = IO(new Bundle { + val in = Input(UInt(EnumExample.getWidth.W)) + val out = Output(EnumExample()) + val valid = Output(Bool()) + }) + + val (enum, valid) = EnumExample.safe(io.in) + io.out := enum + io.valid := valid +} + class CastFromNonLitWidth(w: Option[Int] = None) extends Module { val width = if (w.isDefined) w.get.W else UnknownWidth() @@ -191,6 +203,28 @@ class CastFromNonLitTester extends BasicTester { stop() } +class SafeCastFromNonLitTester extends BasicTester { + for ((enum,lit) <- EnumExample.all zip EnumExample.litValues) { + val mod = Module(new SafeCastFromNonLit) + mod.io.in := lit + assert(mod.io.out === enum) + assert(mod.io.valid === true.B) + } + + val invalid_values = (1 until (1 << EnumExample.getWidth)). + filter(!EnumExample.litValues.map(_.litValue).contains(_)). + map(_.U) + + for (invalid_val <- invalid_values) { + val mod = Module(new SafeCastFromNonLit) + mod.io.in := invalid_val + + assert(mod.io.valid === false.B) + } + + stop() +} + class CastToInvalidEnumTester extends BasicTester { val invalid_value: UInt = EnumExample.litValues.last + 1.U Module(new CastFromLit(invalid_value)) @@ -320,6 +354,10 @@ class StrongEnumSpec extends ChiselFlatSpec with Utils { assertTesterPasses(new CastFromNonLitTester) } + it should "safely cast non-literal UInts to enums correctly and detect illegal casts" in { + assertTesterPasses(new SafeCastFromNonLitTester) + } + it should "prevent illegal literal casts to enums" in { a [ChiselException] should be thrownBy extractCause[ChiselException] { ChiselStage.elaborate(new CastToInvalidEnumTester) @@ -377,6 +415,65 @@ class StrongEnumSpec extends ChiselFlatSpec with Utils { "StrongEnum FSM" should "work" in { assertTesterPasses(new StrongEnumFSMTester) } + + "Casting a UInt to an Enum" should "warn if the UInt can express illegal states" in { + object MyEnum extends ChiselEnum { + val e0, e1, e2 = Value + } + + class MyModule extends Module { + val in = IO(Input(UInt(2.W))) + val out = IO(Output(MyEnum())) + out := MyEnum(in) + } + val (log, _) = grabLog(ChiselStage.elaborate(new MyModule)) + log should include ("warn") + log should include ("Casting non-literal UInt") + } + + it should "NOT warn if the Enum is total" in { + object TotalEnum extends ChiselEnum { + val e0, e1, e2, e3 = Value + } + + class MyModule extends Module { + val in = IO(Input(UInt(2.W))) + val out = IO(Output(TotalEnum())) + out := TotalEnum(in) + } + val (log, _) = grabLog(ChiselStage.elaborate(new MyModule)) + log should not include ("warn") + } + + "Casting a UInt to an Enum with .safe" should "NOT warn" in { + object MyEnum extends ChiselEnum { + val e0, e1, e2 = Value + } + + class MyModule extends Module { + val in = IO(Input(UInt(2.W))) + val out = IO(Output(MyEnum())) + out := MyEnum.safe(in)._1 + } + val (log, _) = grabLog(ChiselStage.elaborate(new MyModule)) + log should not include ("warn") + } + + it should "NOT generate any validity logic if the Enum is total" in { + object TotalEnum extends ChiselEnum { + val e0, e1, e2, e3 = Value + } + + class MyModule extends Module { + val in = IO(Input(UInt(2.W))) + val out = IO(Output(TotalEnum())) + val (res, valid) = TotalEnum.safe(in) + assert(valid.litToBoolean, "It should be true.B") + out := res + } + val (log, _) = grabLog(ChiselStage.elaborate(new MyModule)) + log should not include ("warn") + } } class StrongEnumAnnotator extends Module { |