chipsalliance / chipsalliance/chisel
Bulk connect on BlackBox io doesn't work for Bundles defined compatibility mode
- Dominant language
- Scala
- Stars
- 4.8k
- Forks
- 658
- Avg merge
- 18h 59m
- Merged PRs (30d)
- 14
Description
Note that the title includes `BlackBoxes` defined in `import chisel3._` code that use Bundles defined in `import Chisel._` code, so this bug percolates into attempted migration to `chisel3._` as well. You can connect to the fields of `BlackBox` `io`s just fine, but bulk connecting to the full aggregate doesn't work.
I'm almost certain the bug comes from the special case handling of BlackBox `io`. Since those Bundles are just kind of shims for "top-level" ports, the Bundle doesn't actually exist in the FIRRTL. And due to https://github.com/freechipsproject/chisel3/pull/595, bulk connect semantics for compatibility mode defined bundles take effect even when used in `chisel3._` code.
Proposed solutions:
1. Hacky support for exactly this case--detect when the Bundle involved in a bulk connect is a BlackBox io and do something different
* Adding a FIRRTL mechanism for bulk connecting to an instance could make the implementation of the bulk connect clean and provide a potentially useful Chisel API, but the detection is still one off
1. Generalized support for "shim" Bundles that actually inject their elements into their outer context
* This could provide a reasonable solution for https://github.com/freechipsproject/chisel3/issues/612 although I suspect actual "set name" support is still desirable
* Unclear how this would work for a Vec of shim Bundles, maybe that would be illegal but shim Bundles would be legal in other Bundles and as IO?
1. Change the mechanism for BlackBoxes to use a FIRRTL "set name" API so that the Bundles are no longer shims
* This also fixes https://github.com/freechipsproject/chisel3/issues/612
**Type of issue**: bug report
**Impact**: no functional change
**Development Phase**: request
**Other information**
**If the current behavior is a bug, please provide the steps to reproduce the problem:**
You can run the following code:
```scala
import Chisel._
class MyBlackBox extends BlackBox {
val io = IO(new Bundle {
val in = Input(UInt(8.W))
val out = Output(UInt(8.W))
})
}
class TopModule extends Module {
val io = IO(new Bundle {
val in = Input(UInt(8.W))
val out = Output(UInt(8.W))
})
val inst = Module(new MyBlackBox)
io <> inst.io
}
object Top extends App {
println(chisel3.Driver.emit(() => new TopModule))
}
```
**What is the current behavior?**
And it prints:
```
;buildInfoPackage: chisel3, version: 3.2-SNAPSHOT, scalaVersion: 2.11.12, sbtVersion: 1.1.1
circuit TopModule :
extmodule MyBlackBox :
output out : UInt<8>
input in : UInt<8>
defname = MyBlackBox
module TopModule :
input clock : Clock
input reset : UInt<1>
output io : {flip in : UInt<8>, out : UInt<8>}
clock is invalid
reset is invalid
io is invalid
inst inst of MyBlackBox @[Test.scala 67:20]
inst.out is invalid
inst.in is invalid
io <- ?? @[Test.scala 68:6]
```
Clearly the `io <- ??` is wrong. Prior to the literal refactor, this errored with a `None.get` in the emitter.
**What is the expected behavior?**
Obviously bulk connecting with the `io` of a `BlackBox` should work.
**What is the use case for changing the behavior?**
🐛🔨
Contributor guide
Research direction
Run the supplied TopModule/MyBlackBox reproducer through chisel3.Driver.emit and trace the bulk-connect handling for BlackBox io and compatibility-mode Bundles. Done means the generated circuit no longer contains `io <- ??` and bulk connecting `TopModule.io` to `inst.io` produces valid connections; the supported shim or FIRRTL approach must be established before implementation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- scala
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100