Skip to content

Commit

Permalink
Flush staged commands before/after enter/exit regions. (#4399)
Browse files Browse the repository at this point in the history
Fix BoringUtils (see tests) placement of connection commands
so that they are added to the current command buffer.

Flush staged commands before changing regions to avoid needing
multiple "staging" buffer lists.
Alternatively the current list could be saved and restored.

Tests from #4108 are included and finished, with this change
two of them now pass (the ones encountered in practice, FWIW).
The third is included but marked "ignore" as future work.

This is a bit of a change regarding "staged" commands, as now
before the module is closed they are no longer "secret" since
they are added to the normal command buffers.
They are still staged (deferred) which appears to be useful
to ensure they appear after what's currently being constructed
(specifically, not before).
  • Loading branch information
dtzSiFive authored Sep 13, 2024
1 parent c1e3580 commit 3b53ccb
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 0 deletions.
5 changes: 5 additions & 0 deletions core/src/main/scala/chisel3/RawModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,13 @@ abstract class RawModule extends BaseModule {
}

private[chisel3] def withRegion[A](newRegion: VectorBuilder[Command])(thunk: => A): A = {
_currentRegion ++= stagedSecretCommands
stagedSecretCommands.clear()
var oldRegion = _currentRegion
changeRegion(newRegion)
val result = thunk
_currentRegion ++= stagedSecretCommands
stagedSecretCommands.clear()
changeRegion(oldRegion)
result
}
Expand Down Expand Up @@ -220,6 +224,7 @@ abstract class RawModule extends BaseModule {

// Secret connections can be staged if user bored into children modules
component.secretCommands ++= stagedSecretCommands
stagedSecretCommands.clear()
_component = Some(component)
_component
}
Expand Down
93 changes: 93 additions & 0 deletions src/test/scala/chiselTests/BoringUtilsTapSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,42 @@ class BoringUtilsTapSpec extends ChiselFlatSpec with ChiselRunners with Utils wi
)()
}

// This test requires ability to identify what region to add commands to,
// *after* building them. This is not yet supported.
ignore should "work downwards from grandparent to grandchild through when" in {
class Bar extends RawModule {
val internalWire = Wire(Bool())
}
class Foo extends RawModule {
when(true.B) {
val bar = Module(new Bar)
}
}
class Top extends RawModule {
val foo = Module(new Foo)
val out = IO(Bool())
out := DontCare

when(true.B) {
val w = WireInit(
Bool(),
BoringUtils.tapAndRead((chisel3.aop.Select.collectDeep(foo) { case b: Bar => b }).head.internalWire)
)
}
}
val chirrtl = circt.stage.ChiselStage.emitCHIRRTL(new Top)

// The define should be at the end of the when block.
matchesAndOmits(chirrtl)(
" when UInt<1>(0h1) :",
" inst bar of Bar",
" define w_bore = bar.w_bore"
)()

// Check is valid FIRRTL.
circt.stage.ChiselStage.emitFIRRTLDialect(new Top)
}

it should "work upwards from child to parent" in {
class Foo(parentData: Data) extends RawModule {
val outProbe = IO(probe.Probe(Bool()))
Expand Down Expand Up @@ -109,6 +145,63 @@ class BoringUtilsTapSpec extends ChiselFlatSpec with ChiselRunners with Utils wi
)()
}

it should "work upwards from grandchild to grandparent through when" in {
class Bar(grandParentData: Data) extends RawModule {
val out = IO(Bool())
out := BoringUtils.tapAndRead(grandParentData)
}
class Foo(parentData: Data) extends RawModule {
when(true.B) {
val bar = Module(new Bar(parentData))
}
}
class Top extends RawModule {
val parentWire = Wire(Bool())
parentWire := DontCare
val foo = Module(new Foo(parentWire))
}
val chirrtl = circt.stage.ChiselStage.emitCHIRRTL(new Top)

// The connect should be at the end of the when block.
matchesAndOmits(chirrtl)(
" when UInt<1>(0h1) :",
" inst bar of Bar",
" connect bar.out_bore, out_bore"
)()

// Check is valid FIRRTL.
circt.stage.ChiselStage.emitFIRRTLDialect(new Top)
}

it should "work upwards from grandchild to grandparent into layer" in {
object TestLayer extends layer.Layer(layer.LayerConfig.Extract())
class Bar(grandParentData: Data) extends RawModule {
val out = IO(Bool())
out := BoringUtils.tapAndRead(grandParentData)
}
class Foo(parentData: Data) extends RawModule {
layer.block(TestLayer) {
val bar = Module(new Bar(parentData))
}
}
class Top extends RawModule {
val parentWire = Wire(Bool())
parentWire := DontCare
val foo = Module(new Foo(parentWire))
}
val chirrtl = circt.stage.ChiselStage.emitCHIRRTL(new Top)

// The connect should be at the end of the layerblock.
matchesAndOmits(chirrtl)(
" layerblock TestLayer :",
" inst bar of Bar",
" connect bar.out_bore, out_bore"
)()

// Check is valid FIRRTL and builds to SV.
circt.stage.ChiselStage.emitSystemVerilog(new Top)
}

it should "work from child to its sibling" in {
class Bar extends RawModule {
val a = Wire(Bool())
Expand Down

0 comments on commit 3b53ccb

Please sign in to comment.