chipsalliance / chipsalliance/chisel

Bulk connect on BlackBox io doesn't work for Bundles defined compatibility mode

Open
#870 1 comment 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.