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

add support for using ABCs library merging when providing multiple liberty files #4340

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

gadfort
Copy link
Contributor

@gadfort gadfort commented Apr 15, 2024

Changes:

  • adds the -m flag to the ABC script when multiple liberty files are specified to indicate to ABC that is should merge the libraries, this solves the issue of only the last liberty file specified getting used for synthesis.

Requirements:

  • needs the abc fork to be synced with the main abc to get the new flag.

@QuantamHD
Copy link
Contributor

@povik This one is also useful to Google. Any chance the Yosys team could propritize it?

@povik povik self-assigned this Apr 15, 2024
@povik
Copy link
Member

povik commented Apr 16, 2024

I am testing this change against current ABC. When I pass two Liberty files, the cells from the first one get prefixed with L1_ while the cells from the second get no prefix. That looks weird.

I saw there's the read_lib -p option, but we don't use it with neither of the read_lib invocations.

@gadfort
Copy link
Contributor Author

gadfort commented Apr 16, 2024

@povik thanks for checking this. It looks like when the use prefix option was added it didn't account for one of the libraries.
I've added a PR to fix that berkeley-abc/abc#287.
We shouldn't use the -p since that is what is supposed to add the prefixes to the names, which is not the behavior we want.

@povik
Copy link
Member

povik commented Apr 16, 2024

I've added a PR to fix that berkeley-abc/abc#287.

I will watch that one get merged so we update our fork after that.

We shouldn't use the -p since that is what is supposed to add the prefixes to the names, which is not the behavior we want.

OK, I thought so.

@gadfort
Copy link
Contributor Author

gadfort commented Apr 16, 2024

@povik should have been merged now.

@povik
Copy link
Member

povik commented Apr 17, 2024

Let's go ahead and merge this even if the vendored ABC isn't at the right version yet. There's no harm.

@povik povik merged commit 171577f into YosysHQ:main Apr 17, 2024
17 checks passed
@gadfort gadfort deleted the abc-lib-merge branch April 17, 2024 20:35
@povik povik mentioned this pull request Apr 22, 2024
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