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

Add HDP flush for gfx940 #1434

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

Add HDP flush for gfx940 #1434

wants to merge 9 commits into from
+33 −12

Conversation

wenkaidu
Copy link
Collaborator

@wenkaidu wenkaidu commented Nov 26, 2024 •
edited
Loading

Details

Do not mention proprietary info or link to internal work items in this PR.

Work item: "Internal", or link to GitHub issue (if applicable).
Internal

What were the changes?
Add HDP flush for gfx940

Why were the changes made?
Avoid potential data corruption

How was the outcome achieved?
Add HDP flush for gfx940

Additional Documentation:

Approval Checklist

Do not approve until these items are satisfied.

  • Verify the CHANGELOG has been updated, if
    • there are any NCCL API version changes,
    • any changes impact library users, and/or
    • any changes impact any other ROCm library.

@wenkaidu wenkaidu marked this pull request as ready for review November 27, 2024 17:44
thananon
thananon approved these changes Nov 27, 2024
View reviewed changes
Copy link
Contributor

@thananon thananon left a comment

Choose a reason for hiding this comment

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

If this is what I think it is doing, then it looks good to me.

General question, why was it done the other way before this PR?

@wenkaidu
Copy link
Collaborator Author

If this is what I think it is doing, then it looks good to me.

General question, why was it done the other way before this PR?

It was all about saving a micro-second when possible. I need to convert it back to draft as testing is still ongoing

@wenkaidu wenkaidu marked this pull request as draft November 27, 2024 21:41
@wenkaidu wenkaidu force-pushed the threadfence branch 2 times, most recently from 447ab5c to 40f7bd1 Compare December 17, 2024 17:40
@wenkaidu wenkaidu changed the title Add threadfence for gfx940 Add HDP flush for gfx940 Dec 19, 2024
@wenkaidu wenkaidu marked this pull request as ready for review December 20, 2024 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thananon thananon thananon approved these changes

@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

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

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

Successfully merging this pull request may close these issues.
2 participants
@wenkaidu @thananon