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][LowerTypes] Support per-field inner symbols. #5610

Merged
merged 3 commits into from
Jul 20, 2023

Conversation

dtzSiFive
Copy link
Contributor

Only error if their target is not included in lowered output.

Partiton inner symbols to the lowered fields, error if any symbols target fieldID's not present in lowered form.
(e.g., fieldID == 0).

Copy link
Contributor

@prithayan prithayan left a comment

Choose a reason for hiding this comment

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

Looks good.

lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp Show resolved Hide resolved
lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp Outdated Show resolved Hide resolved
lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp Outdated Show resolved Hide resolved
@dtzSiFive dtzSiFive force-pushed the feature/lowertypes-innersym branch from cb2919d to ba0391d Compare July 19, 2023 19:04
Only error if their target is not included in lowered output.

Partiton inner symbols to the lowered fields, error if any symbols
target fieldID's not present in lowered form.
(e.g., fieldID == 0).
TypeSwitch to get access to getIndexAndSubfieldID.

This will do binary searches to find index for fieldID
in bundles (presently), vs previous impl linear walk of
fields (each has pros/cons).
@dtzSiFive dtzSiFive force-pushed the feature/lowertypes-innersym branch from ba0391d to 6f783b3 Compare July 20, 2023 16:47
@dtzSiFive dtzSiFive merged commit 504099f into llvm:main Jul 20, 2023
@dtzSiFive dtzSiFive deleted the feature/lowertypes-innersym branch July 20, 2023 17:17
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.

3 participants