From 5a6ce6604b5bde06dc88c55bc76aaf76aff87437 Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Tue, 1 Dec 2020 22:44:11 -0800 Subject: Fix RegInit of Bundle lits (#1688) Implemented by folding Element.ref into Data.ref. Element.ref had special handling for literals, but because Bundles can also be literals, there were code paths that tried to get the ref of a Bundle literal which was non-existent. Now, all literals are handled together. Because FIRRTL does not have support for Bundle literals, Bundle literal refs are implemented by materializing a Wire.--- core/src/main/scala/chisel3/Data.scala | 38 ++++++++++++----- core/src/main/scala/chisel3/Element.scala | 10 ----- src/test/scala/chiselTests/BundleLiteralSpec.scala | 48 ++++++++++++++++++++++ 3 files changed, 75 insertions(+), 21 deletions(-) diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 5d398aa6..a1f6abf8 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -453,8 +453,9 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { topBindingOpt match { case Some(tb: TopBinding) if (mod == Builder.currentModule) => case Some(pb: PortBinding) if (mod.flatMap(Builder.retrieveParent(_,Builder.currentModule.get)) == Builder.currentModule) => + case Some(_: UnconstrainedBinding) => case _ => - throwException(s"operand is not visible from the current module") + throwException(s"operand '$this' is not visible from the current module") } if (!MonoConnect.checkWhenVisibility(this)) { throwException(s"operand has escaped the scope of the when in which it was constructed") @@ -472,18 +473,33 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { } } - - // Internal API: returns a ref, if bound. Literals should override this as needed. - private[chisel3] def ref: Arg = { - requireIsHardware(this) - if (Builder.currentModule.isDefined) { - // This is allowed (among other cases) for evaluating args of Printf / Assert / Printable, which are - // partially resolved *after* elaboration completes. If this is resolved, the check should be unconditional. - requireVisible() + // Internal API: returns a ref, if bound + private[chisel3] final def ref: Arg = { + def materializeWire(): Arg = { + if (!Builder.currentModule.isDefined) throwException(s"internal error: cannot materialize ref for $this") + implicit val compileOptions = ExplicitCompileOptions.Strict + implicit val sourceInfo = UnlocatableSourceInfo + WireDefault(this).ref } + requireIsHardware(this) topBindingOpt match { - case Some(binding: LitBinding) => throwException(s"internal error: can't handle literal binding $binding") - case Some(binding: TopBinding) => Node(this) + // Literals + case Some(ElementLitBinding(litArg)) => litArg + case Some(BundleLitBinding(litMap)) => + litMap.get(this) match { + case Some(litArg) => litArg + case _ => materializeWire() // FIXME FIRRTL doesn't have Bundle literal expressions + } + case Some(DontCareBinding()) => + materializeWire() // FIXME FIRRTL doesn't have a DontCare expression so materialize a Wire + // Non-literals + case Some(binding: TopBinding) => + if (Builder.currentModule.isDefined) { + // This is allowed (among other cases) for evaluating args of Printf / Assert / Printable, which are + // partially resolved *after* elaboration completes. If this is resolved, the check should be unconditional. + requireVisible() + } + Node(this) case opt => throwException(s"internal error: unknown binding $opt in generating LHS ref") } } diff --git a/core/src/main/scala/chisel3/Element.scala b/core/src/main/scala/chisel3/Element.scala index 7596bc82..55415f3d 100644 --- a/core/src/main/scala/chisel3/Element.scala +++ b/core/src/main/scala/chisel3/Element.scala @@ -40,16 +40,6 @@ abstract class Element extends Data { override def litOption: Option[BigInt] = litArgOption.map(_.num) private[chisel3] def litIsForcedWidth: Option[Boolean] = litArgOption.map(_.forcedWidth) - // provide bits-specific literal handling functionality here - override private[chisel3] def ref: Arg = topBindingOpt match { - case Some(ElementLitBinding(litArg)) => litArg - case Some(BundleLitBinding(litMap)) => litMap.get(this) match { - case Some(litArg) => litArg - case _ => throwException(s"internal error: DontCare should be caught before getting ref") - } - case _ => super.ref - } - private[chisel3] def legacyConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = { // If the source is a DontCare, generate a DefInvalid for the sink, // otherwise, issue a Connect. diff --git a/src/test/scala/chiselTests/BundleLiteralSpec.scala b/src/test/scala/chiselTests/BundleLiteralSpec.scala index 12e62660..2a3ce2c9 100644 --- a/src/test/scala/chiselTests/BundleLiteralSpec.scala +++ b/src/test/scala/chiselTests/BundleLiteralSpec.scala @@ -169,6 +169,54 @@ class BundleLiteralSpec extends ChiselFlatSpec with Utils { } } } + "Bundle literals" should "work as register reset values" in { + assertTesterPasses{ new BasicTester{ + val r = RegInit((new MyBundle).Lit(_.a -> 42.U, _.b -> true.B, _.c -> MyEnum.sB)) + r := (r.asUInt + 1.U).asTypeOf(new MyBundle) // prevent constprop + + // check reset values on first cycle out of reset + chisel3.assert(r.a === 42.U) + chisel3.assert(r.b === true.B) + chisel3.assert(r.c === MyEnum.sB) + stop() + } } + } + + "partially initialized Bundle literals" should "work as register reset values" in { + assertTesterPasses{ new BasicTester{ + val r = RegInit((new MyBundle).Lit(_.a -> 42.U)) + r.a := r.a + 1.U // prevent const prop + chisel3.assert(r.a === 42.U) // coming out of reset + stop() + } } + } + + "Fields extracted from BundleLiterals" should "work as register reset values" in { + assertTesterPasses{ new BasicTester{ + val r = RegInit((new MyBundle).Lit(_.a -> 42.U).a) + r := r + 1.U // prevent const prop + chisel3.assert(r === 42.U) // coming out of reset + stop() + } } + } + + "DontCare fields extracted from BundleLiterals" should "work as register reset values" in { + assertTesterPasses{ new BasicTester{ + val r = RegInit((new MyBundle).Lit(_.a -> 42.U).b) + r := reset.asBool + printf(p"r = $r\n") // Can't assert because reset value is DontCare + stop() + } } + } + + "DontCare fields extracted from BundleLiterals" should "work in other Expressions" in { + assertTesterPasses{ new BasicTester{ + val x = (new MyBundle).Lit(_.a -> 42.U).b || true.B + chisel3.assert(x === true.B) + stop() + } } + } + "bundle literals with bad field specifiers" should "fail" in { val exc = intercept[BundleLiteralException] { extractCause[BundleLiteralException] { -- cgit v1.2.3