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.

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

[Feature]: add a hsa_amd_signal_wait_all API. #241

Open
benvanik opened this issue Sep 29, 2024 · 6 comments · May be fixed by #250
Open

[Feature]: add a hsa_amd_signal_wait_all API. #241

benvanik opened this issue Sep 29, 2024 · 6 comments · May be fixed by #250
Assignees

Comments

@benvanik
Copy link

Suggestion Description

hsa_amd_signal_wait_any exists and routes to hsaKmtWaitOnMultipleEvents_Ext and is useful for implementing host-side barrier-or packet behavior. There's currently not a hsa_amd_signal_wait_all, though, which is needed to efficiently implement the barrier-and packet behavior. hsaKmtWaitOnMultipleEvents_Ext has a WaitOnAll flag and it'd be useful to have a top-level API that routes to that.

Operating System

No response

GPU

No response

ROCm Component

No response

@atgutier
Copy link
Contributor

Will take a look at this.

@atgutier atgutier self-assigned this Oct 17, 2024
@atgutier atgutier linked a pull request Oct 17, 2024 that will close this issue
@atgutier
Copy link
Contributor

Can you test #250 or point me to any tests you may have for this when you get a chance?

@benvanik
Copy link
Author

(I don't have anything running yet but can try to make a test for this - thanks for implementing it :)

@benvanik
Copy link
Author

I haven't tested it yet but one quirk of hsa_amd_signal_wait_any is that it does not allow 0/NULL signal handles. This is inconsistent with the AQL hsa_barrier_or_packet_t that allows any signal to have a 0/NULL value to have it be ignored (effectively). When implementing soft queues that support the barrier packets it'd be nice to be able to pass the dependency signals directly to the APIs without needing to filter them at the application level. The hsa_barrier_and_packet_t treats 0/NULL as having a 0 value (so effectively ignored). This may be worth doing as a separate thing so I'll file a new issue for it (I think your PR #250 is consistent with hsa_amd_signal_wait_any's behavior by checking the validity of signal handles).

@atgutier
Copy link
Contributor

I haven't tested it yet but one quirk of hsa_amd_signal_wait_any is that it does not allow 0/NULL signal handles. This is inconsistent with the AQL hsa_barrier_or_packet_t that allows any signal to have a 0/NULL value to have it be ignored (effectively). When implementing soft queues that support the barrier packets it'd be nice to be able to pass the dependency signals directly to the APIs without needing to filter them at the application level. The hsa_barrier_and_packet_t treats 0/NULL as having a 0 value (so effectively ignored). This may be worth doing as a separate thing so I'll file a new issue for it (I think your PR #250 is consistent with hsa_amd_signal_wait_any's behavior by checking the validity of signal handles).

Sounds good. I think that 0/NULL semantics matching the barrier should be doable.

@benvanik
Copy link
Author

Cool! Filed at #252!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants