From 7a2a62fcae758ef2c5a9ac1b7a321f454b03a69a Mon Sep 17 00:00:00 2001 From: David Biancolin Date: Mon, 9 Dec 2024 17:31:02 -0800 Subject: [PATCH 1/3] [choice] Adds a means to declare a choice group without use --- .../main/scala/chisel3/choice/package.scala | 30 ++++++++++++++++++- .../scala/chiselTests/ModuleChoiceSpec.scala | 21 ++++++++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/chisel3/choice/package.scala b/core/src/main/scala/chisel3/choice/package.scala index 72c7672c75d..c7e380ff399 100644 --- a/core/src/main/scala/chisel3/choice/package.scala +++ b/core/src/main/scala/chisel3/choice/package.scala @@ -3,8 +3,13 @@ package chisel3 import chisel3.experimental.{BaseModule, SourceInfo} +import chisel3.internal.Builder import chisel3.util.simpleClassName +import scala.collection.mutable +import scala.reflect.runtime.universe._ +import scala.reflect.runtime.{currentMirror => cm} + /** This package contains Chisel language definitions for describing configuration options and their accepted values. */ package object choice { @@ -13,7 +18,7 @@ package object choice { * * @example * {{{ - * import chisel3.option.{Group, Case} + * import chisel3.choice.{Group, Case} * object Platform extends Group { * object FPGA extends Case * object ASIC extends Case @@ -47,4 +52,27 @@ package object choice { */ def ->[T](module: => T): (Case, () => T) = (this, () => module) } + + /** Registers all options in a group with the Builder. + * This lets Chisel know that this layer should be emitted into FIRRTL text. + * + * This API can be used to guarantee that a design will always have certain + * group defined. This is analagous in spirit to [[layer.addLayer]]. + */ + def addGroup[T <: Group: TypeTag](group: T): Unit = { + + val tpe = typeOf[T] + val classSymbol = tpe.typeSymbol.asClass + val classMirror = cm.reflectClass(classSymbol) + + tpe.members.collect { + // Look only for inner objects. Note, this is not recursive. + case m: ModuleSymbol if m.isStatic => + val instance = cm.reflectModule(m.asModule).instance + // Confirms the instance is a subtype of Case + if (cm.classSymbol(instance.getClass).toType <:< typeOf[Case]) { + Builder.options += instance.asInstanceOf[Case] + } + } + } } diff --git a/src/test/scala/chiselTests/ModuleChoiceSpec.scala b/src/test/scala/chiselTests/ModuleChoiceSpec.scala index 86da88ede1a..952086d5c3f 100644 --- a/src/test/scala/chiselTests/ModuleChoiceSpec.scala +++ b/src/test/scala/chiselTests/ModuleChoiceSpec.scala @@ -3,7 +3,7 @@ package chiselTests import chisel3._ -import chisel3.choice.{Case, Group, ModuleChoice} +import chisel3.choice.{addGroup, Case, Group, ModuleChoice} import chiselTests.{ChiselFlatSpec, MatchesAndOmits, Utils} import _root_.circt.stage.ChiselStage @@ -129,3 +129,22 @@ class ModuleChoiceSpec extends ChiselFlatSpec with Utils with MatchesAndOmits { } } +class AddGroupSpec extends ChiselFlatSpec with Utils with MatchesAndOmits { + it should "emit options for a registered group even if there are no consumers" in { + class ModuleWithoutChoice extends Module { + addGroup(Platform) + val out = IO(UInt(8.W)) + val in = IO(UInt(8.W)) + out := in + } + + val chirrtl = ChiselStage.emitCHIRRTL(new ModuleWithoutChoice, Array("--full-stacktrace")) + + info("CHIRRTL emission looks correct") + matchesAndOmits(chirrtl)( + "option Platform :", + "FPGA", + "ASIC" + )() + } +} From 8dbb9b82cd46e6f5a1a71fb37f3aa33cb111c0c9 Mon Sep 17 00:00:00 2001 From: David Biancolin Date: Mon, 9 Dec 2024 20:29:25 -0800 Subject: [PATCH 2/3] Update src/test/scala/chiselTests/ModuleChoiceSpec.scala Co-authored-by: Schuyler Eldridge --- src/test/scala/chiselTests/ModuleChoiceSpec.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/scala/chiselTests/ModuleChoiceSpec.scala b/src/test/scala/chiselTests/ModuleChoiceSpec.scala index 952086d5c3f..9b2c9faef16 100644 --- a/src/test/scala/chiselTests/ModuleChoiceSpec.scala +++ b/src/test/scala/chiselTests/ModuleChoiceSpec.scala @@ -141,10 +141,11 @@ class AddGroupSpec extends ChiselFlatSpec with Utils with MatchesAndOmits { val chirrtl = ChiselStage.emitCHIRRTL(new ModuleWithoutChoice, Array("--full-stacktrace")) info("CHIRRTL emission looks correct") - matchesAndOmits(chirrtl)( - "option Platform :", - "FPGA", - "ASIC" + fileCheckString(chirrtl)( + """|CHECK: option Platform : + |CHECK-NEXT: FPGA + |CHECK-NEXT: ASIC + |""".stripMargin )() } } From a53063dd9a442eb34010b7e96d93add8710f373f Mon Sep 17 00:00:00 2001 From: David Biancolin Date: Mon, 9 Dec 2024 21:15:34 -0800 Subject: [PATCH 3/3] A slightly different implementation avoiding TypeTag --- .../main/scala/chisel3/choice/package.scala | 33 ++++++++++-------- .../scala/chiselTests/ModuleChoiceSpec.scala | 34 ++++++++----------- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/core/src/main/scala/chisel3/choice/package.scala b/core/src/main/scala/chisel3/choice/package.scala index c7e380ff399..e3a0c59dd27 100644 --- a/core/src/main/scala/chisel3/choice/package.scala +++ b/core/src/main/scala/chisel3/choice/package.scala @@ -28,6 +28,22 @@ package object choice { abstract class Group(implicit _sourceInfo: SourceInfo) { self: Singleton => + private[choice] def registerCases(): Unit = { + // Grab a symbol for the derived class (a concrete Group) + val instanceMirror = cm.reflect(this) + val symbol = instanceMirror.symbol + + symbol.typeSignature.members.collect { + // Look only for inner objects in the Group. Note, this is not recursive. + case m: ModuleSymbol if m.isStatic => + val instance = cm.reflectModule(m.asModule).instance + // Confirms the instance is a subtype of Case + if (cm.classSymbol(instance.getClass).toType <:< typeOf[Case]) { + Builder.options += instance.asInstanceOf[Case] + } + } + } + private[chisel3] def sourceInfo: SourceInfo = _sourceInfo private[chisel3] def name: String = simpleClassName(this.getClass()) @@ -59,20 +75,7 @@ package object choice { * This API can be used to guarantee that a design will always have certain * group defined. This is analagous in spirit to [[layer.addLayer]]. */ - def addGroup[T <: Group: TypeTag](group: T): Unit = { - - val tpe = typeOf[T] - val classSymbol = tpe.typeSymbol.asClass - val classMirror = cm.reflectClass(classSymbol) - - tpe.members.collect { - // Look only for inner objects. Note, this is not recursive. - case m: ModuleSymbol if m.isStatic => - val instance = cm.reflectModule(m.asModule).instance - // Confirms the instance is a subtype of Case - if (cm.classSymbol(instance.getClass).toType <:< typeOf[Case]) { - Builder.options += instance.asInstanceOf[Case] - } - } + def addGroup(group: Group): Unit = { + group.registerCases() } } diff --git a/src/test/scala/chiselTests/ModuleChoiceSpec.scala b/src/test/scala/chiselTests/ModuleChoiceSpec.scala index 9b2c9faef16..91c1b0863fb 100644 --- a/src/test/scala/chiselTests/ModuleChoiceSpec.scala +++ b/src/test/scala/chiselTests/ModuleChoiceSpec.scala @@ -23,7 +23,7 @@ class ASICTarget extends FixedIOExtModule[TargetIO](new TargetIO(8)) class VerifTarget extends FixedIORawModule[TargetIO](new TargetIO(8)) -class ModuleChoiceSpec extends ChiselFlatSpec with Utils with MatchesAndOmits { +class ModuleChoiceSpec extends ChiselFlatSpec with Utils with FileCheck { it should "emit options and cases" in { class ModuleWithChoice extends Module { val out = IO(UInt(8.W)) @@ -35,17 +35,14 @@ class ModuleChoiceSpec extends ChiselFlatSpec with Utils with MatchesAndOmits { out := inst.out } - val chirrtl = ChiselStage.emitCHIRRTL(new ModuleWithChoice, Array("--full-stacktrace")) - - info("CHIRRTL emission looks correct") - matchesAndOmits(chirrtl)( - "option Platform :", - "FPGA", - "ASIC", - "instchoice inst of VerifTarget, Platform :", - "FPGA => FPGATarget", - "ASIC => ASICTarget" - )() + generateFirrtlAndFileCheck(new ModuleWithChoice)( + """|CHECK: option Platform : + |CHECK-NEXT: FPGA + |CHECK-NEXT: ASIC + |CHECK: instchoice inst of VerifTarget, Platform : + |CHECK-NEXT: FPGA => FPGATarget + |CHECK-NEXT: ASIC => ASICTarget""".stripMargin + ) } it should "require that all cases are part of the same option" in { @@ -129,7 +126,7 @@ class ModuleChoiceSpec extends ChiselFlatSpec with Utils with MatchesAndOmits { } } -class AddGroupSpec extends ChiselFlatSpec with Utils with MatchesAndOmits { +class AddGroupSpec extends ChiselFlatSpec with Utils with FileCheck { it should "emit options for a registered group even if there are no consumers" in { class ModuleWithoutChoice extends Module { addGroup(Platform) @@ -138,14 +135,11 @@ class AddGroupSpec extends ChiselFlatSpec with Utils with MatchesAndOmits { out := in } - val chirrtl = ChiselStage.emitCHIRRTL(new ModuleWithoutChoice, Array("--full-stacktrace")) - - info("CHIRRTL emission looks correct") - fileCheckString(chirrtl)( + generateFirrtlAndFileCheck(new ModuleWithoutChoice)( """|CHECK: option Platform : - |CHECK-NEXT: FPGA - |CHECK-NEXT: ASIC + |CHECK-NEXT: ASIC @[ + |CHECK-NEXT: FPGA @[ |""".stripMargin - )() + ) } }