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 hook into Chipyard build system for FireSim #1935

Merged
merged 6 commits into from
Aug 7, 2024
Merged

Conversation

abejgonzalez
Copy link
Contributor

Sister PR: firesim/firesim#1759

Related PRs / Issues:

Type of change:

  • Bug fix
  • New feature
  • Other enhancement

Impact:

  • RTL change
  • Software change (RISC-V software)
  • Build system change
  • Other

Contributor Checklist:

  • Did you set main as the base branch?
  • Is this PR's title suitable for inclusion in the changelog and have you added a changelog:<topic> label?
  • Did you state the type-of-change/impact?
  • Did you delete any extraneous prints/debugging code?
  • Did you mark the PR with a changelog: label?
  • (If applicable) Did you update the conda .conda-lock.yml file if you updated the conda requirements file?
  • (If applicable) Did you add documentation for the feature?
  • (If applicable) Did you add a test demonstrating the PR?
  • (If applicable) Did you mark the PR as Please Backport?

@abejgonzalez abejgonzalez requested a review from jerryz123 August 1, 2024 22:36
@abejgonzalez
Copy link
Contributor Author

This works with CY-as-top. Since the environments are separate, if you want to run FireSim commands you need to source sourceme-manager.sh. This invalidates the conda environment of Chipyard (it's cleaner to have them be separate). So if you need to re-build anything related to the target (binaries, CY-RTL-sim, etc) you need to re-source Chipyards env.sh.

Opening this up for reivew/thoughts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is clearly minimal. I think it makes sense to have some sort of "staging" area for FireSim-related scripts + collateral s.t. in the future if Chipyard has it's own config_* files, etc it can put them here?

Copy link
Contributor Author

@abejgonzalez abejgonzalez Aug 5, 2024

Choose a reason for hiding this comment

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

Maybe this should be some simulator agnostic folder (sims/none) where you can generate RTL w/o hooks for a specific simulator + w/o being in a simulator specific directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of this "staging directory" for firesim.

I also think a "verilog-only" staging directory could be done, but IMO that should be a separate set of changes.
Are you sure there won't have to be any firesim-specific things in this staging directory? As firesim is split out more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes there will be more FireSim-specific things as it's split out but IDK what they are right now. Having it be a separate staging folder is fine for now.

@jerryz123
Copy link
Contributor

This works with CY-as-top. Since the environments are separate, if you want to run FireSim commands you need to source sourceme-manager.sh. This invalidates the conda environment of Chipyard (it's cleaner to have them be separate). So if you need to re-build anything related to the target (binaries, CY-RTL-sim, etc) you need to re-source Chipyards env.sh.

This seems quite cumbersome. If this is the case, then we should leave the firesim-related packages in the CY conda environment. I don't think we can expect users to be adept at managing/switching between multiple environments.

@abejgonzalez
Copy link
Contributor Author

This works with CY-as-top. Since the environments are separate, if you want to run FireSim commands you need to source sourceme-manager.sh. This invalidates the conda environment of Chipyard (it's cleaner to have them be separate). So if you need to re-build anything related to the target (binaries, CY-RTL-sim, etc) you need to re-source Chipyards env.sh.

This seems quite cumbersome. If this is the case, then we should leave the firesim-related packages in the CY conda environment. I don't think we can expect users to be adept at managing/switching between multiple environments.

If we want to have packages from both environments exist in one source of env.sh, I think the cleaner solution is to overlay the environments (conda supports nested environments). This allows each repo to update conda packages separately w/o having a giant single environment (at the cost of having 2 nested environments - 1 for Chipyard, 1 for FireSim).

With that being said, I think it's alright to have two separate env's that need to be sourced separately. Maybe polling Chipyard/FireSim users on whats convenient to them would be good. I suspect a lot of users/devs have separate terminals, tmux windows, etc to build things w/ FireSim v.s. Chipyard (esp. since FireSim builds/runs are typically longer lived).

@abejgonzalez
Copy link
Contributor Author

This works with CY-as-top. Since the environments are separate, if you want to run FireSim commands you need to source sourceme-manager.sh. This invalidates the conda environment of Chipyard (it's cleaner to have them be separate). So if you need to re-build anything related to the target (binaries, CY-RTL-sim, etc) you need to re-source Chipyards env.sh.

This seems quite cumbersome. If this is the case, then we should leave the firesim-related packages in the CY conda environment. I don't think we can expect users to be adept at managing/switching between multiple environments.

If we want to have packages from both environments exist in one source of env.sh, I think the cleaner solution is to overlay the environments (conda supports nested environments). This allows each repo to update conda packages separately w/o having a giant single environment (at the cost of having 2 nested environments - 1 for Chipyard, 1 for FireSim).

With that being said, I think it's alright to have two separate env's that need to be sourced separately. Maybe polling Chipyard/FireSim users on whats convenient to them would be good. I suspect a lot of users/devs have separate terminals, tmux windows, etc to build things w/ FireSim v.s. Chipyard (esp. since FireSim builds/runs are typically longer lived).

Ok looks like people like the combined environment. I think the best way to do this is to have a stacked environment. I'll try it out and update this PR.

Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

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

The non-conda things look good. Happy to merge this once the Conda is fixed as a WIP towards independent firesim

@abejgonzalez
Copy link
Contributor Author

This works with CY-as-top. Since the environments are separate, if you want to run FireSim commands you need to source sourceme-manager.sh. This invalidates the conda environment of Chipyard (it's cleaner to have them be separate). So if you need to re-build anything related to the target (binaries, CY-RTL-sim, etc) you need to re-source Chipyards env.sh.

This seems quite cumbersome. If this is the case, then we should leave the firesim-related packages in the CY conda environment. I don't think we can expect users to be adept at managing/switching between multiple environments.

If we want to have packages from both environments exist in one source of env.sh, I think the cleaner solution is to overlay the environments (conda supports nested environments). This allows each repo to update conda packages separately w/o having a giant single environment (at the cost of having 2 nested environments - 1 for Chipyard, 1 for FireSim).
With that being said, I think it's alright to have two separate env's that need to be sourced separately. Maybe polling Chipyard/FireSim users on whats convenient to them would be good. I suspect a lot of users/devs have separate terminals, tmux windows, etc to build things w/ FireSim v.s. Chipyard (esp. since FireSim builds/runs are typically longer lived).

Ok looks like people like the combined environment. I think the best way to do this is to have a stacked environment. I'll try it out and update this PR.

Update on this. A stacked environment doesn't work for us since things like Python don't support stacking (stacking is for commandline programs - i.e. appending to PATH).

@jerryz123
Copy link
Contributor

I think we ought to continue with the single environment then

@abejgonzalez abejgonzalez changed the title WIP: Add hook into Chipyard build system for FireSim Add hook into Chipyard build system for FireSim Aug 7, 2024
@abejgonzalez abejgonzalez self-assigned this Aug 7, 2024
@abejgonzalez abejgonzalez merged commit 5016bf9 into main Aug 7, 2024
56 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants