Skip to content

Commit

Permalink
Fix literal handling for views of empty Aggregates (backport #4071) (#…
Browse files Browse the repository at this point in the history
…4074)

* Fix literal handling for views of empty Aggregates (#4071)

Previously, a view of an empty Record or Vec would always have a
litValue of 0.

(cherry picked from commit 11c3f51)

# Conflicts:
#	src/test/scala/chiselTests/experimental/DataView.scala

* Fixup backport conflicts

---------

Co-authored-by: Jack Koenig <koenig@sifive.com>
  • Loading branch information
mergify[bot] and jackkoenig authored May 20, 2024
1 parent f57f66d commit 7f1ab3c
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 0 deletions.
9 changes: 9 additions & 0 deletions core/src/main/scala/chisel3/Aggregate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ sealed abstract class Aggregate extends Data {
}

topBindingOpt match {
// Don't accidentally invent a literal value for a view that is empty
case Some(_: AggregateViewBinding) if this.getElements.isEmpty =>
reifySingleData(this) match {
case Some(target: Aggregate) => target.checkingLitOption(checkForDontCares)
case _ =>
val msg =
s"It should not be possible to have an empty Aggregate view that doesn't reify to a single target, but got $this"
Builder.exception(msg)(UnlocatableSourceInfo)
}
case Some(_: BundleLitBinding | _: VecLitBinding | _: AggregateViewBinding) =>
// Records store elements in reverse order and higher indices are more significant in Vecs
this.getElements.foldRight(Option(BigInt(0)))(shiftAdd)
Expand Down
34 changes: 34 additions & 0 deletions src/test/scala/chiselTests/experimental/DataView.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import chisel3._
import chisel3.experimental.conversions._
import chisel3.experimental.dataview._
import chisel3.experimental.{Analog, HWTuple2}
import chisel3.experimental.BundleLiterals._
import chisel3.experimental.VecLiterals._
import chisel3.reflect.DataMirror.internal.chiselTypeClone
import chisel3.util.{Decoupled, DecoupledIO, Valid, ValidIO}
import chiselTests.ChiselFlatSpec
Expand Down Expand Up @@ -999,6 +1001,38 @@ class DataViewSpec extends ChiselFlatSpec {
ChiselStage.emitCHIRRTL(new MyModule)
}

it should "NOT invent literal values for views of empty Aggregates" in {
class MyModule extends Module {
val emptyBundle = IO(Output(new Bundle {}))
val emptyVec = IO(Output(Vec(0, UInt(8.W))))

val bundleView = emptyBundle.viewAs[Bundle]
val vecView = emptyVec.viewAs[Vec[UInt]]

emptyBundle.litOption should be(None)
bundleView.litOption should be(None)
emptyVec.litOption should be(None)
vecView.litOption should be(None)
}
ChiselStage.emitCHIRRTL(new MyModule)
}

it should "support literal values for views of empty Aggregates" in {
class MyModule extends Module {
val emptyBundle = (new Bundle {}).Lit()
val emptyVec = Vec(0, UInt(8.W)).Lit()

val bundleView = emptyBundle.viewAs[Bundle]
val vecView = emptyVec.viewAs[Vec[UInt]]

emptyBundle.litOption should be(Some(0))
bundleView.litOption should be(Some(0))
emptyVec.litOption should be(Some(0))
vecView.litOption should be(Some(0))
}
ChiselStage.emitCHIRRTL(new MyModule)
}

behavior.of("PartialDataView")

it should "still error if the mapping is non-total in the view" in {
Expand Down

0 comments on commit 7f1ab3c

Please sign in to comment.