Skip to content

Converting Yosys to Submodule #3156

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Converting Yosys to Submodule #3156

wants to merge 9 commits into from

Conversation

loglav03
Copy link
Contributor

@loglav03 loglav03 commented Jun 20, 2025

Description

Converted Yosys from vendored in subdirectory to submodule. Yosys submodule points to version 0.53.
Important Notes About Yosys Submodule:

  • Yosys now lives in libs/EXTERNAL instead of in the VTR root.
  • Points to Yosys version 0.53.
  • The CmakeLists in libs/EXTERNAL does the process for initializing and updating submodules within yosys, and building yosys.

Motivation and Context

Converting yosys to a submodule makes it easier to update yosys to a newer version if needed. It also makes it easier to implement yosys-slang to replace synlig/surelog.

How Has This Been Tested?

Ran the required CI tests -> ALL PASSED (with updated golden results)

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • [ x] All new and existing tests passed

@github-actions github-actions bot added lang-cpp C/C++ code build Build system lang-perl Perl code lang-python Python code lang-make CMake/Make code lang-hdl Hardware Description Language (Verilog/VHDL) lang-shell Shell scripts (bash etc.) external_libs Parmys labels Jun 20, 2025
@loglav03 loglav03 changed the title [WIP] Converting Yosys to Submodule Converting Yosys to Submodule Jun 24, 2025
@loglav03 loglav03 marked this pull request as ready for review June 24, 2025 13:26
@AlexandreSinger
Copy link
Contributor

AlexandreSinger commented Jun 24, 2025

Thank you so much @loglav03 !

I am going to frame this and put this on my wall for the reason why submodules are necessary:
image

I have a few comments, but I cannot put them in the standard review spots since your PR is so large that it crashes the GitHub UI for me...

Please resolve the following and it looks good to me!

  • CMakeLists.txt : Please delete the line you commented out. Please also delete everything else in that if statement. I believe it goes unused; however, be careful with the set(MAKE_PROGRAM) line. I think that is supposed to be in libs/EXTERNAL/CmakeLists.txt with your new changes. That define for "-D_YOSYS_" is interesting, I am not sure who is using this; but if it is used by parmys it should be moved to the if statement below that adds the parmys directory, if its used by YOSYS it should be moved to the new section you added; however I am not sure.
  • dev/subtree_config.xml : Now that you have made Yosys a submodule, please remove it from the subtree config. This is as simple as just deleting the following lines:
    <subtree
    name="yosys"
    internal_path="yosys"
    external_url="https://github.com/YosysHQ/yosys.git"
    default_external_ref="yosys-0.32"/>
  • libs/EXTERNAL/CMakeLists.txt : What you added here is very interesting. It builds Yosys from our repo instead of adding its CMakeLists (which is a good choice in my opinion; their CMakeLists would interfere with our build system I think). If you got this from somewhere else, can you please add a comment with a link to where you got it. The consequence of this is that now it looks like we have two builds happening simultaneously:
image

I do not think this is a problem. From prior investigations into our build system, I have found that two builds were happening simultaneously for a long time; it is just now being more explicit about it. I do not want this to block your PR; however can you please raise an issue about this so we can track it. We really should not have two builds happening simultaneously as it can throttle machines when multiple cores are used to compile. I am not sure if the "MAKE_PROGRAM" variable is the culprit for this, but I am pretty sure this is not an easy problem to fix.

I like this PR a lot and want to see it in ASAP, so please just resolve the three comments above and we can merge this in. Thank you so much for doing this.

@loglav03
Copy link
Contributor Author

You're welcome! I'll go ahead and resolve those issues and raise an issue about the multiple builds if I can't easily solve it myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build system external_libs lang-cpp C/C++ code lang-hdl Hardware Description Language (Verilog/VHDL) lang-make CMake/Make code lang-perl Perl code lang-python Python code lang-shell Shell scripts (bash etc.) Parmys
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants