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

Fix synchronization in allreduce8Read kernel #1457

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fix synchronization in allreduce8Read kernel #1457

wants to merge 1 commit into from
+21 −33

Conversation

dsidler
Copy link

@dsidler dsidler commented Dec 10, 2024

Running kernel allreduce8Read across 64 vGPUs (in CPX mode) revealed synchronization bugs. The PR addresses them by:

  • Synchronize threads before signaling that output (outChannels) are valid to guarantee ordering between all data writes in the block with corresponding signals.

Following changes are not affecting correctness:

  • Don't synchronize outChannels every iteration.
  • Synchronize input channels only once at the beginning of the kernel execution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wenkaidu wenkaidu Awaiting requested review from wenkaidu wenkaidu is a code owner

@gilbertlee-amd gilbertlee-amd Awaiting requested review from gilbertlee-amd gilbertlee-amd is a code owner

@akolliasAMD akolliasAMD Awaiting requested review from akolliasAMD akolliasAMD is a code owner

@edgargabriel edgargabriel Awaiting requested review from edgargabriel edgargabriel is a code owner

@PedramAlizadeh PedramAlizadeh Awaiting requested review from PedramAlizadeh PedramAlizadeh is a code owner

@nusislam nusislam Awaiting requested review from nusislam nusislam is a code owner

@nileshnegi nileshnegi Awaiting requested review from nileshnegi nileshnegi is a code owner

@KawtharShafie KawtharShafie Awaiting requested review from KawtharShafie KawtharShafie is a code owner

@AtlantaPepsi AtlantaPepsi Awaiting requested review from AtlantaPepsi AtlantaPepsi is a code owner

@mberenjk mberenjk Awaiting requested review from mberenjk mberenjk is a code owner

@corey-derochie-amd corey-derochie-amd Awaiting requested review from corey-derochie-amd corey-derochie-amd is a code owner

@mustafabar mustafabar Awaiting requested review from mustafabar mustafabar is a code owner

@thananon thananon Awaiting requested review from thananon thananon is a code owner

@haripriya-amd haripriya-amd Awaiting requested review from haripriya-amd haripriya-amd is a code owner

At least 1 approving review is required to merge this pull request.

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

Successfully merging this pull request may close these issues.
1 participant
@dsidler