-
Notifications
You must be signed in to change notification settings - Fork 528
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
Tweak synchronous memcpy implementation #1809
Conversation
use launch_fence instead of barrier_bit
…into feature_memcpy_the_2nd
…into feature_memcpy_the_2nd
…into feature_memcpy_the_2nd
…into feature_memcpy_the_2nd
FYI jenkins build 15 failed and 17 failed, but build 16 succeeded. I believe this PR is actually passing CI now if we could try kicking off another try. |
Done. build 18 is in progress now. |
@gargrahul did something change recently? I'm seeing this error in the CI logs: |
src/hip_memory.cpp
Outdated
if (da.handle == 0 || sa.handle == 0) { // Handle degenerate case. | ||
throwing_result_check(hsa_memory_copy(dst, src, n), | ||
__FILE__, __func__, __LINE__); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we still need this change as we have observed hangs in hsa_memory_copy().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignore the comment missed your latest commit :)
Lets merge this in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets merge this.
…loper-Tools/HIP into feature_memcpy_the_2nd
…into feature_memcpy_the_2nd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved based on test build report 736
The existing one can have issues on certain systems, therefore this limits use of direct memcpy via largeBAR to sizes where it is unequivocally better.
Also addresses SWDEV-220030.