In BoringUtils.drive, don't bore into the final module for inputs. #3998
Conversation
This follows up on #3960 to address the behavior at the final module for the sink being driven. We already have special handling for boring out from the original source, and we need the inverse here. When we reach the final sink, if it is an input port, we should return it, rather than boring into the module and connecting to it.
| @@ -254,8 +257,9 @@ object BoringUtils { | |||
| path.zip(connectionLocation).foldLeft(source) { | |||
| case (rhs, (module, conLoc)) if (module.isFullyClosed) => boringError(module); DontCare.asInstanceOf[A] | |||
| case (rhs, (module, _)) | |||
| if (up && module == path(0) && isPort(rhs) && (!createProbe.nonEmpty || !createProbe.get.writable)) => { | |||
| // When drilling from the original source, if it's already a port just return it. | |||
| if ((up || isDriveDone(rhs)) && module == path(0) && isPort(rhs) && | |||
There was a problem hiding this comment.
This diff isn't great due to line wrapping, but the change was up -> (up || isDriveDone(rhs)), which has the logic to know if we need to stop at the last boundary or go inside it. We stop on an input, but we go inside on an output. The tests show what I mean. Previously, we'd always go inside, which creates illegal FIRRTL for driving and input.
There was a problem hiding this comment.
Aside to I suppose mostly myself: We could probably change the RWProbe bores from inside behavior now, actually.
There was a problem hiding this comment.
This logic could use a cleanup in general... I've only made it worse. So maybe settling on the final RWProbe behavior could be done as part of that.
| @@ -254,8 +257,9 @@ object BoringUtils { | |||
| path.zip(connectionLocation).foldLeft(source) { | |||
| case (rhs, (module, conLoc)) if (module.isFullyClosed) => boringError(module); DontCare.asInstanceOf[A] | |||
| case (rhs, (module, _)) | |||
| if (up && module == path(0) && isPort(rhs) && (!createProbe.nonEmpty || !createProbe.get.writable)) => { | |||
| // When drilling from the original source, if it's already a port just return it. | |||
| if ((up || isDriveDone(rhs)) && module == path(0) && isPort(rhs) && | |||
There was a problem hiding this comment.
Aside to I suppose mostly myself: We could probably change the RWProbe bores from inside behavior now, actually.
…3998) This follows up on #3960 to address the behavior at the final module for the sink being driven. We already have special handling for boring out from the original source, and we need the inverse here. When we reach the final sink, if it is an input port, we should use it, rather than boring into the module and connecting to it.
Contributor Checklist
docs/src?Type of Improvement
Desired Merge Strategy
Release Notes
This follows up on #3960 to address the behavior at the final module for the sink being driven. We already have special handling for boring out from the original source, and we need the inverse here. When we reach the final sink, if it is an input port, we should use it, rather than boring into the module and connecting to it.
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.