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.

Sign up for GitHub

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

Jump to bottom

Enable firesim to use conda #1140

Merged
merged 10 commits into from
Apr 13, 2022
Merged

Enable firesim to use conda #1140

merged 10 commits into from
Apr 13, 2022

Conversation

timsnyder-siv
Copy link
Contributor

@timsnyder-siv timsnyder-siv commented Mar 11, 2022 •
edited
Loading

Related PRs or Issues:
firesim/firesim#986

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?
  • Did you delete any extraneous prints/debugging code?
  • (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"?

Release Notes

Slightly modify build_toolchains.sh to enable it to build using the conda tools.

@timsnyder-siv timsnyder-siv mentioned this pull request Mar 11, 2022
Merged
12 tasks
abejgonzalez
abejgonzalez reviewed Mar 11, 2022
View reviewed changes
Copy link
Contributor

@abejgonzalez abejgonzalez left a comment

Choose a reason for hiding this comment

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

This seems fine to me but CI is failing since you aren't on a more recent version of Chipyard. Could you rebase this branch on main to properly run the CI.

scripts/build-toolchains.sh Show resolved Hide resolved
@timsnyder
log commands in build_utils.sh
871dc29
@timsnyder
explicitly disable boost for spike build
c223d09 
because riscv-software-src/riscv-isa-sim#834 and we don't need the debug socket
@timsnyder
tweak build-toolchains.sh to work with conda env
9aee0ab 
* more env vars need to be cleared when using the RISCV cross compiler
* use LD_GOLD to link QEMU because QEMU's LDFLAGS_NOPIC uses incorrect format for the version
  of bfd ld
@timsnyder
add comment about spike autoconf issue w/ boost
7783e6e
@timsnyder-siv
Copy link
Contributor Author

@abejgonzalez prepare-chipyard-cores and prepare-chipyard-peripherals look like they are failing because the commits pointed to by rocket-chip and hardfloat don't seem to be fetchable?

@abejgonzalez
Copy link
Contributor

@abejgonzalez prepare-chipyard-cores and prepare-chipyard-peripherals look like they are failing because the commits pointed to by rocket-chip and hardfloat don't seem to be fetchable?

This is an issue with our recent change to switch to shallow clones... frankly this should probably be reverted or bypassable. I don't have time to look into this issue until later in the week.

davidbiancolin
davidbiancolin reviewed Mar 18, 2022
View reviewed changes
Copy link
Contributor

@davidbiancolin davidbiancolin left a comment

Choose a reason for hiding this comment

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

These changes seem reasonable to me.

@timsnyder-siv
Copy link
Contributor Author

@abejgonzalez can I get an approval to merge this?

abejgonzalez
abejgonzalez approved these changes Apr 12, 2022
View reviewed changes
Copy link
Contributor

@abejgonzalez abejgonzalez left a comment

Choose a reason for hiding this comment

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

LGTM

@abejgonzalez abejgonzalez self-requested a review April 12, 2022 18:05
abejgonzalez
abejgonzalez requested changes Apr 12, 2022
View reviewed changes
Copy link
Contributor

@abejgonzalez abejgonzalez left a comment

Choose a reason for hiding this comment

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

Can you re-build the toolchain in CI just to confirm that things work? Once that is done I'm fine with merging.

chipyard/.github/workflows/chipyard-run-tests.yml

Line 10 in 46d9029

tools-cache-version: v13

abejgonzalez
abejgonzalez approved these changes Apr 13, 2022
View reviewed changes
Copy link
Contributor

@abejgonzalez abejgonzalez left a comment

Choose a reason for hiding this comment

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

LGTM

@timsnyder-siv
Copy link
Contributor Author

@abejgonzalez , can you click the checkmark now that I've invalidated the tool cache? I'd like to get this merged while there aren't any conflicts.

@abejgonzalez
Copy link
Contributor

I LGTM'ed. I'm happy to merge this if you want

@timsnyder-siv
Copy link
Contributor Author

@abejgonzalez please merge. I didn't realize I wouldn't be able to once it was approved and passing.

@abejgonzalez abejgonzalez merged commit c063b22 into main Apr 13, 2022
@timsnyder-siv timsnyder-siv mentioned this pull request May 19, 2022
Merged
15 tasks
@jerryz123 jerryz123 deleted the use_conda branch October 1, 2022 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbiancolin davidbiancolin davidbiancolin left review comments

@abejgonzalez abejgonzalez abejgonzalez approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@timsnyder-siv @abejgonzalez @davidbiancolin @timsnyder