-
Notifications
You must be signed in to change notification settings - Fork 813
Allow "lite" Chipyard builds with a minimal subset of submodules #2212
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
Conversation
|
This is a really cool idea! Some thoughts that come to mind:
The other thought is, how to support people integrating new generators according to the rules, now that the rule also includes editing the shell script and a slightly different sbt process? Since all the effort to refactor things just collapses if people add their new submodules to default build anyway. Or maybe that doesn't matter as long proper integration is enforced when upstreaming? (+mandatory "idrk" disclaimer xD) |
Default should probably be full.
Some submodules have internal submodules, which might need to be initialized as well. I think the speed improvement will be noticeable once all the optional blocks are not initialized. Another motivation for this is to lower the barrier-to-entry for adding new generators to chipyard. We can add new projects without having to rationalize that the usefulness outweighs the additional bloat.
Yes, I haven't gone through the process of adding the flags to build-setup. I've only added them to init-submodules for now. Feedback here on the right user interface would be appreciated.
Yes, the documentation for adding new generators, and the process for doing so, should be simplified. |
|
I like CY-lite. It would be ideal if we can flatten stuff like testchipip, rocketchip, inclusive l2 during this process, but I know it is a lot of work... |
|
Flattening RC and RC-LLC are non-starters (and I don't think this buys anything). |
740c9ea to
df2e679
Compare
abejgonzalez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Is it better to have the default be full Chipyard or lean Chipyard? The default for conda build right now is full, with lean being a flag, so it feels like a potential confusion to have Chipyard be the opposite.
Default should probably be full.
- So might be worth exploring which submodules should be part of a minimal build. Are there enough of them unused to have a speedup/size reduction?
Some submodules have internal submodules, which might need to be initialized as well. I think the speed improvement will be noticeable once all the optional blocks are not initialized.
Another motivation for this is to lower the barrier-to-entry for adding new generators to chipyard. We can add new projects without having to rationalize that the usefulness outweighs the additional bloat.
- Maybe there could be a separate --lean flag which allows a more stripped down baseline, the --full flag which adds everything everything in, and the default is some in between which hopefully covers most existing use cases.
Assuming some people clone "minimal" Chipyard, then the default flow would be to run the init-submod...* script to add those submodules back in? IMO a minimal Chipyard makes a lot of sense as the default, then we tell users to use the said script to re-build it up to what they want (most initial users probably only care about the core submodules - Rocket + L2 + testchipip + etc - for the most part)
Yes, I haven't gone through the process of adding the flags to build-setup. I've only added them to init-submodules for now. Feedback here on the right user interface would be appreciated.
Spitballing the minimal setup, build-setup is defaulting to minimal Chipyard. Then we move the init-submodules to the top-level of the repo and clean up it's API/name to make it clear this is how to add/remove packages (i.e. submodules). Then users after build-setup can modify the packages using that script.
The other thought is, how to support people integrating new generators according to the rules, now that the rule also includes editing the shell script and a slightly different sbt process? Since all the effort to refactor things just collapses if people add their new submodules to default build anyway. Or maybe that doesn't matter as long proper integration is enforced when upstreaming?
Yes, the documentation for adding new generators, and the process for doing so, should be simplified.
I personally am not a fan of the symlinking but I don't know of a better way to do this right now.
62ebb39 to
d96d34c
Compare
80cacdc to
a5f45ff
Compare
2820cbf to
d89548d
Compare
|
Can we settle on 1 unique discovery mechanism? Either .git or the chipyard folder? |
iansseijelly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
iansseijelly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There are a lot of submodules in generators/. Currently they are all cloned, initialized, and compiled, even though most people only use a small subset of these at a time.
This PR demonstrates a path towards a "modular generator submodule" approach, here some submodules can be left uninitialized, with the consequence being that configs which depend on those submodules will not appear on the classpath, and will not be available to the user.
This PR demonstrates this with some submodules, but the approach should scale to any submodule that does not have a crazy dependency structure. Currently modularized submods are:
The changes are:
Related PRs / Issues:
Type of change:
Impact:
Contributor Checklist:
mainas the base branch?changelog:<topic>label?changelog:label?.conda-lock.ymlfile if you updated the conda requirements file?Please Backport?CI Help:
Add the following labels to modify the CI for a set of features.
Generally, a label added only affect subsequent changes to the PR (i.e. new commits, force pushing, closing/reopening).
See
ci:*for full list of labels:ci:fpga-deploy- Run FPGA-based E2E testingci:local-fpga-buildbitstream-deploy- Build local FPGA bitstreams for platforms that are releasedci:disable- Disable CI