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

[CreateSiFiveMetadata] Generate firrtl.class instead of om.class #6736

Merged
merged 14 commits into from
Apr 8, 2024

Conversation

prithayan
Copy link
Contributor

Update the object model generated by the CreateSiFiveMetadata pass to generate firrtl dialect, instead of om dialect.
Ensure all firrtl.class associated with the metadata are instantiated inside a top level firrtl.class SiFive_Metadata , which is instantiated inside the top level firrtl.module. This is required for ensuring that subsequent passes can lower it correctly.
Generate all the hierarchical paths to the memory and associate them with the path operation for the memory metadata.

@prithayan prithayan force-pushed the dev/pbarua/om-metadata branch from e7fc23e to bdb3a0b Compare February 28, 2024 17:20
@prithayan prithayan force-pushed the dev/pbarua/om-metadata branch from bdb3a0b to 7f369c6 Compare March 11, 2024 23:53
@prithayan prithayan marked this pull request as ready for review March 12, 2024 13:46
@prithayan prithayan force-pushed the dev/pbarua/om-metadata branch from d376c05 to 369ce58 Compare April 2, 2024 17:31
Copy link
Contributor

@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

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

I took another brief look, and I think this is looking good. Is this ready for a final review?

@prithayan
Copy link
Contributor Author

I took another brief look, and I think this is looking good. Is this ready for a final review?

Thanks Mike, yes, this is ready for final review.

Copy link
Contributor

@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

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

Some minor comments. The one thing I think we need to consider is whether we're keeping the instance graph up to date or not. I don't think the current PR is valid, since it adds classes and objects but then says we can preserve the instance graph.

@prithayan
Copy link
Contributor Author

Thanks Mike, yes this pass is updating the hierarchy, the analysis is no longer preserved.

@prithayan prithayan force-pushed the dev/pbarua/om-metadata branch from 7533430 to 20fccce Compare April 8, 2024 14:33
@prithayan prithayan merged commit fbef8b9 into main Apr 8, 2024
4 checks passed
@prithayan prithayan deleted the dev/pbarua/om-metadata branch April 8, 2024 19:33
cepheus69 pushed a commit to cepheus69/circt that referenced this pull request Apr 22, 2024
…vm#6736)

Update the object model generated by the CreateSiFiveMetadata pass to generate
 firrtl dialect, instead of om dialect. Ensure all firrtl.class associated with
 the metadata are instantiated inside a top level firrtl.class SiFive_Metadata,
 which is instantiated inside the top level firrtl.module. This is required for
 ensuring that subsequent passes can lower it correctly.
Also generate all the hierarchical paths to the memory and associates them with
 the path operation for the memory metadata.
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.

2 participants