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] Move LowerLayers after LowerXMR #8405

Draft
wants to merge 2 commits into
base: dev/seldridge/hw-factor-out-hierpath-builder
Choose a base branch
from

Conversation

seldridge
Copy link
Member

@seldridge seldridge commented Apr 9, 2025

Move the LowerLayers pass after the LowerXMR pass. To do this, all
passes at the end of the CHIRRTL to Low FIRRTL pipeline are moved after
LowerXMR. This is necessary because the LowerLayers pass cannot, at
present, be moved after the passes at the end of the pipeline.

This is done to enable forcing out of layers. By lowering probes to XMRs,
the layers can be lowered trivially to modules/instances and their XMRs
will now (seemingly) magically just work. By doing the loweirng in this
way, it avoids ever having to represent an input probe in the FIRRTL
dialect.

A consequence of this is that there are now simplifying
assumptions (preconditions) that can be made about the LowerLayers pass:

  1. It will never see a number of probe ops because the LowerXMR pass has
    a postcondition that all of these are removed.

  2. Input and output ports can never be created on modules created from
    layer blocks.

While I am generally always in favor of passes being relocatable anywhere
in the pipeline, this is one pass that really does not make sense to be
relocatable. In effect, this pass is part of the lowering from FIRRTL to
HW. There is no point in being able to use it earlier in the pipeline.
That said, it still is tested to work with things which we may one-day
preserve, like WhenOps.

Todo

  • Handle zero width

@seldridge seldridge requested a review from darthscsi as a code owner April 9, 2025 20:07
@seldridge seldridge requested a review from rwy7 April 9, 2025 20:08
@seldridge seldridge marked this pull request as draft April 10, 2025 01:54
Move the `LowerLayers` pass after the `LowerXMR` pass.  To do this, all
passes at the end of the CHIRRTL to Low FIRRTL pipeline are moved after
`LowerXMR`.  This is necessary because the `LowerLayers` pass cannot, at
present, be moved after the passes at the end of the pipeline.

This is done to enable forcing out of layers.  By lowering probes to XMRs,
the layers can be lowered trivially to modules/instances and their XMRs
will now (seemingly) magically just work.  By doing the loweirng in this
way, it avoids ever having to represent an input probe in the FIRRTL
dialect.

A consequence of this is that there are now simplifying
assumptions (preconditions) that can be made about the `LowerLayers` pass:

1. It will never see a number of probe ops because the `LowerXMR` pass has
   a postcondition that all of these are removed.

2. Input and output ports can never be created on modules created from
   layer blocks.

While I am generally always in favor of passes being relocatable anywhere
in the pipeline, this is one pass that really does _not_ make sense to be
relocatable.  In effect, this pass is part of the lowering from FIRRTL to
HW.  There is no point in being able to use it earlier in the pipeline.
That said, it still is tested to work with things which we may one-day
preserve, like `WhenOp`s.

Signed-off-by: Schuyler Eldridge <[email protected]>
@seldridge seldridge force-pushed the dev/seldridge/firrtl-lower-layers-support-clone-xmrref-ops branch from 772757e to 38b6bef Compare April 12, 2025 03:39
@seldridge seldridge changed the title [firrtl] Clone XMRRef Ops into layers [firrtl] Move LowerLayers after LowerXMR Apr 12, 2025
@seldridge
Copy link
Member Author

The changes in this PR introduce a lot of unreachable code in LowerLayers. I will clean this up as long as there is consensus that we want to enforce the precondition that LowerXMR runs before this always (and thereby certain ops will never be seen). It is possible to continue to support this, but it will not be load bearing in firtool.

@seldridge seldridge changed the base branch from main to dev/seldridge/hw-factor-out-hierpath-builder April 12, 2025 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant