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 option for a custom flatten block separator char #4839

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

mikesinouye
Copy link
Contributor

What are the reasons/motivation for this change?
Enables elaborated instance name parity with the default behavior of a commercial synthesis tool.

Explain how this is achieved.
New option in flatten for a custom block separator string.

If applicable, please suggest to reviewers how they can test the change.
flatten -separator /

@mikesinouye mikesinouye marked this pull request as ready for review January 10, 2025 18:36
@mikesinouye
Copy link
Contributor Author

Hey @povik, whenever you get a chance if you could weigh in on this change, thank you!

@povik
Copy link
Member

povik commented Jan 13, 2025

I wonder if there are other places we should be offering this option for consistency, but I can't think of anything atm

@gadfort
Copy link
Contributor

gadfort commented Jan 13, 2025

@povik the synth command can take in -flatten I think it would be helpful if we could set it there too

@povik
Copy link
Member

povik commented Jan 13, 2025

@gadfort We would prefer a scratchpad variable for that (there are many synth passes and scratchpad lets us expose it everywhere)

@povik povik merged commit 6225abe into YosysHQ:main Jan 13, 2025
26 checks passed
@gadfort
Copy link
Contributor

gadfort commented Jan 13, 2025

@povik I didn't know that was an option, I don't see that in the docs, how do you use it?

@povik
Copy link
Member

povik commented Jan 13, 2025

The way you would use it is you would have

scratchpad -set flatten.separator /

or similar at the top of your script.

Then we need code inside flatten which would pick this up.

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