Support Property in BoringUtils. #3784
Conversation
|
Note that this PR is currently stacked on top of #3783. |
| case (false, false) => | ||
| (left, right) match { | ||
| case (_: Property[_], _: Property[_]) => PropAssign(si, left.lref, Node(right)) | ||
| case (_, _) => Connect(si, left.lref, Node(right)) |
There was a problem hiding this comment.
What if left: Property[_] and right: _? this should be forbidden explicitly and throw it at elaboration time.
There was a problem hiding this comment.
True. Maybe I can delegate to MonoConnect which checks these things. Or just add the runtime check here.
There was a problem hiding this comment.
See my comment on the other thread--this would only happen if there was a bug in the implementation of BoringUtils, so I think it is reasonable to just deal with the two cases we care about.
| case (false, false) => | ||
| (left, right) match { | ||
| case (_: Property[_], _: Property[_]) => PropAssign(si, left.lref, Node(right)) | ||
| case (_, _) => Connect(si, left.lref, Node(right)) |
There was a problem hiding this comment.
What should happen for boring from property to data and when/what error will show up? I assume this will blow up later. Is the error message reasonable?
There was a problem hiding this comment.
Yep Jiuyang had a similar question. I will add an error test and make sure the error is reasonable.
There was a problem hiding this comment.
Ok, so this logic is only ever used to connect ports/wires created by BoringUtils itself. If the user bores from a Property to a Data, the Property ports will be bored and secret connections from Property to Property made here, but then when the bored output is connected to a non-Property data, you'll get the usual error:
Connection between sink (Foo.a: IO[Bool]) and source (chisel3.properties.Property$$anon$9@371acd0e) failed @: Sink (Bool) and Source (chisel3.properties.Property$$anon$9@66435227) have different types.
This adds support for BoringUtils.bore to bore and connect Property ports.
mikeurbach
force-pushed
the
mikeurbach/property-boring-utils
branch
from
45dec73 to
16cb7f3
Compare
This adds support for BoringUtils.bore to bore and connect Property ports. (cherry picked from commit 178d8f0)
This adds support for BoringUtils.bore to bore and connect Property ports. (cherry picked from commit 178d8f0) Co-authored-by: Mike Urbach <[email protected]>
Contributor Checklist
docs/src?Type of Improvement
Desired Merge Strategy
Release Notes
This adds support for BoringUtils.bore to bore and connect Property ports.
Reviewer Checklist (only modified by reviewer)
3.6.x,5.x, or6.xdepending on impact, API modification or big change:7.0)?Enable auto-merge (squash), clean up the commit message, and label withPlease Merge.Create a merge commit.