Fix widths for literal values in Bundle literals (#4082)

* Fix widths for literal values in Bundle literals

Previously, the user-specified (or unspecified minimum width) of the
literal would be used in some operations like concatenation.

* Implicitly truncate too-wide literals in Bundle literals

This is to fix a bug in concatenation without breaking things relying on
implicit truncation. This will be a warning in newer versions of Chisel.

core/src/main/scala/chisel3/Aggregate.scala

@@ -1122,10 +1122,10 @@ abstract class Record extends Aggregate {
 
     requireIsChiselType(this, "bundle literal constructor model")
     val clone = cloneType
-    val cloneFields = getRecursiveFields(clone, "(bundle root)").toMap
+    val cloneFields = getRecursiveFields(clone, "_").toMap
 
     // Create the Bundle literal binding from litargs of arguments
-    val bundleLitMap = elems.map { fn => fn(clone) }.flatMap {
+    val bundleLitMapping = elems.map { fn => fn(clone) }.flatMap {
       case (field, value) =>
         val fieldName = cloneFields.getOrElse(
           field,
@@ -1207,12 +1207,31 @@ abstract class Record extends Aggregate {
     }
 
     // don't convert to a Map yet to preserve duplicate keys
-    val duplicates = bundleLitMap.map(_._1).groupBy(identity).collect { case (x, elts) if elts.size > 1 => x }
+    val duplicates = bundleLitMapping.map(_._1).groupBy(identity).collect { case (x, elts) if elts.size > 1 => x }
     if (!duplicates.isEmpty) {
       val duplicateNames = duplicates.map(cloneFields(_)).mkString(", ")
       throw new BundleLiteralException(s"duplicate fields $duplicateNames in Bundle literal constructor")
     }
-    clone.bind(BundleLitBinding(bundleLitMap.toMap))
+    // Check widths and sign extend as appropriate.
+    val bundleLitMap = bundleLitMapping.view.map {
+      case (field, value) =>
+        field.width match {
+          // If width is unknown, then it is set by the literal value.
+          case UnknownWidth()                 => field -> value
+          case width @ KnownWidth(widthValue) =>
+            // TODO make this a warning then an error, but for older versions, just truncate.
+            val valuex = if (widthValue < value.width.get) {
+              // Mask the value to the width of the field.
+              val mask = (BigInt(1) << widthValue) - 1
+              value.cloneWithValue(value.num & mask).cloneWithWidth(width)
+            } else if (widthValue > value.width.get) value.cloneWithWidth(width)
+            // Otherwise, ensure width is same as that of the field.
+            else value
+
+            field -> valuex
+        }
+    }.toMap
+    clone.bind(BundleLitBinding(bundleLitMap))
     clone
   }
 

core/src/main/scala/chisel3/internal/firrtl/IR.scala

@@ -121,14 +121,20 @@ private[chisel3] object ir {
       elem
     }
 
-    /** Provides a mechanism that LitArgs can have their width adjusted
-      * to match other members of a VecLiteral
+    /** Provides a mechanism that LitArgs can have their width adjusted.
       *
       * @param newWidth the new width for this
       * @return
       */
     def cloneWithWidth(newWidth: Width): this.type
 
+    /** Provides a mechanism that LitArgs can have their value adjusted.
+      *
+      * @param newWidth the new width for this
+      * @return
+      */
+    def cloneWithValue(newValue: BigInt): this.type
+
     protected def minWidth: Int
     if (forcedWidth) {
       require(
@@ -150,6 +156,8 @@ private[chisel3] object ir {
       ULit(n, newWidth).asInstanceOf[this.type]
     }
 
+    def cloneWithValue(newValue: BigInt): this.type = ULit(newValue, w).asInstanceOf[this.type]
+
     require(n >= 0, s"UInt literal ${n} is negative")
   }
 
@@ -163,6 +171,8 @@ private[chisel3] object ir {
     def cloneWithWidth(newWidth: Width): this.type = {
       SLit(n, newWidth).asInstanceOf[this.type]
     }
+
+    def cloneWithValue(newValue: BigInt): this.type = SLit(newValue, w).asInstanceOf[this.type]
   }
 
   /** Literal property value.

src/test/scala/chiselTests/BundleLiteralSpec.scala

@@ -344,6 +344,18 @@ class BundleLiteralSpec extends ChiselFlatSpec with Utils {
     exc.getMessage should include(".c")
   }
 
+  "bundle literals with too-wide of literal values" should "truncate" in {
+    class SimpleBundle extends Bundle {
+      val a = UInt(4.W)
+      val b = UInt(4.W)
+    }
+    val chirrtl = ChiselStage.emitCHIRRTL(new RawModule {
+      val lit = (new SimpleBundle).Lit(_.a -> 0xde.U, _.b -> 0xad.U)
+      val x = lit.asUInt
+    })
+    chirrtl should include("node x = cat(UInt<4>(0he), UInt<4>(0hd))")
+  }
+
   "partial bundle literals" should "fail to pack" in {
     ChiselStage.emitCHIRRTL {
       new RawModule {
@@ -367,4 +379,19 @@ class BundleLiteralSpec extends ChiselFlatSpec with Utils {
       lit.litOption should equal(Some(0))
     })
   }
+
+  "bundle literals" should "use the widths of the Bundle fields rather than the widths of the literals" in {
+    class SimpleBundle extends Bundle {
+      val a = UInt(4.W)
+      val b = UInt(4.W)
+    }
+    val chirrtl = ChiselStage.emitCHIRRTL(new RawModule {
+      // Whether the user specifies a width or not.
+      val lit = (new SimpleBundle).Lit(_.a -> 0x3.U, _.b -> 0x3.U(3.W))
+      lit.a.getWidth should be(4)
+      lit.b.getWidth should be(4)
+      val cat = lit.asUInt
+    })
+    chirrtl should include("node cat = cat(UInt<4>(0h3), UInt<4>(0h3))")
+  }
 }

src/test/scala/chiselTests/ConnectableSpec.scala

@@ -1060,7 +1060,7 @@ class ConnectableSpec extends ChiselFunSpec with Utils {
           "connect out.valid, in.valid",
           "connect in.ready, out.ready",
           "connect out.data.b, in.data.b",
-          "connect out.data.c, UInt<1>(0h1)"
+          "connect out.data.c, UInt<2>(0h1)"
         ),
         Nil
       )
@@ -1535,11 +1535,11 @@ class ConnectableSpec extends ChiselFunSpec with Utils {
         out,
         Seq(
           """wire w0 : { foo : UInt<3>, flip bar : UInt<3>}""",
-          """connect w0.bar, UInt<1>(0h1)""",
-          """connect w0.foo, UInt<1>(0h0)""",
+          """connect w0.bar, UInt<3>(0h1)""",
+          """connect w0.foo, UInt<3>(0h0)""",
           """wire w1 : { foo : UInt<3>, flip bar : UInt<3>}""",
-          """connect w1.bar, UInt<1>(0h1)""",
-          """connect w1.foo, UInt<1>(0h0)""",
+          """connect w1.bar, UInt<3>(0h1)""",
+          """connect w1.foo, UInt<3>(0h0)""",
           """connect w1, w0"""
         ),
         Nil
@@ -1625,7 +1625,7 @@ class ConnectableSpec extends ChiselFunSpec with Utils {
       testCheck(
         out,
         Seq(
-          """connect out.data, UInt<1>(0h0)""",
+          """connect out.data, UInt<32>(0h0)""",
           """connect in.ready, out.ready""",
           """connect out.valid, in.valid"""
         ),

src/test/scala/chiselTests/VecLiteralSpec.scala

@@ -436,6 +436,7 @@ class VecLiteralSpec extends ChiselFreeSpec with Utils {
 
   class VecExample extends RawModule {
     val out = IO(Output(Vec(2, new SubBundle)))
+    // Note that 22.U is too wide for bar so gets truncated below.
     val bundle = Vec(2, new SubBundle).Lit(
       0 -> (new SubBundle).Lit(_.foo -> 42.U, _.bar -> 22.U),
       1 -> (new SubBundle).Lit(_.foo -> 7.U, _.bar -> 3.U)
@@ -445,10 +446,10 @@ class VecLiteralSpec extends ChiselFreeSpec with Utils {
 
   "vec literals can contain bundles and should not be bulk connected" in {
     val chirrtl = ChiselStage.emitCHIRRTL(new VecExample)
-    chirrtl should include("""connect out[0].bar, UInt<5>(0h16)""")
-    chirrtl should include("""connect out[0].foo, UInt<6>(0h2a)""")
-    chirrtl should include("""connect out[1].bar, UInt<2>(0h3)""")
-    chirrtl should include("""connect out[1].foo, UInt<3>(0h7)""")
+    chirrtl should include("""connect out[0].bar, UInt<4>(0h6)""")
+    chirrtl should include("""connect out[0].foo, UInt<8>(0h2a)""")
+    chirrtl should include("""connect out[1].bar, UInt<4>(0h3)""")
+    chirrtl should include("""connect out[1].foo, UInt<8>(0h7)""")
   }
 
   "vec literals can have bundle children" in {
@@ -530,4 +531,20 @@ class VecLiteralSpec extends ChiselFreeSpec with Utils {
       lit.litOption should equal(Some(0))
     })
   }
+
+  "Vec literals should use the width of the Vec element rather than the widths of the literals" in {
+    val chirrtl = ChiselStage.emitCHIRRTL(new RawModule {
+      // Whether the user specifies a width or not.
+      val lit0 = (Vec(2, UInt(4.W))).Lit(0 -> 0x3.U, 1 -> 0x2.U(3.W))
+      lit0(0).getWidth should be(4)
+      lit0(1).getWidth should be(4)
+      val uint0 = lit0.asUInt
+      val lit1 = Vec.Lit(0x3.U, 0x2.U(4.W))
+      lit1(0).getWidth should be(4)
+      lit1(1).getWidth should be(4)
+      val uint1 = lit1.asUInt
+    })
+    chirrtl should include("node uint0 = cat(UInt<4>(0h2), UInt<4>(0h3))")
+    chirrtl should include("node uint1 = cat(UInt<4>(0h2), UInt<4>(0h3))")
+  }
 }