Make it legal for concrete resets to drive abstract reset #2018
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This has been legal in FIRRTL since v1.2.3 (when reset inference started using a unification-style algorithm) but was never exposed in the Chisel API. Also delete the overridden connects in AsyncReset and ResetType which just duplicate logic from MonoConnect.
azidar
approved these changes
Jul 8, 2021
mergify bot
pushed a commit
that referenced
this pull request
Jul 9, 2021
This has been legal in FIRRTL since v1.2.3 (when reset inference started using a unification-style algorithm) but was never exposed in the Chisel API. Also delete the overridden connects in AsyncReset and ResetType which just duplicate logic from MonoConnect. (cherry picked from commit 4b7b771)
mergify bot
pushed a commit
that referenced
this pull request
Jul 9, 2021
This has been legal in FIRRTL since v1.2.3 (when reset inference started using a unification-style algorithm) but was never exposed in the Chisel API. Also delete the overridden connects in AsyncReset and ResetType which just duplicate logic from MonoConnect. (cherry picked from commit 4b7b771)
mergify bot
pushed a commit
that referenced
this pull request
Jul 9, 2021
This has been legal in FIRRTL since v1.2.3 (when reset inference started using a unification-style algorithm) but was never exposed in the Chisel API. Also delete the overridden connects in AsyncReset and ResetType which just duplicate logic from MonoConnect. (cherry picked from commit 4b7b771)
mergify bot
added a commit
that referenced
this pull request
Jul 9, 2021
…2018) (#2029) * Make it legal for concrete resets to drive abstract reset (#2018) This has been legal in FIRRTL since v1.2.3 (when reset inference started using a unification-style algorithm) but was never exposed in the Chisel API. Also delete the overridden connects in AsyncReset and ResetType which just duplicate logic from MonoConnect. (cherry picked from commit 4b7b771) * Fix issues with tests Co-authored-by: Jack Koenig <[email protected]>
mergify bot
added a commit
that referenced
this pull request
Jul 9, 2021
…2018) (#2028) * Make it legal for concrete resets to drive abstract reset (#2018) This has been legal in FIRRTL since v1.2.3 (when reset inference started using a unification-style algorithm) but was never exposed in the Chisel API. Also delete the overridden connects in AsyncReset and ResetType which just duplicate logic from MonoConnect. (cherry picked from commit 4b7b771) * Fix issues with tests Co-authored-by: Jack Koenig <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mergify bot
added a commit
that referenced
this pull request
Jul 9, 2021
…2018) (#2027) * Make it legal for concrete resets to drive abstract reset (#2018) This has been legal in FIRRTL since v1.2.3 (when reset inference started using a unification-style algorithm) but was never exposed in the Chisel API. Also delete the overridden connects in AsyncReset and ResetType which just duplicate logic from MonoConnect. (cherry picked from commit 4b7b771) * Fix issues with tests Co-authored-by: Jack Koenig <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
@jackkoenig I think this introduce some bug: |
|
Not this PR, I believe #2023 is responsible, looking into it and should have a fix tomorrow. |
azidar
pushed a commit
that referenced
this pull request
Jul 9, 2021
This has been legal in FIRRTL since v1.2.3 (when reset inference started using a unification-style algorithm) but was never exposed in the Chisel API. Also delete the overridden connects in AsyncReset and ResetType which just duplicate logic from MonoConnect.
|
This should now be fixed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This has been legal in FIRRTL since v1.2.3 (when reset inference started
using a unification-style algorithm) but was never exposed in the Chisel
API.
Also delete the overridden connects in AsyncReset and ResetType which
just duplicate logic from MonoConnect.
Contributor Checklist
docs/src?Type of Improvement
API Impact
This makes it legal to drive a concrete reset (
Bool()orAsyncReset()) with an abstract reset (Reset()), this has been legal in FIRRTL since v1.2.3 and was always intended to be exposed in Chisel but I forgot 🙃Backend Code Generation Impact
No impact
Desired Merge Strategy
Release Notes
Make it legal to drive a concrete reset (
Bool()orAsyncReset()) with an abstract reset (Reset()). Reset inference in FIRRTL works both forwards and backwards so the connection to the concrete reset will influence the type of the driving abstract reset.Reviewer Checklist (only modified by reviewer)
Please Merge?