Skip to content

treewide: Bump and improve snitch_cluster integration #29

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

Merged
merged 1 commit into from
May 9, 2025
Merged

Conversation

colluca
Copy link
Contributor

@colluca colluca commented May 6, 2025

Updates the Snitch cluster to the latest version on main, in preparation of multicast integration.

In summary:

  • Bumps the cluster I$, as used by latest Snitch
  • Transitions to JSON5 Snitch configuration
  • Update LLVM toolchain to latest release
  • Picobello-specific cluster wrapper and package are promoted to Picobello's Bender.yml, and generated in same folder as other files
  • Bender files are encoded as prerequisites of compile.tcl
  • Symlink snRuntime sources that do not change from the out-of-context cluster runtime (requires workspace entry in Bender.yml)
  • Add recently added riscv-tests in Snitch

@colluca colluca requested a review from Copilot May 6, 2025 12:15
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates and improves the Snitch cluster integration in preparation for multicast integration. Key changes include updating the Snitch cluster dependency to a new repository, transitioning to a JSON5 configuration, adding new targets for the Snitch cluster files, and updating the LLVM toolchain version.

Files not reviewed (4)
  • Makefile: Language not supported
  • cfg/snitch_cluster.json: Language not supported
  • iis-env.sh: Language not supported
  • target/sim/vsim/vsim.mk: Language not supported

Copy link
Contributor

@Lore0599 Lore0599 left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Do we want to wait for the Snitch PR#227 to be merged before pointing to the upstreamed version?

@colluca
Copy link
Contributor Author

colluca commented May 6, 2025

Overall LGTM. Do we want to wait for the Snitch PR#227 to be merged before pointing to the upstreamed version?

If there's any other development ongoing I would suggest merging already, so people can already work on top of this and we don't have to resolve the conflicts later.

In that case, we can open a quick issue to remind us to bump the Snitch dependency.

@Lore0599
Copy link
Contributor

Lore0599 commented May 6, 2025

Yes let's merge it and open the issue

@colluca colluca force-pushed the bump-snitch branch 8 times, most recently from fa19855 to 2f6f676 Compare May 8, 2025 17:33
@colluca colluca marked this pull request as ready for review May 8, 2025 17:35
@Lore0599 Lore0599 self-requested a review May 9, 2025 15:01
@Lore0599 Lore0599 dismissed their stale review May 9, 2025 15:09

All changes have been addressed

Copy link
Contributor

@Lore0599 Lore0599 left a comment

Choose a reason for hiding this comment

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

LGTM

@colluca colluca enabled auto-merge (squash) May 9, 2025 15:11
@colluca colluca merged commit cfab270 into main May 9, 2025
5 checks passed
@colluca colluca deleted the bump-snitch branch May 9, 2025 15:57
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.

2 participants