[FIRRTL] Input probe support. #6121
Merged
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
dtzSiFive
force-pushed
the
feature/input-probes
branch
2 times, most recently
from
5c603e2
to
9d1a22d
Compare
dtzSiFive
commented
Sep 22, 2023
| // and ensure they work before allowing them from user input. | ||
| if (hasInputRef(port.type, port.isOutput())) | ||
| return emitError(std::get<1>(portAndLoc), | ||
| "input probes not yet supported"); |
There was a problem hiding this comment.
Locally, some time ago, added verifier that rejected input probes on public modules. Other than test churn, I'm not sure why that wasn't landed. Should go revive, suspect it was just unreachable and so not worth the change.
dtzSiFive
force-pushed
the
feature/input-probes
branch
4 times, most recently
from
3b4a6a6
to
5c9329f
Compare
prithayan
approved these changes
Oct 2, 2023
Don't require it but if it's valid, grab and maintain it.
Ensure supported input probes don't block dedup. (They will be removed)
dtzSiFive
force-pushed
the
feature/input-probes
branch
from
5c9329f
to
394dbeb
Compare
|
Thanks!! |
mikeurbach
pushed a commit
that referenced
this pull request
Oct 2, 2023
Add and enable support for input probes. Per spec, all uses (XMR read/force/release) of a probe must resolve to below the point of use in the hierarchy. With this PR this is now supported in majority of cases. The main exception to full support, with test demonstrating, is "n-turns" where a module uses an input probe (that resolves below that point) by sending it out and it comes back in equivalently driven in all instantiation contexts. In this PR: * Input probes enabled in parser * Pass added to run after "de-squiggling" to drop all input probes, diagnosing when this is not possible (illegal input). * Test cases involving input probes from spec now work.
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.
Add and enable support for input probes.
Per spec, all uses (XMR read/force/release) of a probe must resolve to below the point of use in the hierarchy.
With this PR this is now supported in majority of cases.
The main exception to full support, with test demonstrating, is "n-turns" where a module uses an input probe (that resolves below that point) by sending it out and it comes back in equivalently driven in all instantiation contexts.
In this PR: