Skip to content

Fix host synchronization#417

Merged
pxl-th merged 3 commits intomasterfrom
pxl-th/synchronize
Apr 27, 2023
Merged

Fix host synchronization#417
pxl-th merged 3 commits intomasterfrom
pxl-th/synchronize

Conversation

@pxl-th
Copy link
Member

@pxl-th pxl-th commented Apr 27, 2023

  • Fix typo in isempty for list of active kernels.
  • Move to the next kernel in the list only when done waiting on the previous kernel.
    Previously this may have led to a following sync issue:
- monitor pops kernel to wait on, moves to the next kernel and starts waiting on the pop'd kernel
- host calls AMDGPU.synchronize(), but since it is empty it returns immediately
- monitor finishes waiting on pop'd kernel
  • Remove scratch alloc limiter.

@pxl-th pxl-th added bug Something isn't working sync labels Apr 27, 2023
@@ -119,14 +119,16 @@ function ROCKernel(kernel #= ::HostKernel =#; localmem::Int=0)
group_segment_size = UInt32(group_segment_size + localmem)
private_segment_size = executable_symbol_kernel_private_segment_size(exec_symbol)
Copy link
Member

Choose a reason for hiding this comment

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

Is this advisory? E.g. does the kernel run correctly if it get's less than the requested memory?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not... If decreased to a too low value, kernel crashes.
I remember it was added baaack when we were dealing with HSA_STATUS_ERROR_OUT_OF_RESOURCES queue errors.
But since then there's been a lot of fixes regarding memory usage, so maybe worth removing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed. Everything works fine without it.

@pxl-th pxl-th force-pushed the pxl-th/synchronize branch from 1a25a9a to 1975276 Compare April 27, 2023 14:53
@pxl-th pxl-th merged commit 919025a into master Apr 27, 2023
@pxl-th pxl-th deleted the pxl-th/synchronize branch April 27, 2023 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants