Skip to content

Feature: add collective operation to snitch #250

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

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

Lura518
Copy link

@Lura518 Lura518 commented Jun 16, 2025

This PR adds collective operation to the snitch cluster.

On the narrow interface we add a seperate CSR register which allows to set the user field of the narrow axi transmission.
The reduction/multicast operation is encoded in the axi user field of the narrow transmission and is executed outside of the cluster.

On the wide interface an special opcode for the iDMA was added. This opscode allows to set the user field in the iDMA directly.

@Lore0599 Lore0599 self-requested a review June 16, 2025 14:11
@Lura518 Lura518 force-pushed the feature/narrow_reduction branch from f4d5be4 to fa1a967 Compare June 18, 2025 14:03
@Lura518 Lura518 changed the title Feature: reduction capabilites on the narrow port Feature: add collective operation to snitch Jun 18, 2025
@Lura518
Copy link
Author

Lura518 commented Jun 18, 2025

TODO:

  • Currently the encoding of multicast / reduction etc is not defined consistently. This should be changed before we merge
  • We could make both CSR register more general like the iDMA > Set User High and Set User Low

@Lura518 Lura518 force-pushed the feature/narrow_reduction branch from fa1a967 to 93cb306 Compare June 18, 2025 14:12
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.

Thanks for the contribution. This is just an initial review :)

@Lura518 Lura518 force-pushed the feature/narrow_reduction branch from 5c564e6 to 87ac36f Compare July 4, 2025 07:38
@colluca
Copy link
Collaborator

colluca commented Jul 4, 2025

@Lura518 can you rebase on the latest develop branch? There seem to be some conflicts, I think we already talked about the changes I made upstream.

@Lura518 Lura518 force-pushed the feature/narrow_reduction branch from 87ac36f to 5a7f22a Compare July 4, 2025 12:20
Copy link
Collaborator

@colluca colluca left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

I have quite some minor comments, regarding primarily the style guide and good practices. Let's address these, I can also take these over if you want.

Then let's have a call to discuss Lorenzo's comment on the XBAR port ordering. Perhaps we can also clean up the XBAR instantiations, if we make axi_mcast_xbar a proper superset of axi_xbar. I would have time to work on this next week, would be good to merge it in the same PR.

@Lura518 Lura518 force-pushed the feature/narrow_reduction branch from 5a7f22a to 4825c62 Compare July 4, 2025 13:49
Raphael added 6 commits July 9, 2025 14:08
* Extend snitch by adding CSR register for the user field

* All collective operation will be forwarded to the (default-) SoC Port for further processing.
  Even if the crossbar is defined as mcast it is currently not being used as such.
  The problem are reduction operation as the crossbar does not support such operation and therefor
  all collectiv ops are forwarded to the SoC port (to be processed in the router there)

* Bump bender dependencies

(Cherry-Picked from 6402d71 of Lorenzos Fork)
* New configuration for the narrow reduction

* Rename existing configuration options

* Integrate multicast/reduction into the global barrier functions

(Cherry-Picked from c9ed65c of Lorenzos Fork)
* Extend the axi xbar to offload collectiv operation to the SoC

* Add new iDMA Opscode

(Cherry-Picked from 0226903 of Lorenzos Fork)
* Extend the dma driver to support collectiv operation by setting the user field in the axi transmission

(Cherry-Picked from 3af3efa of Lorenzos Fork)
@Lura518 Lura518 force-pushed the feature/narrow_reduction branch from 4825c62 to a7bc2ec Compare July 9, 2025 12:23
Copy link
Collaborator

@colluca colluca left a comment

Choose a reason for hiding this comment

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

Apart from a few questions, all the TODOs are a note to self, I can implement these now.

* Bump axi bnder version due to renaming on axi side

* Rename rest of collectiv to collective

* multicast rule / port fix
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.

3 participants