Skip to content

Commit

Permalink
Fix widths for literal values in Bundle literals (#4082)
Browse files Browse the repository at this point in the history
* 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.

(cherry picked from commit 3939e57)

# Conflicts:
#	core/src/main/scala/chisel3/internal/firrtl/IR.scala
#	src/test/scala/chiselTests/BundleLiteralSpec.scala
#	src/test/scala/chiselTests/ConnectableSpec.scala
#	src/test/scala/chiselTests/VecLiteralSpec.scala
  • Loading branch information
jackkoenig authored and mergify[bot] committed May 23, 2024
1 parent e5d3734 commit 976dc81
Show file tree
Hide file tree
Showing 5 changed files with 563 additions and 4 deletions.
27 changes: 23 additions & 4 deletions core/src/main/scala/chisel3/Aggregate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1084,10 +1084,10 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio

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,
Expand Down Expand Up @@ -1169,12 +1169,31 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
}

// 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
}

Expand Down
Loading

0 comments on commit 976dc81

Please sign in to comment.