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

[FIRRTL][LowerToHW] Lower contract ops #8159

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fabianschuiki
Copy link
Contributor

Lower firrtl.contract to verif.contract ops. The lowering itself is pretty straightforward, since the ops are basically equivalent. The only exception are block arguments in FIRRTL that get replaced with op results in Verif due to the switch from an SSACFG region with dominance to a graph region.

The block arguments require a slight change to how nested operations are handled by the pass. Currently, a post-order walk over the operations is used. This is problematic since the parent operation does not get an opportunity to lower its block arguments before its child operations are lowered, which may need access to those block arguments. Switching to a pre-order walk does not work, since that wouldn't allow us to modify the operations during the walk, which we obviously do during the lowering. This commit therefore adds a worklist that tracks operation ranges that are yet to be lowered. The worklist allows parent operations to lower themselves and any of their block arguments, and then add the nested regions, blocks, or operations onto the worklist for lowering. It's basically a pre-walk with the ability to mutate the parent operation before children are lowered.

Block arguments are also no longer automatically assumed to be already lowered. This was only valid for modules, since we lower the module ports before we lower the module body. In case of ops like contracts, the block arguments still need to be lowered. To fix this, modules push a 1-1 mapping of their block arguments into the map of lowered values, which allows other ops to provide some other lowering for their block arguments.

@fabianschuiki fabianschuiki added the FIRRTL Involving the `firrtl` dialect label Jan 31, 2025
@fabianschuiki fabianschuiki force-pushed the fschuiki/imdce-with-blocks branch from be71092 to 725e092 Compare February 4, 2025 18:14
@fabianschuiki fabianschuiki force-pushed the fschuiki/lower-firrtl-contracts branch 2 times, most recently from 9d4e065 to e78f330 Compare February 4, 2025 21:14
@fabianschuiki fabianschuiki force-pushed the fschuiki/imdce-with-blocks branch 2 times, most recently from 018c5eb to 8a3165b Compare February 5, 2025 21:24
@fabianschuiki fabianschuiki force-pushed the fschuiki/lower-firrtl-contracts branch 2 times, most recently from 545ae59 to ee14267 Compare February 6, 2025 17:46
@fabianschuiki fabianschuiki force-pushed the fschuiki/imdce-with-blocks branch 2 times, most recently from 251ccfd to 97a2d65 Compare February 6, 2025 21:37
@fabianschuiki fabianschuiki force-pushed the fschuiki/lower-firrtl-contracts branch from ee14267 to 3bf3a99 Compare February 6, 2025 21:37
@fabianschuiki fabianschuiki force-pushed the fschuiki/imdce-with-blocks branch from 97a2d65 to 249010f Compare February 6, 2025 21:39
@fabianschuiki fabianschuiki force-pushed the fschuiki/lower-firrtl-contracts branch from 3bf3a99 to 050e628 Compare February 6, 2025 21:39
Base automatically changed from fschuiki/imdce-with-blocks to main February 6, 2025 21:43
Lower `firrtl.contract` to `verif.contract` ops. The lowering itself is
pretty straightforward, since the ops are basically equivalent. The only
exception are block arguments in FIRRTL that get replaced with op
results in Verif due to the switch from an SSACFG region with dominance
to a graph region.

The block arguments require a slight change to how nested operations are
handled by the pass. Currently, a post-order walk over the operations is
used. This is problematic since the parent operation does not get an
opportunity to lower its block arguments before its child operations are
lowered, which may need access to those block arguments. Switching to a
pre-order walk does not work, since that wouldn't allow us to modify
the operations during the walk, which we obviously do during the
lowering. This commit therefore adds a worklist that tracks operation
ranges that are yet to be lowered. The worklist allows parent operations
to lower themselves and any of their block arguments, and then add the
nested regions, blocks, or operations onto the worklist for lowering.
It's basically a pre-walk with the ability to mutate the parent
operation before children are lowered.

Block arguments are also no longer automatically assumed to be already
lowered. This was only valid for modules, since we lower the module
ports before we lower the module body. In case of ops like contracts,
the block arguments still need to be lowered. To fix this, modules push
a 1-1 mapping of their block arguments into the map of lowered values,
which allows other ops to provide some other lowering for their block
arguments.
@fabianschuiki fabianschuiki force-pushed the fschuiki/lower-firrtl-contracts branch from 050e628 to 439231c Compare February 6, 2025 21:44
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.

1 participant