Skip to content

Commit

Permalink
Materialize wires for .ref of Aggregate views (backport #4080) (#4085)
Browse files Browse the repository at this point in the history
* Materialize wires for .ref of Aggregate views (#4080)

Fixes muxing of Aggregate views, and fixes ProbeValues of Aggregate
views.

(cherry picked from commit 5af27e5)

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

* Resolve backport conflicts

---------

Co-authored-by: Jack Koenig <koenig@sifive.com>
  • Loading branch information
mergify[bot] and jackkoenig authored May 24, 2024
1 parent e30ff6d commit 990966c
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 6 deletions.
13 changes: 7 additions & 6 deletions core/src/main/scala/chisel3/Data.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

package chisel3

import chisel3.experimental.dataview.reify
import chisel3.experimental.dataview.{reify, reifySingleData}

import scala.language.experimental.macros
import chisel3.experimental.{Analog, BaseModule}
Expand Down Expand Up @@ -644,11 +644,12 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
topBindingOpt match {
// DataView
case Some(ViewBinding(target)) => reify(target).ref
case Some(AggregateViewBinding(viewMap)) =>
viewMap.get(this) match {
case None => materializeWire() // FIXME FIRRTL doesn't have Aggregate Init expressions
// This should not be possible because Element does the lookup in .topBindingOpt
case x: Some[_] => throwException(s"Internal Error: In .ref for $this got '$topBindingOpt' and '$x'")
case Some(_: AggregateViewBinding) =>
reifySingleData(this) match {
// If this is an identity view (a view of something of the same type), return ref of target
case Some(target) if this.typeEquivalent(target) => target.ref
// Otherwise, we need to materialize hardware of the correct type
case _ => materializeWire()
}
// Literals
case Some(ElementLitBinding(litArg)) => litArg
Expand Down
38 changes: 38 additions & 0 deletions src/test/scala/chiselTests/experimental/DataView.scala
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,44 @@ class DataViewSpec extends ChiselFlatSpec {
verilog should include("assign z = sel ? b : d;")
}

it should "support muxing between views of Bundles" in {
import SimpleBundleDataView._
class MyModule extends Module {
val cond = IO(Input(Bool()))
val in1 = IO(Input(new BundleA(8)))
val in2 = IO(Input(new BundleA(8)))
val out = IO(Output(new BundleB(8)))

out := Mux(cond, in1.viewAs[BundleB], in2.viewAs[BundleB])
}
val firrtl = ChiselStage.emitCHIRRTL(new MyModule)
val lines = Seq(
"wire _out_WIRE : { bar : UInt<8>}",
"_out_WIRE.bar <= in1.foo",
"wire _out_WIRE_1 : { bar : UInt<8>}",
"_out_WIRE_1.bar <= in2.foo",
"node _out_T = mux(cond, _out_WIRE, _out_WIRE_1)"
)
for (line <- lines) {
firrtl should include(line)
}
}

it should "not generate extra wires when muxing between identity views of Bundles" in {
import SimpleBundleDataView._
class MyModule extends Module {
val cond = IO(Input(Bool()))
val in1 = IO(Input(new BundleA(8)))
val in2 = IO(Input(new BundleA(8)))
val out = IO(Output(new BundleA(8)))

out := Mux(cond, in1.viewAs[BundleA], in2.viewAs[BundleA])
}
val firrtl = ChiselStage.emitCHIRRTL(new MyModule)
firrtl should include("node _out_T = mux(cond, in1, in2)")
firrtl shouldNot include("wire")
}

// This example should be turned into a built-in feature
it should "enable viewing Seqs as Vecs" in {

Expand Down

0 comments on commit 990966c

Please sign in to comment.