Skip to content

Indirectly Device Accessible Iterator Trait and ADL Customization Point #2126

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 54 commits into from
May 5, 2025

Conversation

danhoeflinger
Copy link
Contributor

@danhoeflinger danhoeflinger commented Mar 12, 2025

Types are "Indirectly Device Accessible" when they can represent data which can inherently be dereferenced in a SYCL kernel. Marking types as such can help oneDPL avoid unnecessary data movement surrounding sycl kernels.

Provides a default implementation for the Argument-Dependant Lookup (ADL) customization point is_onedpl_indirectly_device_accessible, as well as defined ADL overloads for oneDPL's provided iterators to provide rules for when they should be considered "indirectly device accessible".

Provides a public trait, is_indirectly_device_accessible[_v] which can be used to query whether a type is "indirectly device accessible". This can be used in combination with sycl::is_device_copyable and a check that it is a random access iterator to determine if it can be passed directly into sycl kernels without extra logic to copy the data to the device.


PR implementing the proposed design in #1999, and the current state of specification PR uxlfoundation/oneAPI-spec#620.

Will not merge until specification has been merged.

@danhoeflinger danhoeflinger marked this pull request as draft March 12, 2025 17:13
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/passed_directly_trait branch from ccc2347 to b159f2e Compare March 21, 2025 19:23
@danhoeflinger danhoeflinger changed the title [Draft] Passed directly trait Passed Directly Trait and ADL Customization Point Apr 4, 2025
@danhoeflinger danhoeflinger marked this pull request as ready for review April 4, 2025 16:32
@danhoeflinger danhoeflinger changed the title Passed Directly Trait and ADL Customization Point Device Accessible Content Iterator Trait and ADL Customization Point Apr 4, 2025
@danhoeflinger danhoeflinger added this to the 2022.9.0 milestone Apr 7, 2025
@danhoeflinger danhoeflinger changed the title Device Accessible Content Iterator Trait and ADL Customization Point Indirectly Device Accessible Iterator Trait and ADL Customization Point Apr 17, 2025
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/passed_directly_trait branch from d07a3a2 to 2bf81cc Compare April 17, 2025 16:07
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/passed_directly_trait branch from 0da03d0 to ef96683 Compare May 5, 2025 12:57
Signed-off-by: Dan Hoeflinger <[email protected]>
@@ -398,6 +450,10 @@ class zip_iterator
return !(*this < __it);
}

friend auto
is_onedpl_indirectly_device_accessible_iterator(const zip_iterator&)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a couple of questions.
Do we have some risk that we use not fully qualified name for zip_iterator?
Do we have some risk that something zip_iterator value (with qualificators) doesn't
match to const zip_iterator&?

Copy link
Contributor Author

@danhoeflinger danhoeflinger May 5, 2025

Choose a reason for hiding this comment

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

Since this is a hidden friend, we are using the current exact instantiated type of zip_iterator here.

As for const zip_iterator&, I suppose that we should just use zip_iterator. Copies don't matter because this is only used at compile time to determine output type. I've adjusted all of these ADL overloads to just use the type itself without reference.

Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
MikeDvorskiy
MikeDvorskiy previously approved these changes May 5, 2025
Copy link
Contributor

@MikeDvorskiy MikeDvorskiy left a comment

Choose a reason for hiding this comment

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

LGTM

mmichel11
mmichel11 previously approved these changes May 5, 2025
Copy link
Contributor

@mmichel11 mmichel11 left a comment

Choose a reason for hiding this comment

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

I have reviewed the commits since my last review. Everything LGTM.

@danhoeflinger danhoeflinger dismissed stale reviews from mmichel11 and MikeDvorskiy via 2528954 May 5, 2025 20:37
Copy link
Contributor

@mmichel11 mmichel11 left a comment

Choose a reason for hiding this comment

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

I reviewed the test change and it LGTM.

@danhoeflinger danhoeflinger merged commit 48717e3 into main May 5, 2025
18 of 19 checks passed
@danhoeflinger danhoeflinger deleted the dev/dhoeflin/passed_directly_trait branch May 5, 2025 21:09
timmiesmith pushed a commit that referenced this pull request Jun 9, 2025
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.

5 participants