chisel
chisel copied to clipboard
chipsalliance
BoringUtils.bore does not work properly in `when` scope
Type of issue: Bug Report
Please provide the steps to reproduce the problem:
class Top extends Module {
val io = IO(Input(Bool()))
val inner = Module(new Inner)
when(io) {
val b = BoringUtils.bore(inner.a)
}
}
class Inner extends Module {
val a = WireDefault(false.B)
}
What is the current behavior?
STDERR:
/nfs-nvme/home/tanghaojin/chisel-template/Top.fir:22:13: error: use of unknown declaration 'b'
connect b, inner.b_bore @[src/main/scala/gcd/GCD.scala 17:29]
^
And the firrtl output is
FIRRTL version 3.1.0
circuit Top :
module Inner :
input clock : Clock
input reset : Reset
output b_bore : UInt<1> @[src/main/scala/gcd/GCD.scala 22:29]
wire a : UInt<1> @[src/main/scala/gcd/GCD.scala 27:22]
connect a, UInt<1>(0h0) @[src/main/scala/gcd/GCD.scala 27:22]
connect b_bore, a @[src/main/scala/gcd/GCD.scala 22:29]
module Top :
input clock : Clock
input reset : UInt<1>
input io : UInt<1> @[src/main/scala/gcd/GCD.scala 18:14]
inst inner of Inner @[src/main/scala/gcd/GCD.scala 20:21]
connect inner.clock, clock
connect inner.reset, reset
when io : @[src/main/scala/gcd/GCD.scala 21:12]
wire b : UInt<1> @[src/main/scala/gcd/GCD.scala 22:29]
connect b, inner.b_bore @[src/main/scala/gcd/GCD.scala 22:29]
What is the expected behavior? Pass verilog generation
Please tell us about your environment:
- version: 6.0.0-M3
- OS:
Linux xxx 5.15.0-69-generic #76~20.04.1-Ubuntu SMP Mon Mar 20 15:54:19 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Other Information
What is the use case for changing the behavior?
OK this is certainly a bug. Like declaring register or memory inside when block. In the boring utils case, I personally think it should be forbidden.
OK this is certainly a bug. Like declaring register or memory inside when block. In the boring utils case, I personally think it should be forbidden.
I agree that boring utils should not be used in when scope directly. But in our case, it is wrapped in some APIs, and these APIs are used in when. Maybe we still need boring utils working in when properly.
Discussing in Chisel dev, we can probably fix this with a new internal* API that allows optionally instantiating a new Wire or Reg (or IO?) outside of the when scope. BoringUtils would then use this API to ensure there is a wire to connect to outside of the when.
Just noticed, did we have same issue for Mem.read, Mem.write in when block sometime ago?
FYI, https://github.com/llvm/circt/issues/1561 may also relate to this issue.
While sharing kinda related bits--
I'm not sure if it's reachable or good Chisel if it's possible, but FWIW CIRCT's wiring for taps is careful to insert connect's in a place that is safe re:the things it's connecting, primarily to ensure can plumb probes to declarations under when and especially to ensure could XMR into a module instantiated under a when, see: https://github.com/llvm/circt/pull/4323/files#diff-8d84fe9849e43b9a0268cc454d6dc82d0dbe16e18392f355ff1d155f41c323b9 -- idea being if you can instantiate a module under a when that shouldn't mean you can't access its contents via tap/probe/XMR. Is this something that could be done here (also, "should" it?)?
I'll see if can put together an example for being able to tap declarations under when (vs here where the boring command itself is in the when), since as long as we can declare things that way they seem like they should be tap-able, but if that's obviously the same scenario please LMK :wink: . If nothing else should be able to define them out manually if needed, hopefully.
Never taught it to insert temporary/wire for case of wiring between two declarations under sibling regions (when/else blocks, "scopes"?), partly because this is only likely to be a thing to route out XMR's, just a heads-up (there's a test for this in that link commit FWIW).
@dtzSiFive: having Chisel be able to add statements to do the connections inside the when (carefully and safely!) is the best outcome here. This, however, very much goes against the design of Chisel's internals which wants to assume that the construction of Chisel's IR is created from a serial execution of a Chisel module's constructor that: (1) changes builder state to indicate that we are now in a new module, (2) serially "pushes" commands into a command array for that module, (3) "closes" the module when the constructor finishes, and (4) pops back to the previous module. Technologies (D/I) were then built on top of this which assumed that modules were unchanging after (3) happens.
To get around this, two separate approaches have been tried:
- Injecting Aspects pack FIRRTL text into a separate area and then rely on a FIRRTL compiler to append this text. This cannot add ports. This is now largely deprecated.
- Modern
BoringUtilsadds secret connections and secret ports alongside the normal builder command array to allow for more stuff to be added to a module (with restrictions on adding ports in certain situations).
When (2) was added, this was a compromise because of concerns around Chisel's internals and the incompatibility of arbitrary modifications with D/I.
That said, we're rapidly approaching a point where rethinking Chisel's internals to use a more standard compiler architecture makes sense. (Contrary to common wisdom that linked lists are never used outside of coding interviews, there is a sound reason why intrusive linked lists are the building blocks of both LLVM and MLIR compiler IRs. 😉)
tl;dr: Yes, we should be able to add these directly from Chisel, inside the when blocks, without resorting to clever tricks.
