Skip to content

Commit

Permalink
Fix boring tap of non-passive source from parent.
Browse files Browse the repository at this point in the history
Expected result is fully aligned, which is what happens
when reading from a probe.

When the source is already at the LCA (from parent)
there's no probe and the secret connections only support
a few commands.

For lack of a way to do, e.g., `:#=` here,
use `read(probe(x))` to get the fully aligned result
that's expected and bored down to the child.

This isn't ideal but fixes this using only the sorts of expressions
and commands that we've already committed to supporting.

Fixes #3557.
  • Loading branch information
dtzSiFive committed May 23, 2024
1 parent 7e0cd10 commit 1607e7e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 10 deletions.
6 changes: 6 additions & 0 deletions core/src/main/scala/chisel3/RawModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import chisel3.experimental.hierarchy.{InstanceClone, ModuleClone}
import chisel3.properties.{DynamicObject, Property, StaticObject}
import chisel3.internal.Builder._
import chisel3.internal.firrtl.ir._
import chisel3.reflect.DataMirror
import _root_.firrtl.annotations.{IsModule, ModuleTarget}
import scala.collection.immutable.VectorBuilder
import scala.collection.mutable.ArrayBuffer
Expand Down Expand Up @@ -217,6 +218,11 @@ abstract class RawModule extends BaseModule {
// For non-probe, directly create Nodes for lhs, skipping visibility check to support BoringUtils.drive.
(left, right) match {
case (_: Property[_], _: Property[_]) => PropAssign(si, Node(left), Node(right))
// Use `connect lhs, read(probe(rhs))` if lhs is passive version of rhs.
// This provides solution for this: https://github.com/chipsalliance/chisel/issues/3557
case (_, _) if !DataMirror.checkAlignmentTypeEquivalence(left, right) &&
DataMirror.checkAlignmentTypeEquivalence(left, Output(chiselTypeOf(right)))
=> Connect(si, Node(left), ProbeRead(ProbeExpr(Node(right))))
case (_, _) => Connect(si, Node(left), Node(right))
}
}
Expand Down
12 changes: 2 additions & 10 deletions src/test/scala/chiselTests/BoringUtilsTapSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -470,13 +470,7 @@ class BoringUtilsTapSpec extends ChiselFlatSpec with ChiselRunners with Utils wi

class Foo extends RawModule {
val a = WireInit(DecoupledIO(Bool()), DontCare)
val dummyA = Wire(Output(chiselTypeOf(a)))
// FIXME we shouldn't need this intermediate wire
// https://github.com/chipsalliance/chisel/issues/3557
dummyA :#= a
dontTouch(a)

val bar = Module(new Bar(dummyA))
val bar = Module(new Bar(a))
}

val chirrtl = circt.stage.ChiselStage.emitCHIRRTL(new Foo, Array("--full-stacktrace"))
Expand All @@ -486,9 +480,7 @@ class BoringUtilsTapSpec extends ChiselFlatSpec with ChiselRunners with Utils wi
"input bore : { ready : UInt<1>, valid : UInt<1>, bits : UInt<1>}",
"module Foo :",
"wire a : { flip ready : UInt<1>, valid : UInt<1>, bits : UInt<1>}",
// FIXME shouldn't need intermediate wire
"wire dummyA : { ready : UInt<1>, valid : UInt<1>, bits : UInt<1>}",
"connect bar.bore, dummyA"
"connect bar.bore, read(probe(a))"
)()

// Check that firtool also passes
Expand Down

0 comments on commit 1607e7e

Please sign in to comment.