Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LowerFIRRTLToHW] Use backedges over temporary wires, NFC #5513

Merged
merged 2 commits into from
Jun 28, 2023

Conversation

youngar
Copy link
Member

@youngar youngar commented Jun 28, 2023

This changes LowerFIRRTLToHW to use backedges instead of temporary wire operations. The temporary wires were guaranteed to be optimized away. Now we slightly more gracefully handle the, technically illegal, cases of these wires having no value driven to them. This change is NFC in the FIRRTL pipeline where these invariants have already been verified. We can remove the infrastructure for creating these temporary wires, as it is not used anymore.

@youngar youngar added the FIRRTL Involving the `firrtl` dialect label Jun 28, 2023
@youngar
Copy link
Member Author

youngar commented Jun 28, 2023

This is a follow up from #5510.

This changes LowerFIRRTLToHW to use backedges instead of temporary wire
operations. The temporary wires were guaranteed to be optimized away.
Now we slightly more gracefully handle the, technically illegal, cases
of these wires having no value driven to them.  This change is NFC in
the FIRRTL pipeline where these invariants have already been verified.
We can remove the infrastructure for creating these temporary wires, as
there are now more uses.
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🥳

Foreign typed ports can never be considered an InOut port, so they can
be handled by the code path which checks if the port is an input port,
which is handled identically.
@youngar youngar merged commit 29ad033 into llvm:main Jun 28, 2023
5 checks passed
@youngar youngar deleted the firrt-lower-less-wires branch June 28, 2023 22:23
calebmkim pushed a commit to andrewb1999/circt that referenced this pull request Jul 12, 2023
This changes LowerFIRRTLToHW to use backedges instead of temporary wire
operations. The temporary wires were guaranteed to be optimized away.
Now we slightly more gracefully handle the, technically illegal, cases
of these wires having no value driven to them.  This change is NFC in
the FIRRTL pipeline where these invariants have already been verified.
We can remove the infrastructure for creating these temporary wires, as
it is not used anymore.

Foreign typed ports can never be considered an InOut port, so they can
be handled by the code path which checks if the port is an input port,
which is handled identically.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FIRRTL Involving the `firrtl` dialect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants