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

[Seq] Lower FirMemOp to HWModuleGeneratedOp #5024

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

fabianschuiki
Copy link
Contributor

Add the LowerFirMem pass which lowers seq.firmem ops to a corresponding hw.module.generated op. The pass intends to reflect what LowerToHW does in lowerMemoryDecls: it collects a list of memory ops in the design, distills them down into memory parameters, deduplicates those parameters, creates one hw.module.generated op for each unique memory config, and replaces the memory ops with instances of this generated module.

The LowerFirMem pass is intended as an intermediate solution to allow seq.firmem to be lowered through HWMemSimImpl without changing the latter's implementation. Existing tools that have passes operating on hw.module.generated-style memories can simply run the LowerFirMem pass and have the new seq.firmem work with their existing pipeline.

A few commits down the road we'll want to make HWMemSimImpl work directly with seq.firmem ops, at which point LowerFirMem can go away again. At that point tools are expected to have updated their memory passes to work with seq.firmem directly, completing the transition.

This also adds the LowerFirMem pass to the firtool pipeline, but it doesn't do anything yet since nothing in the pipeline currently creates seq.firmem ops. This will be changed in a follow-up commit.

Follow-up work: This pass is just temporary. It will work with seq.mem directly once seq.firmem and seq.hlmem are merged. As soon as HWMemSimImpl and other passes are updated to work with seq.mem directly themselves, this pass will go away again.

@fabianschuiki fabianschuiki added FIRRTL Involving the `firrtl` dialect Seq Involving the `seq` dialect labels Apr 12, 2023
Copy link
Contributor

@darthscsi darthscsi left a comment

Choose a reason for hiding this comment

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

LGTM

lib/Dialect/Seq/Transforms/LowerFirMem.cpp Outdated Show resolved Hide resolved
Base automatically changed from fschuiki/seq-firmem to main June 22, 2023 16:19
@fabianschuiki fabianschuiki force-pushed the fschuiki/lower-firmem branch 3 times, most recently from c1120f5 to 5c04613 Compare June 22, 2023 17:23
Add the `LowerFirMem` pass which lowers `seq.firmem` ops to a
corresponding `hw.module.generated` op. The pass intends to reflect what
`LowerToHW` does in `lowerMemoryDecls`: it collects a list of memory ops
in the design, distills them down into memory parameters, deduplicates
those parameters, creates one `hw.module.generated` op for each unique
memory config, and replaces the memory ops with instances of this
generated module.

The `LowerFirMem` pass is intended as an intermediate solution to allow
`seq.firmem` to be lowered through `HWMemSimImpl` without changing the
latter's implementation. Existing tools that have passes operating on
`hw.module.generated`-style memories can simply run the `LowerFirMem`
pass and have the new `seq.firmem` work with their existing pipeline.

A few commits down the road we'll want to make `HWMemSimImpl` work
directly with `seq.firmem` ops, at which point `LowerFirMem` can go away
again. At that point tools are expected to have updated their memory
passes to work with `seq.firmem` directly, completing the transition.

This also adds the `LowerFirMem` pass to the firtool pipeline, but it
doesn't do anything yet since nothing in the pipeline currently creates
`seq.firmem` ops. This will be changed in a follow-up commit.
@fabianschuiki fabianschuiki merged commit 7d4d0bb into main Jun 26, 2023
@fabianschuiki fabianschuiki deleted the fschuiki/lower-firmem branch June 26, 2023 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FIRRTL Involving the `firrtl` dialect Seq Involving the `seq` dialect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants