Skip to content

Commit

Permalink
Fix boring tap of non-passive source from parent. (#4083)
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.

(cherry picked from commit 0dbedc3)

# Conflicts:
#	core/src/main/scala/chisel3/RawModule.scala
  • Loading branch information
dtzSiFive authored and jackkoenig committed May 24, 2024
1 parent 3e4382b commit afd7bfc
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 10 deletions.
12 changes: 12 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 @@ -209,8 +210,19 @@ abstract class RawModule extends BaseModule {
case (false, true) => Connect(si, left.lref, ProbeRead(Node(right)))
case (false, false) =>
(left, right) match {
<<<<<<< HEAD
case (_: Property[_], _: Property[_]) => PropAssign(si, left.lref, Node(right))
case (_, _) => Connect(si, left.lref, Node(right))
=======
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))
>>>>>>> 0dbedc315 (Fix boring tap of non-passive source from parent. (#4083))
}
}
val secretCommands = if (_closed) {
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 afd7bfc

Please sign in to comment.