ucb-bar / chisel2-deprecated

chisel.eecs.berkeley.edu
388 stars 90 forks source link

Bulk connects do not override bundle components #699

Open colinschmidt opened 8 years ago

colinschmidt commented 8 years ago

This code behaves differently in Chisel2 and 3. It would be good if the compatibility checker would catch it.

io.rocc.resp.ready := !(wb_reg_valid && wb_ctrl.wxd)
...
csr.io.rocc <> io.rocc

In Chisel2 because the csr module does not write to rocc.resp.ready the initial assignment is preserved and last connect semantics are ignored.

In Chisel3 last connect semantics are honored and the second bulk connect overrides the previous assignment. This leads to a random value since csr.io.rocc.resp.ready was never written to.

This anomalous behavior in Chisel2 can cause issues in porting from chisel2 to chisel3 as it silently generates a different circuit.

sdtwigg commented 8 years ago

Does csr.io.rocc.resp.ready exist? If so, I think you should expect csr.io.rocc <> io.rocc to override the assignment.

Would a dead code elimination pass help with resolving this class of issue?

colinschmidt commented 8 years ago

I think a good solution (at least to help migration) would just be another chisel 3 compatibility warning/error when you do this.

Any bulk connects to a bundle after any part of it has been assigned to (bulk or otherwise) will potentially hit this issue.

I just ran into it again with something like the following.

a.io.bundle <> io.bundle
...
b.io.bundle <> io.bundle

In this case the module b only reads from its io.bundle but chisel3 generates an assign to io.bundle's outputs with b.io.bundle causing a random value to be assigned.

jkorinth commented 7 years ago

It would be helpful to be able to specify inclusion/exclusion filters in the bulk connect. Something along the lines of

a.io.bundle <> io.bundle exclude x, y, z
b.io.bundle <> io.bundle only x, y, z

Ideally, the syntax would support wildcards/regex on the port/sub-bundle names, e.g.:

a.io.bundle <> io.bundle exclude x.*

Besides solving double assignments with only the last-one-wins-rule, this would also facilitate literate programming in the sense of making connections more explicit.

sdtwigg commented 7 years ago

Bundle.elements: LinkedHashMap[String, Data] is already available. So users already are free to write a function that connects two Bundles with different semantics (like the proposed filtering) than the current string matching scheme.

On Sat, Sep 3, 2016 at 12:02 AM, jkorinth notifications@github.com wrote:

It would be helpful to be able to specify inclusion/exclusion filters in the bulk connect. Something along the lines of

a.io.bundle <> io.bundle exclude x, y, z b.io.bundle <> io.bundle only x, y, z

Ideally, the syntax would support wildcards/regex on the port/sub-bundle names, e.g.:

a.io.bundle <> io.bundle exclude x.*

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ucb-bar/chisel/issues/699#issuecomment-244531348, or mute the thread https://github.com/notifications/unsubscribe-auth/AA9_-LR0CYiMe3q2i9XoSAsOC_kEZsc5ks5qmRuDgaJpZM4ISNlI .