-
Notifications
You must be signed in to change notification settings - Fork 318
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 FirRomOp to HWModuleGeneratedOp #5800
Open
vowstar
wants to merge
2
commits into
llvm:main
Choose a base branch
from
vowstar:seq-lower-firrom
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
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
Add a new `FirRomOp` to the Seq dialect to capture the semantics of FIRRTL ROM at the HW level. The intent behind this change is the same as for `FirMemOp`. ROMs are represented as a declaration with the `seq.firrom` which returns a value representing the ROM itself. This value is of type `!seq.firrom<12 x 42>` which represents an opaque handle to the ROM; it encodes the `depth (12)` and `width (42)` of the ROM. ROM ports are represented as separate operations: - `seq.firrom.read_port` These operations take the ROM itself as their first operand. The remaining operands are the specific signals required for the corresponding type of port: clocks, enables, mode select, etc. Read ports also return the read data as their result. Enables are optional to allow for more concise IR for simple memory configurations. The reason why adding a new `FirRomOp` instead of reusing the `FirMemOp` to process Read-Only-Memory is because there are still huge differences in the behaviors of ROM, SRAM, and RegFile in different foundry memory compilers, and their DFT processing is also very different. If they are all merged into `FirMemOp`, a lot of attributes must be added to `FirMemOp`, resulting in the post-processing having to judge these attributes, which increases the burden of post-processing. Therefore, adding `FirRomOp` as a new op is conducive to behavior distinction and later maintenance, especially when we want to add DFT and physical implementation details in the real chip design process, it will be more conducive to implementation, and reduce the cost of code errors caused by maintenance. This commit merely adds the `FirRomOp` alongside some canonicalization and general tests, but the op isn't used by any part of CIRCT yet. Use in FIRRTL-to-HW lowering follows as a separate commit. Signed-off-by: Huang Rui <[email protected]>
2e5d2d7
to
5c9d0c6
Compare
SpriteOvO
reviewed
Aug 9, 2023
5c9d0c6
to
cc6a548
Compare
Add the `LowerFirRom` pass which lowers `seq.firrom` ops to a corresponding `hw.module.generated` op. The pass collects a list of ROM ops in the design, distills them down into ROM parameters, deduplicates those parameters, creates one `hw.module.generated` op for each unique ROM config, and replaces the ROM ops with instances of this generated module. This commit reflects that ROM can configure the `numReadPorts` parameter, because there are multi-port ROMs, for example: https://www.intel.com/content/dam/altera-www/global/en_US/others/support/examples/download/dual_port_rom.zip Signed-off-by: Huang Rui <[email protected]>
cc6a548
to
5df53b3
Compare
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 the
LowerFirRom
pass which lowersseq.firrom
ops to acorresponding
hw.module.generated
op. The pass collects a listof ROM ops in the design, distills them down into ROM parameters,
deduplicates those parameters, creates one
hw.module.generated
opfor each unique ROM config, and replaces the ROM ops with instances
of this generated module.
This commit reflects that ROM can configure the
numReadPorts
parameter, because there are multi-port ROMs, for example:
https://www.intel.com/content/dam/altera-www/global/en_US/others/support/examples/download/dual_port_rom.zip
This commit blocked by: #5798