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

Replace snitch_icache with cluster_icache dependency #112

Merged
merged 3 commits into from
Mar 18, 2024

Conversation

micprog
Copy link
Member

@micprog micprog commented Mar 12, 2024

In order to keep a single common source for IPs, the snitch_cluster instruction cache was extracted into its own repository. This repository was updated with features from Mempool, allowing reuse of common features, improvements, and bugfixes. This PR updates the icache to point to this new common source through a bender dependency.

@micprog micprog marked this pull request as ready for review March 12, 2024 13:20
@colluca
Copy link
Collaborator

colluca commented Mar 12, 2024

Thanks @micprog!

LGTM, only thing I would change concerns the CI fix. Since pulp-platform/common_cells#217 was merged to master already, I would temporarily use that hash in the Bender.yml, so we can then keep the common_cells assertions on with vsim. Later we can bump to v1.33.1. Alternatively, we can wait for the next release altogether (should not be delayed for long anyways).

Let me know what you think, I can implement the remaining changes and merge.

@micprog
Copy link
Member Author

micprog commented Mar 15, 2024

I think waiting shortly for the common_cells release would make sense to keep consistency and maximum testability. Separating the CI fixes from this PR would also be valuable as a separation of concerns.

@colluca colluca merged commit 5eb17af into main Mar 18, 2024
27 checks passed
@colluca colluca deleted the michaero/icache_dep branch March 18, 2024 22:23
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.

None yet

4 participants