Skip to content

[SYCL] Add nested calls detection to shortcut functions #13659

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

Merged
merged 15 commits into from
May 29, 2024

Conversation

KseniyaTikhomirova
Copy link
Contributor

@KseniyaTikhomirova KseniyaTikhomirova commented May 6, 2024

Original impl does not cover shortcut functions.
This version has thread_local global simple type variable that could track nested call within some queue functions like submit, memset, memcpy and others. Shortcut functions use common part submitMemOpHelper where detection is also added.
Reduction impl updated to eliminate nested call we did internally. It is even better since common logic with dependency tracking used in queue methods is not needed there.

Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
This reverts commit 57e0e33.
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
@KseniyaTikhomirova KseniyaTikhomirova marked this pull request as ready for review May 21, 2024 10:44
@KseniyaTikhomirova KseniyaTikhomirova requested a review from a team as a code owner May 21, 2024 10:44
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
@KseniyaTikhomirova
Copy link
Contributor Author

@uditagarwal97 kindly ping

Copy link
Contributor

@uditagarwal97 uditagarwal97 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
@KseniyaTikhomirova
Copy link
Contributor Author

hi @intel/llvm-gatekeepers, this PR is ready for merge, thanks

@aelovikov-intel aelovikov-intel merged commit fd0491c into intel:sycl May 29, 2024
14 checks passed
aelovikov-intel added a commit that referenced this pull request Jun 6, 2025
1) Move `reduction::withAuxHandler` -> `HandlerAccess::postProcess`, as
that's what it's really about.
2) Add comments describing what I see as issues with the
previous/current implementation plus minor fixes of what I could address
easily.
3) Added `HandlerAccess:preProcess` instead of using `addCounterInit`
introduced in
#13659. The original idea behind
`withAuxHandler` and reduction implementations in general is to decouple
them from SYCL RT internals as much as possible and `addCounterInit` was
a step in the exact opposite direction.

---------

Co-authored-by: Sergey Semenov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants