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

Multicluster MemPool #115

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Multicluster MemPool #115

wants to merge 14 commits into from

Conversation

SamuelRiedel
Copy link
Member

This PR extends MemPool to support multiple clusters. It mainly addresses the hardware side and the basic corresponding software changes. Updating the kernels to multi-cluster kernels will be done in a second step.

Depends on #113 and #114

Changelog

Added

  • Add support for a multi-cluster MemPool

Checklist

  • Automated tests pass
  • Changelog updated
  • Code style guideline is observed

@SamuelRiedel SamuelRiedel marked this pull request as ready for review August 28, 2024 22:27
Copy link
Collaborator

@mbertuletti mbertuletti left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. I added some comments and requests.


if (NumTiles < NumGroups)
if (NumTilesPerCluster < NumGroupsPerCluster)
Copy link
Collaborator

Choose a reason for hiding this comment

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

A condition should be also added on the number of SubGroups. NumTilesPerGroup > NumSubGroups.

for (genvar g = 0; g < NumGroups; g++) begin: gen_wfi_groups
for (genvar sg = 0; sg < NumSubGroupsPerGroup; sg++) begin: gen_wfi_sub_groups
for (genvar t = 0; t < NumTilesPerSubGroup; t++) begin: gen_wfi_tiles
for (genvar cl = 0; cl < NumClusters; cl++) begin: gen_wfi_clusters
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we represent the wfi as a multi-dimensional packed array at least on the cluster dimension? This would make the code more readable. This comment is also valid for follwoing signals indexed over the cluster dimension.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It could be useful to add a barrier function for the synchronization of a single cluster. Are clusters bounded to work all in parallel on the same task at the moment?

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