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

Merge the Cadence and Synopsys plugins into Hammer #713

Merged
merged 12 commits into from
Mar 20, 2023

Conversation

vighneshiyer
Copy link
Contributor

I copied over the files from the Cadence and Synopsys plugin repos into hammer. There should be no functional changes. Running the e2e tests with the Cadence flow is still pending. Do we believe the Synopsys flow still works out-of-the-box?

Copy link
Contributor

@harrisonliew harrisonliew left a comment

Choose a reason for hiding this comment

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

Let's not include Synopsys DC & ICC. These plugins are too old/not maintained (erase them from the git history too).

Otherwise, I don't see why directly copying them in should break anything.

.gitmodules Outdated Show resolved Hide resolved
hammer/drc/icv/README.md Show resolved Hide resolved
@vighneshiyer vighneshiyer force-pushed the merge-synopsys-cadence-plugins branch 2 times, most recently from 6144871 to 6fb207b Compare March 14, 2023 00:02
@vighneshiyer
Copy link
Contributor Author

I've squashed my commits and removed the DC/ICC plugins - let me know if there's anything else you'd like to fix. I still need to try the e2e tests.

Copy link
Contributor

@harrisonliew harrisonliew 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, pending e2e testing.

doc/CAD-Tools/ICV.md Outdated Show resolved Hide resolved
hammer/synthesis/genus/__init__.py Outdated Show resolved Hide resolved
@vighneshiyer vighneshiyer force-pushed the merge-synopsys-cadence-plugins branch from 588a2e8 to 734fd32 Compare March 15, 2023 00:35
@vighneshiyer vighneshiyer marked this pull request as ready for review March 15, 2023 00:36
Copy link
Contributor

@harrisonliew harrisonliew left a comment

Choose a reason for hiding this comment

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

Approving again, pending the other e2e test combos.

@vighneshiyer
Copy link
Contributor Author

I've tested RTL->GDS in e2e using Cadence tools using sky130 on the A machines. There is an issue with post-syn and post-par simulation where the dff stdcell doesn't appear to update on the rising clock edge, but I suspect this is an issue with the PDK's verilog models and not related to this PR. We can merge this unless there's something else you'd like to check.

@harrisonliew
Copy link
Contributor

All relevant issues from hammer-cadence-plugins and hammer-synopsys-plugins have been transferred over now.

@vighneshiyer
Copy link
Contributor Author

Everything looks good to go - you can look over the diff one more time and if it looks good, we can merge.

@harrisonliew
Copy link
Contributor

I had to cherry-pick this commit and update sky130 bwrc keys.

@harrisonliew harrisonliew added this pull request to the merge queue Mar 20, 2023
@harrisonliew harrisonliew merged commit 419766d into master Mar 20, 2023
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