Skip to content

Commit

Permalink
Name the targets of identity views (#4222) (#4225)
Browse files Browse the repository at this point in the history
(cherry picked from commit 530155d)

Co-authored-by: Jack Koenig <[email protected]>
  • Loading branch information
mergify[bot] and jackkoenig authored Jun 27, 2024
1 parent 261ca7e commit 11288ce
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 4 deletions.
8 changes: 7 additions & 1 deletion core/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import _root_.firrtl.AnnotationSeq
import _root_.firrtl.renamemap.MutableRenameMap
import _root_.firrtl.util.BackendCompilationUtilities._
import _root_.firrtl.{ir => fir}
import chisel3.experimental.dataview.{reify, reifySingleTarget}
import chisel3.experimental.dataview.{reify, reifyIdentityView, reifySingleTarget}
import chisel3.internal.Builder.Prefix
import logger.{LazyLogging, LoggerOption}

Expand Down Expand Up @@ -874,6 +874,12 @@ private[chisel3] object Builder extends LazyLogging {
case Clone(m: experimental.hierarchy.ModuleClone[_]) => namer(m.getPorts, prefix)
case _ =>
}
case (d: Data) =>
// Views are often returned in lieu of the target, so name the target (as appropriate).
// If a view but not identity, return the view and name it since it shows up in .toString and error messages.
// TODO recurse on targets of non-identity views, perhaps with additional prefix from the view.
val reified = reifyIdentityView(d).fold(d)(_._1)
namer(reified, prefix)
case (id: HasId) => namer(id, prefix)
case Some(elt) => nameRecursively(prefix, elt, namer)
case (iter: Iterable[_]) if iter.hasDefiniteSize =>
Expand Down
6 changes: 3 additions & 3 deletions src/test/scala/chiselTests/Vec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,9 @@ class VecSpec extends ChiselPropSpec with Utils {

})
chirrtl should include("output io : { foo : UInt<1>, bar : UInt<1>[0]}")
chirrtl should include("wire _zero_WIRE : { foo : UInt<1>, bar : UInt<1>[0]}")
chirrtl should include("connect _zero_WIRE.foo, UInt<1>(0h0)")
chirrtl should include("connect io, _zero_WIRE")
chirrtl should include("wire zero : { foo : UInt<1>, bar : UInt<1>[0]}")
chirrtl should include("connect zero.foo, UInt<1>(0h0)")
chirrtl should include("connect io, zero")
chirrtl should include("wire w : UInt<1>[0]")
chirrtl should include("connect w, m.io.bar")
}
Expand Down
18 changes: 18 additions & 0 deletions src/test/scala/chiselTests/naming/NamePluginSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -437,4 +437,22 @@ class NamePluginSpec extends ChiselFlatSpec with Utils {
Select.wires(top).map(_.instanceName) should be(List())
}
}

"identity views" should "forward names to their targets" in {
import chisel3.experimental.dataview._
class Test extends Module {
val x = {
val _w = Wire(UInt(3.W))
_w.viewAs[UInt]
}
val y = Wire(UInt(3.W)).readOnly // readOnly is implemented with views

val z = Wire(UInt(3.W))
val zz = z.viewAs[UInt] // But don't accidentally override names
}

aspectTest(() => new Test) { top: Test =>
Select.wires(top).map(_.instanceName) should be(List("x", "y", "z"))
}
}
}

0 comments on commit 11288ce

Please sign in to comment.