Fix RegInit of Bundle lits (bp #1688) (#1689)

* 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.

(cherry picked from commit 5a6ce66)

* Waive bincompat issue on package private method

Co-authored-by: Jack Koenig <koenig@sifive.com>

build.sbt

@@ -197,6 +197,7 @@ lazy val core = (project in file("core")).
       ProblemFilters.exclude[DirectMissingMethodProblem]("chisel3.internal.firrtl.Converter.convert"),
       ProblemFilters.exclude[IncompatibleMethTypeProblem]("chisel3.internal.firrtl.Converter.extractType"),
       ProblemFilters.exclude[DirectMissingMethodProblem]("chisel3.internal.firrtl.Converter.extractType$default$2"),
+      ProblemFilters.exclude[FinalMethodProblem]("chisel3.Data.ref"),
       // Scala 2.11 only issue, new concrete methods in traits require recompilation of implementing classes
       // Not a problem because HasId is package private so all implementers are in chisel3 itself
       // Note there is no problem for user subtypes of Record because setRef is implemented by Data

core/src/main/scala/chisel3/Data.scala

@@ -454,8 +454,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")
@@ -473,18 +474,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")
     }
   }

core/src/main/scala/chisel3/Element.scala

@@ -41,16 +41,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.

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] {