From 45248dda00e6bd36ec53891d7fabc06da62d33e1 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 13 Jan 2025 13:03:34 -0500 Subject: [PATCH 01/31] initial draft for customization point Signed-off-by: Dan Hoeflinger --- rfcs/proposed/passed_directly_API/README.md | 114 ++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 rfcs/proposed/passed_directly_API/README.md diff --git a/rfcs/proposed/passed_directly_API/README.md b/rfcs/proposed/passed_directly_API/README.md new file mode 100644 index 00000000000..04bbc26b9e9 --- /dev/null +++ b/rfcs/proposed/passed_directly_API/README.md @@ -0,0 +1,114 @@ +# Passed Directly Customiation Point for User Defined Types + +## Introduction + +OneDPL handles some types of input data automatically as input to its dpcpp (sycl-based) backend as described +[here](https://uxlfoundation.github.io/oneDPL/parallel_api/pass_data_algorithms.html). Unified Shared Memory (USM) +pointers refer to data which is device accessible inherently, so no processing is required to pass this type of input +data to SYCL kernels, we refer to this trait as "passed directly". OneDPL also defines some rules for its provided +[iterator types](https://uxlfoundation.github.io/oneDPL/parallel_api/iterators.html) to be passed directly to SYCL +under some circumstances (based usually on their base types). + +Internally, these rules are defined with a trait `oneapi::dpl::__ranges::is_passed_directly` which evaluates to +`std::true_type` or `std::false_type` to indicate whether the type `T` should be passed directly to sycl kernels. +There exists a unofficial legacy `is_passed_directly` trait which types can define like this: +`using is_passed_directly = std::true_type;` which is supported within oneDPL. This method is currently used for a +number of helper types within the SYCLomatic compatability headers, (`device_pointer`, `device_iterator`, +`tagged_pointer`, `constant_iterator`, `iterator_adaptor`). There is no official public API for users who want to +create their own types which could be passed directly to SYCL kernels, this is a gap we should fill in with an official +public API. + +Without something like this users are forced to only rely upon our provided types, or reach into implementation details +which are not part of oneDPL's specified interface. + +## Proposal + +Create a customization point `oneapi::dpl::is_passed_directly_to_sycl_kernels` free function which allows users to +define to mark their types as passed directly: + +``` +template +constexpr bool is_passed_directly_to_sycl_kernels(const T&); +``` + +oneDPL will provide a default implementation which will defer to the existing trait: + +``` +template +constexpr +bool +is_passed_directly_to_sycl_kernels(const T&) +{ + return oneapi::dpl::__ranges::is_passed_directly_v; +} +``` + +Users can use any constexpr logic based on their type to determine if the type can be passed directly into a SYCL kernel +without any processing. Below is an example of a type which contains a pair of iterators, and should be treated as +passed directly if and only if both base iterators are also passed directly. OneDPL will use this customization point +internally when determining how to handle incoming data, picking up any user customizations in the process. + +``` +namespace user +{ + template + struct iterator_pair + { + It1 first; + It2 second; + }; + + template + constexpr + bool + is_passed_directly_to_sycl_kernels(const iterator_pair& pair) + { + return oneapi::dpl::is_passed_directly_to_sycl_kernels(pair.first) && + oneapi::dpl::is_passed_directly_to_sycl_kernels(pair.second); + } +} //namespace user +``` + +This allows the user to provide rules for their types next to their implementation, without cluttering the +implementation of the type itself with extra typedefs, etc. + +This option can exist in concert with existing methods, the legacy `is_passed_directly` typedef in types, the internal +`oneapi::dpl::__ranges::is_passed_directly` trait specializations. It would be possible to simplify the internal +implementation away from explicit specializations of the trait to the customization point, but that is not required +at first implementation. + +## Alternatives considered +### Public trait struct explicit specialization +We could simply make public our internal structure `oneapi::dpl::__ranges::is_passed_directly` as +`oneapi::dpl::is_passed_directly` for users to specialize to define rules for their types. This would be a similar +mechanism to `sycl::is_device_copyable`. The implementation details of this option should avoid some complexity required +to properly implement the customization point. + +However, as we have learned from experience within oneDPL, explicit specialization of a structure in another library's +namespace makes for maintenance problems. It either requires lots of closing of nested namespaces, opening of the +external library's namespace for the specialization or it requires separating these specializations to a separate +location removed from the types they are specializing for. OneDPL has chosen to use the later, which can be seen in +`include/oneapi/dpl/pstl/hetero/dpcpp/sycl_traits.h`. This has made for several errors where changes to structures +should have included changes to sycl_traits, but did not, and needed to be fixed later. + +In an effort to avoid this same issue for our users, we propose a similar method but instead with a constexpr +customization point, allowing the user to override that customization point within their own namespace as a free +function. + +### Require specifically named typedef / using in user's type +We could simply make official our requirements for user's types to include a typedef or using statment to define if the +type is passed directly like `using is_passed_directly = std::true_type;`, where the absence of this would be equivalent +to a `std::false_type`. + +However, this clutters the user type definitions with specifics of oneDPL. It also may not be as clear what this +signifies for maintenance of user code without appropriate comments describing the details of oneDPL and SYCL. Users +have expressed that this is undesirable. + +### Testing +We will need a detailed test checking both positive and negative responses to `is_passed_directly_to_sycl_kernels` come +as expected, with custom types and combinations of iterators, usm pointers etc. + +## Open Questions + +Is there a better / more concise name than `is_passed_directly_to_sycl_kernels` we can use which properly conveys the +meaning to the users? \ No newline at end of file From 5ff56d95b8dacf2469031e5a5adda574fbfcd546 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 13 Jan 2025 13:07:15 -0500 Subject: [PATCH 02/31] extra question Signed-off-by: Dan Hoeflinger --- rfcs/proposed/passed_directly_API/README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rfcs/proposed/passed_directly_API/README.md b/rfcs/proposed/passed_directly_API/README.md index 04bbc26b9e9..7c5f54968e8 100644 --- a/rfcs/proposed/passed_directly_API/README.md +++ b/rfcs/proposed/passed_directly_API/README.md @@ -111,4 +111,7 @@ as expected, with custom types and combinations of iterators, usm pointers etc. ## Open Questions Is there a better / more concise name than `is_passed_directly_to_sycl_kernels` we can use which properly conveys the -meaning to the users? \ No newline at end of file +meaning to the users? + +Should we be targetting Experimental or fully supported with this proposal? + (Do we think user feedback is required to solidify an interface / experience?) \ No newline at end of file From 643a2cb98468904ee7cb59fb2e6a58edbcfb8926 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 13 Jan 2025 13:13:41 -0500 Subject: [PATCH 03/31] spelling Signed-off-by: Dan Hoeflinger --- rfcs/proposed/passed_directly_API/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/proposed/passed_directly_API/README.md b/rfcs/proposed/passed_directly_API/README.md index 7c5f54968e8..2fabab4058f 100644 --- a/rfcs/proposed/passed_directly_API/README.md +++ b/rfcs/proposed/passed_directly_API/README.md @@ -96,7 +96,7 @@ customization point, allowing the user to override that customization point with function. ### Require specifically named typedef / using in user's type -We could simply make official our requirements for user's types to include a typedef or using statment to define if the +We could simply make official our requirements for user's types to include a typedef or using statement to define if the type is passed directly like `using is_passed_directly = std::true_type;`, where the absence of this would be equivalent to a `std::false_type`. @@ -113,5 +113,5 @@ as expected, with custom types and combinations of iterators, usm pointers etc. Is there a better / more concise name than `is_passed_directly_to_sycl_kernels` we can use which properly conveys the meaning to the users? -Should we be targetting Experimental or fully supported with this proposal? +Should we be targeting Experimental or fully supported with this proposal? (Do we think user feedback is required to solidify an interface / experience?) \ No newline at end of file From 6c0fc33d8d24cc2351d5b6eb01ac5fdce307fa14 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 13 Jan 2025 13:24:27 -0500 Subject: [PATCH 04/31] brief comment on implementation details Signed-off-by: Dan Hoeflinger --- rfcs/proposed/passed_directly_API/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rfcs/proposed/passed_directly_API/README.md b/rfcs/proposed/passed_directly_API/README.md index 2fabab4058f..41bc94b2f8d 100644 --- a/rfcs/proposed/passed_directly_API/README.md +++ b/rfcs/proposed/passed_directly_API/README.md @@ -77,6 +77,12 @@ This option can exist in concert with existing methods, the legacy `is_passed_di implementation away from explicit specializations of the trait to the customization point, but that is not required at first implementation. +### Implementation details +To make this robust, we will follow an C++17 updated version of what is discussed in +[Eric Niebler's Post](https://ericniebler.com/2014/10/21/customization-point-design-in-c11-and-beyond/), using a +callable, and using an `inline constexpr` to avoid issues with ODR and to avoid issues with resolving customization +points when not separating the call to two steps with a `using` statement first. + ## Alternatives considered ### Public trait struct explicit specialization We could simply make public our internal structure `oneapi::dpl::__ranges::is_passed_directly` as From 3bacd14bc053328d16691d40297794da9d6fa7da Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 13 Jan 2025 13:28:26 -0500 Subject: [PATCH 05/31] Adding a simpler example Signed-off-by: Dan Hoeflinger --- rfcs/proposed/passed_directly_API/README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/rfcs/proposed/passed_directly_API/README.md b/rfcs/proposed/passed_directly_API/README.md index 41bc94b2f8d..51bf8a7af3d 100644 --- a/rfcs/proposed/passed_directly_API/README.md +++ b/rfcs/proposed/passed_directly_API/README.md @@ -43,6 +43,27 @@ is_passed_directly_to_sycl_kernels(const T&) } ``` +Below is a simple example of a type and customization point definition which is always passed directly. + +``` +namespace user +{ + + struct my_passed_directly_type + { + /* unspecified user definition */ + }; + + template + constexpr + bool + is_passed_directly_to_sycl_kernels(const my_passed_directly_type&) + { + return true; + } +} //namespace user +``` + Users can use any constexpr logic based on their type to determine if the type can be passed directly into a SYCL kernel without any processing. Below is an example of a type which contains a pair of iterators, and should be treated as passed directly if and only if both base iterators are also passed directly. OneDPL will use this customization point From f24f8d2872adcb05354dd979a6c2eca2d292d60b Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 13 Jan 2025 13:32:29 -0500 Subject: [PATCH 06/31] spelling Signed-off-by: Dan Hoeflinger --- rfcs/proposed/passed_directly_API/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/proposed/passed_directly_API/README.md b/rfcs/proposed/passed_directly_API/README.md index 51bf8a7af3d..f2ea12f30ba 100644 --- a/rfcs/proposed/passed_directly_API/README.md +++ b/rfcs/proposed/passed_directly_API/README.md @@ -13,7 +13,7 @@ Internally, these rules are defined with a trait `oneapi::dpl::__ranges::is_pass `std::true_type` or `std::false_type` to indicate whether the type `T` should be passed directly to sycl kernels. There exists a unofficial legacy `is_passed_directly` trait which types can define like this: `using is_passed_directly = std::true_type;` which is supported within oneDPL. This method is currently used for a -number of helper types within the SYCLomatic compatability headers, (`device_pointer`, `device_iterator`, +number of helper types within the SYCLomatic compatibility headers, (`device_pointer`, `device_iterator`, `tagged_pointer`, `constant_iterator`, `iterator_adaptor`). There is no official public API for users who want to create their own types which could be passed directly to SYCL kernels, this is a gap we should fill in with an official public API. From 1145efaab9598037db998ea194e15b36c7227ad2 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 13 Jan 2025 14:37:40 -0500 Subject: [PATCH 07/31] name update and capitalization Signed-off-by: Dan Hoeflinger --- rfcs/proposed/passed_directly_API/README.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/rfcs/proposed/passed_directly_API/README.md b/rfcs/proposed/passed_directly_API/README.md index f2ea12f30ba..f278921857d 100644 --- a/rfcs/proposed/passed_directly_API/README.md +++ b/rfcs/proposed/passed_directly_API/README.md @@ -23,12 +23,12 @@ which are not part of oneDPL's specified interface. ## Proposal -Create a customization point `oneapi::dpl::is_passed_directly_to_sycl_kernels` free function which allows users to +Create a customization point `oneapi::dpl::is_passed_directly_to_onedpl` free function which allows users to define to mark their types as passed directly: ``` template -constexpr bool is_passed_directly_to_sycl_kernels(const T&); +constexpr bool is_passed_directly_to_onedpl(const T&); ``` oneDPL will provide a default implementation which will defer to the existing trait: @@ -37,7 +37,7 @@ oneDPL will provide a default implementation which will defer to the existing tr template constexpr bool -is_passed_directly_to_sycl_kernels(const T&) +is_passed_directly_to_onedpl(const T&) { return oneapi::dpl::__ranges::is_passed_directly_v; } @@ -57,7 +57,7 @@ namespace user template constexpr bool - is_passed_directly_to_sycl_kernels(const my_passed_directly_type&) + is_passed_directly_to_onedpl(const my_passed_directly_type&) { return true; } @@ -82,10 +82,10 @@ namespace user template constexpr bool - is_passed_directly_to_sycl_kernels(const iterator_pair& pair) + is_passed_directly_to_onedpl(const iterator_pair& pair) { - return oneapi::dpl::is_passed_directly_to_sycl_kernels(pair.first) && - oneapi::dpl::is_passed_directly_to_sycl_kernels(pair.second); + return oneapi::dpl::is_passed_directly_to_onedpl(pair.first) && + oneapi::dpl::is_passed_directly_to_onedpl(pair.second); } } //namespace user ``` @@ -132,13 +132,13 @@ signifies for maintenance of user code without appropriate comments describing t have expressed that this is undesirable. ### Testing -We will need a detailed test checking both positive and negative responses to `is_passed_directly_to_sycl_kernels` come +We will need a detailed test checking both positive and negative responses to `is_passed_directly_to_onedpl` come as expected, with custom types and combinations of iterators, usm pointers etc. ## Open Questions -Is there a better / more concise name than `is_passed_directly_to_sycl_kernels` we can use which properly conveys the +Is there a better / more concise name than `is_passed_directly_to_onedpl` we can use which properly conveys the meaning to the users? -Should we be targeting Experimental or fully supported with this proposal? +Should we be targeting Experimental or fully Supported with this proposal? (Do we think user feedback is required to solidify an interface / experience?) \ No newline at end of file From d1d0718ad9ff5997f5de91d252bc903b30effdd1 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 14 Jan 2025 09:15:30 -0500 Subject: [PATCH 08/31] name adjustment Signed-off-by: Dan Hoeflinger --- rfcs/proposed/passed_directly_API/README.md | 25 +++++++++++---------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/rfcs/proposed/passed_directly_API/README.md b/rfcs/proposed/passed_directly_API/README.md index f278921857d..5ebc54591e0 100644 --- a/rfcs/proposed/passed_directly_API/README.md +++ b/rfcs/proposed/passed_directly_API/README.md @@ -23,12 +23,12 @@ which are not part of oneDPL's specified interface. ## Proposal -Create a customization point `oneapi::dpl::is_passed_directly_to_onedpl` free function which allows users to +Create a customization point `oneapi::dpl::is_passed_directly_in_onedpl_device_policies` free function which allows users to define to mark their types as passed directly: ``` template -constexpr bool is_passed_directly_to_onedpl(const T&); +constexpr bool is_passed_directly_in_onedpl_device_policies(const T&); ``` oneDPL will provide a default implementation which will defer to the existing trait: @@ -37,7 +37,7 @@ oneDPL will provide a default implementation which will defer to the existing tr template constexpr bool -is_passed_directly_to_onedpl(const T&) +is_passed_directly_in_onedpl_device_policies(const T&) { return oneapi::dpl::__ranges::is_passed_directly_v; } @@ -57,7 +57,7 @@ namespace user template constexpr bool - is_passed_directly_to_onedpl(const my_passed_directly_type&) + is_passed_directly_in_onedpl_device_policies(const my_passed_directly_type&) { return true; } @@ -82,10 +82,10 @@ namespace user template constexpr bool - is_passed_directly_to_onedpl(const iterator_pair& pair) + is_passed_directly_in_onedpl_device_policies(const iterator_pair& pair) { - return oneapi::dpl::is_passed_directly_to_onedpl(pair.first) && - oneapi::dpl::is_passed_directly_to_onedpl(pair.second); + return oneapi::dpl::is_passed_directly_in_onedpl_device_policies(pair.first) && + oneapi::dpl::is_passed_directly_in_onedpl_device_policies(pair.second); } } //namespace user ``` @@ -99,7 +99,7 @@ implementation away from explicit specializations of the trait to the customizat at first implementation. ### Implementation details -To make this robust, we will follow an C++17 updated version of what is discussed in +To make this robust, we will follow a C++17 updated version of what is discussed in [Eric Niebler's Post](https://ericniebler.com/2014/10/21/customization-point-design-in-c11-and-beyond/), using a callable, and using an `inline constexpr` to avoid issues with ODR and to avoid issues with resolving customization points when not separating the call to two steps with a `using` statement first. @@ -132,13 +132,14 @@ signifies for maintenance of user code without appropriate comments describing t have expressed that this is undesirable. ### Testing -We will need a detailed test checking both positive and negative responses to `is_passed_directly_to_onedpl` come -as expected, with custom types and combinations of iterators, usm pointers etc. +We will need a detailed test checking both positive and negative responses to +`is_passed_directly_in_onedpl_device_policies` have the expected result, with custom types and combinations of +iterators, usm pointers etc. ## Open Questions -Is there a better / more concise name than `is_passed_directly_to_onedpl` we can use which properly conveys the -meaning to the users? +Is there a better / more concise name than `is_passed_directly_in_onedpl_device_policies` we can use which properly +conveys the meaning to the users (and is perhaps less verbose)? Should we be targeting Experimental or fully Supported with this proposal? (Do we think user feedback is required to solidify an interface / experience?) \ No newline at end of file From 81ad0a242f12e5354b3752f9e052599e68c2203b Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 14 Jan 2025 09:15:55 -0500 Subject: [PATCH 09/31] mend --- .gitignore | 2 +- rfcs/proposed/passed_directly_API/README.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 8ce10921d74..c1db052baad 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ # -------- CMake -------- -build/* +build*/* # -------- IDE -------- .vscode/* diff --git a/rfcs/proposed/passed_directly_API/README.md b/rfcs/proposed/passed_directly_API/README.md index 5ebc54591e0..9eb5a1f3888 100644 --- a/rfcs/proposed/passed_directly_API/README.md +++ b/rfcs/proposed/passed_directly_API/README.md @@ -23,8 +23,8 @@ which are not part of oneDPL's specified interface. ## Proposal -Create a customization point `oneapi::dpl::is_passed_directly_in_onedpl_device_policies` free function which allows users to -define to mark their types as passed directly: +Create a customization point `oneapi::dpl::is_passed_directly_in_onedpl_device_policies` free function which allows +users to define to mark their types as passed directly: ``` template From 36b8ec561522992f8fa1c07fa5672cfbdc0b3d77 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 14 Jan 2025 09:30:48 -0500 Subject: [PATCH 10/31] rename directory Signed-off-by: Dan Hoeflinger --- .../README.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename rfcs/proposed/{passed_directly_API => iterators_pass_directly_customization}/README.md (100%) diff --git a/rfcs/proposed/passed_directly_API/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md similarity index 100% rename from rfcs/proposed/passed_directly_API/README.md rename to rfcs/proposed/iterators_pass_directly_customization/README.md From b0dac4f69cc132cd3f845b4ab28ba639b5e23346 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 14 Jan 2025 10:09:57 -0500 Subject: [PATCH 11/31] address feedback and minor adjustments Signed-off-by: Dan Hoeflinger --- .../README.md | 54 ++++++++++--------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 9eb5a1f3888..7fe50479589 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -1,30 +1,30 @@ -# Passed Directly Customiation Point for User Defined Types +# Passed Directly Customization Point for User Defined Types ## Introduction -OneDPL handles some types of input data automatically as input to its dpcpp (sycl-based) backend as described +oneDPL handles some types of input data automatically as input to its dpcpp (sycl-based) backend as described [here](https://uxlfoundation.github.io/oneDPL/parallel_api/pass_data_algorithms.html). Unified Shared Memory (USM) pointers refer to data which is device accessible inherently, so no processing is required to pass this type of input -data to SYCL kernels, we refer to this trait as "passed directly". OneDPL also defines some rules for its provided +data to SYCL kernels, we refer to this trait as "passed directly". oneDPL also defines some rules for its provided [iterator types](https://uxlfoundation.github.io/oneDPL/parallel_api/iterators.html) to be passed directly to SYCL under some circumstances (based usually on their base types). Internally, these rules are defined with a trait `oneapi::dpl::__ranges::is_passed_directly` which evaluates to -`std::true_type` or `std::false_type` to indicate whether the type `T` should be passed directly to sycl kernels. -There exists a unofficial legacy `is_passed_directly` trait which types can define like this: +`std::true_type` or `std::false_type` to indicate whether the iterator type `T` should be passed directly to sycl +kernels. There exists an unofficial legacy `is_passed_directly` trait which types can define like this: `using is_passed_directly = std::true_type;` which is supported within oneDPL. This method is currently used for a number of helper types within the SYCLomatic compatibility headers, (`device_pointer`, `device_iterator`, `tagged_pointer`, `constant_iterator`, `iterator_adaptor`). There is no official public API for users who want to -create their own types which could be passed directly to SYCL kernels, this is a gap we should fill in with an official -public API. +create their own iterator types which could be passed directly to SYCL kernels, this is a gap that should be filled +with an official public API. -Without something like this users are forced to only rely upon our provided types, or reach into implementation details -which are not part of oneDPL's specified interface. +Without something like this users are forced to only rely upon our provided iterator types, or reach into implementation +details which are not part of oneDPL's specified interface. ## Proposal Create a customization point `oneapi::dpl::is_passed_directly_in_onedpl_device_policies` free function which allows -users to define to mark their types as passed directly: +users to mark their types as passed directly: ``` template @@ -54,7 +54,6 @@ namespace user /* unspecified user definition */ }; - template constexpr bool is_passed_directly_in_onedpl_device_policies(const my_passed_directly_type&) @@ -66,8 +65,14 @@ namespace user Users can use any constexpr logic based on their type to determine if the type can be passed directly into a SYCL kernel without any processing. Below is an example of a type which contains a pair of iterators, and should be treated as -passed directly if and only if both base iterators are also passed directly. OneDPL will use this customization point -internally when determining how to handle incoming data, picking up any user customizations in the process. +passed directly if and only if both base iterators are also passed directly. oneDPL will use this customization point +internally when determining how to handle incoming data, picking up any user defined customizations. + +When using device policies, oneDPL will check iterator types by calling `is_passed_directly_in_onedpl_device_policies` +and if `true` is returned, will pass the iterator directly to sycl kernels rather than the data into sycl buffers, and +then using the sycl buffer to handle the device accessibility of the data. Users may also call +`oneapi::dpl::is_passed_directly_in_onedpl_device_policies` themselves to check how the oneDPL internals will treat any +iterator types. This may be useful to ensure that no extra overhead occurs in device policy calls. ``` namespace user @@ -98,14 +103,16 @@ This option can exist in concert with existing methods, the legacy `is_passed_di implementation away from explicit specializations of the trait to the customization point, but that is not required at first implementation. -### Implementation details +### Implementation Details To make this robust, we will follow a C++17 updated version of what is discussed in [Eric Niebler's Post](https://ericniebler.com/2014/10/21/customization-point-design-in-c11-and-beyond/), using a callable, and using an `inline constexpr` to avoid issues with ODR and to avoid issues with resolving customization -points when not separating the call to two steps with a `using` statement first. +points when not separating the call to two steps with a `using` statement first. Using his proposed method will allow +both qualified and unqualified calls to `is_passed_directly_in_onedpl_device_policies` to pick up the default +implementation provided by oneDPL. ## Alternatives considered -### Public trait struct explicit specialization +### Public Trait Struct Explicit Specialization We could simply make public our internal structure `oneapi::dpl::__ranges::is_passed_directly` as `oneapi::dpl::is_passed_directly` for users to specialize to define rules for their types. This would be a similar mechanism to `sycl::is_device_copyable`. The implementation details of this option should avoid some complexity required @@ -114,7 +121,7 @@ to properly implement the customization point. However, as we have learned from experience within oneDPL, explicit specialization of a structure in another library's namespace makes for maintenance problems. It either requires lots of closing of nested namespaces, opening of the external library's namespace for the specialization or it requires separating these specializations to a separate -location removed from the types they are specializing for. OneDPL has chosen to use the later, which can be seen in +location removed from the types they are specializing for. oneDPL has chosen to use the later, which can be seen in `include/oneapi/dpl/pstl/hetero/dpcpp/sycl_traits.h`. This has made for several errors where changes to structures should have included changes to sycl_traits, but did not, and needed to be fixed later. @@ -122,7 +129,7 @@ In an effort to avoid this same issue for our users, we propose a similar method customization point, allowing the user to override that customization point within their own namespace as a free function. -### Require specifically named typedef / using in user's type +### Require Specifically Named Typedef / Using in User's Type We could simply make official our requirements for user's types to include a typedef or using statement to define if the type is passed directly like `using is_passed_directly = std::true_type;`, where the absence of this would be equivalent to a `std::false_type`. @@ -131,15 +138,14 @@ However, this clutters the user type definitions with specifics of oneDPL. It al signifies for maintenance of user code without appropriate comments describing the details of oneDPL and SYCL. Users have expressed that this is undesirable. -### Testing +## Testing We will need a detailed test checking both positive and negative responses to `is_passed_directly_in_onedpl_device_policies` have the expected result, with custom types and combinations of iterators, usm pointers etc. ## Open Questions -Is there a better / more concise name than `is_passed_directly_in_onedpl_device_policies` we can use which properly -conveys the meaning to the users (and is perhaps less verbose)? - -Should we be targeting Experimental or fully Supported with this proposal? - (Do we think user feedback is required to solidify an interface / experience?) \ No newline at end of file +* Is there a better, more concise name than `is_passed_directly_in_onedpl_device_policies` that properly conveys the +meaning to the users? +* Should we be targeting Experimental or fully Supported with this proposal? +(Do we think user feedback is required to solidify an interface / experience?) \ No newline at end of file From 4823c6eb10801e48a9e82bce836454302c2355eb Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 14 Jan 2025 17:00:38 -0500 Subject: [PATCH 12/31] improvements for clarity Signed-off-by: Dan Hoeflinger --- .../README.md | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 7fe50479589..cc66f603a97 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -68,11 +68,12 @@ without any processing. Below is an example of a type which contains a pair of i passed directly if and only if both base iterators are also passed directly. oneDPL will use this customization point internally when determining how to handle incoming data, picking up any user defined customizations. -When using device policies, oneDPL will check iterator types by calling `is_passed_directly_in_onedpl_device_policies` -and if `true` is returned, will pass the iterator directly to sycl kernels rather than the data into sycl buffers, and -then using the sycl buffer to handle the device accessibility of the data. Users may also call -`oneapi::dpl::is_passed_directly_in_onedpl_device_policies` themselves to check how the oneDPL internals will treat any -iterator types. This may be useful to ensure that no extra overhead occurs in device policy calls. +When using device policies, oneDPL will run compile time checks on argument iterator types by calling +`is_passed_directly_in_onedpl_device_policies` as a `constexpr`. If `true` is returned, oneDPL will pass the iterator +directly to sycl kernels rather than copying the data into sycl buffers and using accessors to those buffers in the +kernel. Users may also call `oneapi::dpl::is_passed_directly_in_onedpl_device_policies` themselves to check how the +oneDPL internals will treat any iterator types. This may be useful to ensure that no extra overhead occurs in device +policy calls. ``` namespace user @@ -103,13 +104,18 @@ This option can exist in concert with existing methods, the legacy `is_passed_di implementation away from explicit specializations of the trait to the customization point, but that is not required at first implementation. +`oneapi::dpl::is_passed_directly_in_onedpl_device_policies()` will be defined in `oneapi/dpl/execution`. It must be +included prior to calling or overriding `oneapi::dpl::is_passed_directly_in_onedpl_device_policies()` with their own +customizations. + ### Implementation Details -To make this robust, we will follow a C++17 updated version of what is discussed in -[Eric Niebler's Post](https://ericniebler.com/2014/10/21/customization-point-design-in-c11-and-beyond/), using a -callable, and using an `inline constexpr` to avoid issues with ODR and to avoid issues with resolving customization -points when not separating the call to two steps with a `using` statement first. Using his proposed method will allow -both qualified and unqualified calls to `is_passed_directly_in_onedpl_device_policies` to pick up the default -implementation provided by oneDPL. +We will follow a C++17 updated version of what is discussed in +[Eric Niebler's Post](https://ericniebler.com/2014/10/21/customization-point-design-in-c11-and-beyond/). Using his +proposed method will allow unqualified calls to `is_passed_directly_in_onedpl_device_policies()` after a +`using oneapi::dpl::is_passed_directly_in_onedpl_device_policies;` statement, as well as qualified calls to +`oneapi::dpl::is_passed_directly_in_onedpl_device_policies()` to find the default implementation provided by oneDPL. +Both options will also have access to any user defined customizations defined in the same namespace of the type. +With access to c++17, we will use `inline constexpr` to avoid issues with ODR, rather than his described method. ## Alternatives considered ### Public Trait Struct Explicit Specialization @@ -148,4 +154,4 @@ iterators, usm pointers etc. * Is there a better, more concise name than `is_passed_directly_in_onedpl_device_policies` that properly conveys the meaning to the users? * Should we be targeting Experimental or fully Supported with this proposal? -(Do we think user feedback is required to solidify an interface / experience?) \ No newline at end of file +(Do we think user feedback is required to solidify an interface / experience?) From b79e7f5035399ea8e92989aef871d65360448fa3 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 14 Jan 2025 17:27:35 -0500 Subject: [PATCH 13/31] adding the wrapper class alternative Signed-off-by: Dan Hoeflinger --- .../README.md | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index cc66f603a97..461139281c5 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -117,9 +117,22 @@ proposed method will allow unqualified calls to `is_passed_directly_in_onedpl_de Both options will also have access to any user defined customizations defined in the same namespace of the type. With access to c++17, we will use `inline constexpr` to avoid issues with ODR, rather than his described method. -## Alternatives considered +### Drawbacks +#### Unavailable For SFINAE +While `is_passed_directly_in_onedpl_device_policies` is defined to be `constexpr`, and all user customizations must also +be `constexpr`, they will be unavailable for `std::enable_if` or other SFINAE checks. These checks only have access to +the type of the template parameter, and do not have access to any named instance of that type. Therefore, without +imposing a requirement like default constructibility on types, we cannot use +`is_passed_directly_in_onedpl_device_policies` in this context, as we have no instance to use as the argument to our +Argument Dependant Lookup (ADL) function. This is an inconvenience, and it will require some refactoring of the code +which processes input sequences, but it should only impact internal usage of +`is_passed_directly_in_onedpl_device_policies`. I don't anticipate users wanting to incorporate this function into +their own SFINAE checks. Alternatives below do not have such drawbacks, but I still believe this to be the superior +option for users. + +## Alternatives Considered ### Public Trait Struct Explicit Specialization -We could simply make public our internal structure `oneapi::dpl::__ranges::is_passed_directly` as +oneDPL could make public our internal structure `oneapi::dpl::__ranges::is_passed_directly` as `oneapi::dpl::is_passed_directly` for users to specialize to define rules for their types. This would be a similar mechanism to `sycl::is_device_copyable`. The implementation details of this option should avoid some complexity required to properly implement the customization point. @@ -136,7 +149,7 @@ customization point, allowing the user to override that customization point with function. ### Require Specifically Named Typedef / Using in User's Type -We could simply make official our requirements for user's types to include a typedef or using statement to define if the +oneDPL could make official our requirements for user's types to include a typedef or using statement to define if the type is passed directly like `using is_passed_directly = std::true_type;`, where the absence of this would be equivalent to a `std::false_type`. @@ -144,6 +157,18 @@ However, this clutters the user type definitions with specifics of oneDPL. It al signifies for maintenance of user code without appropriate comments describing the details of oneDPL and SYCL. Users have expressed that this is undesirable. +### Wrapper Class +oneDPL could provide some wrapper iterator `direct_iterator` which wraps an arbitrary base iterator and marks it as +passed directly. `direct_iterator` could utilize either of the above alternatives to accomplish this, and signal +that the iterator should be passed directly. It would need to pass through all operations to the wrapped base iterator, +and make sure no overhead is added in its usage. +There is some complexity in adding such a wrapper iterator, and it would need to be considered carefully to make sure no +problems would be introduced. This wrapper class may obfuscate users types, and make them more unwieldy to use. It is +also less expressive than the other options in that it only has the ability to unilaterally mark a type as passed +directly. There is no logic that can be used to express some iterator type which may be conditionally passed directly, +other than to have logic to conditionally apply the wrapper in the first place. This option seems less clear and has +more opportunity to cause problems. + ## Testing We will need a detailed test checking both positive and negative responses to `is_passed_directly_in_onedpl_device_policies` have the expected result, with custom types and combinations of From 47b8be36c21b1c5417a66e3b93286186607d73b7 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 14 Jan 2025 17:30:46 -0500 Subject: [PATCH 14/31] spacing Signed-off-by: Dan Hoeflinger --- rfcs/proposed/iterators_pass_directly_customization/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 461139281c5..731987c034d 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -162,6 +162,7 @@ oneDPL could provide some wrapper iterator `direct_iterator` which wraps an arbi passed directly. `direct_iterator` could utilize either of the above alternatives to accomplish this, and signal that the iterator should be passed directly. It would need to pass through all operations to the wrapped base iterator, and make sure no overhead is added in its usage. + There is some complexity in adding such a wrapper iterator, and it would need to be considered carefully to make sure no problems would be introduced. This wrapper class may obfuscate users types, and make them more unwieldy to use. It is also less expressive than the other options in that it only has the ability to unilaterally mark a type as passed From 92b1d335ebdf17518c344d9a19c615f2f7c82182 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 14 Jan 2025 22:49:19 -0500 Subject: [PATCH 15/31] updating with new strategy Signed-off-by: Dan Hoeflinger --- .../README.md | 78 +++++++++---------- 1 file changed, 36 insertions(+), 42 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 731987c034d..30d85364a4f 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -23,26 +23,39 @@ details which are not part of oneDPL's specified interface. ## Proposal -Create a customization point `oneapi::dpl::is_passed_directly_in_onedpl_device_policies` free function which allows -users to mark their types as passed directly: +Create a customization point `oneapi::dpl::is_passed_directly_in_onedpl_device_policies` which allows users to mark +their types as "passed directly" by returning either `std::true_type{}` or as "non passed directly" by returning +`std::false_type{}`. Also create also a helper `oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` +which is an `inline constexpr bool` indicating the passed directly status of the templated type `T`.`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` is very useful in `std::enable_if` and other SFINAE +contexts, since we only require the typename rather than a named instance of the type to determine if the type is passed +directly. It is also useful in the context of defining customizations for user types which are conditionally passed +directly, as shown in an example below. -``` -template -constexpr bool is_passed_directly_in_onedpl_device_policies(const T&); -``` - -oneDPL will provide a default implementation which will defer to the existing trait: +The default implementation will defer to the existing `oneapi::dpl::__ranges::is_passed_directly` structure, which is +already used internally to oneDPL. ``` template constexpr -bool +auto is_passed_directly_in_onedpl_device_policies(const T&) { - return oneapi::dpl::__ranges::is_passed_directly_v; + return oneapi::dpl::__ranges::is_passed_directly{}; } ``` +`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` will rely upon the customization point +`is_passed_directly_in_onedpl_device_policies`, as well as `std::declval()` and `decltype` to determine the passed +directly status of a type. oneDPL will use this customization point internally when determining how to handle incoming +data, picking up any user defined customizations. + +When using device policies, oneDPL will run compile time checks on argument iterator types by calling +`is_passed_directly_in_onedpl_device_policies` as a `constexpr`. If `std::true_type` is returned, oneDPL will pass the +iterator directly to sycl kernels rather than copying the data into sycl buffers and using accessors to those buffers in +the kernel. Users may also call `oneapi::dpl::is_passed_directly_in_onedpl_device_policies[_v]` themselves to check how +the oneDPL internals will treat any iterator types. This may be useful to ensure that no extra overhead occurs in device +policy calls. + Below is a simple example of a type and customization point definition which is always passed directly. ``` @@ -55,43 +68,37 @@ namespace user }; constexpr - bool + auto is_passed_directly_in_onedpl_device_policies(const my_passed_directly_type&) { - return true; + return std::true_type{}; } } //namespace user ``` Users can use any constexpr logic based on their type to determine if the type can be passed directly into a SYCL kernel without any processing. Below is an example of a type which contains a pair of iterators, and should be treated as -passed directly if and only if both base iterators are also passed directly. oneDPL will use this customization point -internally when determining how to handle incoming data, picking up any user defined customizations. - -When using device policies, oneDPL will run compile time checks on argument iterator types by calling -`is_passed_directly_in_onedpl_device_policies` as a `constexpr`. If `true` is returned, oneDPL will pass the iterator -directly to sycl kernels rather than copying the data into sycl buffers and using accessors to those buffers in the -kernel. Users may also call `oneapi::dpl::is_passed_directly_in_onedpl_device_policies` themselves to check how the -oneDPL internals will treat any iterator types. This may be useful to ensure that no extra overhead occurs in device -policy calls. +passed directly if and only if both base iterators are also passed directly. ``` namespace user { template - struct iterator_pair + struct iterator_pair { It1 first; It2 second; }; template - constexpr - bool - is_passed_directly_in_onedpl_device_policies(const iterator_pair& pair) + constexpr + auto is_passed_directly_in_onedpl_device_policies(const iterator_pair&) { - return oneapi::dpl::is_passed_directly_in_onedpl_device_policies(pair.first) && - oneapi::dpl::is_passed_directly_in_onedpl_device_policies(pair.second); + if constexpr (oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v && + oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v) + return std::true_type{}; + else + return std::false_type{}; } } //namespace user ``` @@ -104,8 +111,8 @@ This option can exist in concert with existing methods, the legacy `is_passed_di implementation away from explicit specializations of the trait to the customization point, but that is not required at first implementation. -`oneapi::dpl::is_passed_directly_in_onedpl_device_policies()` will be defined in `oneapi/dpl/execution`. It must be -included prior to calling or overriding `oneapi::dpl::is_passed_directly_in_onedpl_device_policies()` with their own +`oneapi::dpl::is_passed_directly_in_onedpl_device_policies[_v]` will be defined in `oneapi/dpl/execution` which must be +included prior to calling or overriding `oneapi::dpl::is_passed_directly_in_onedpl_device_policies()` with customizations. ### Implementation Details @@ -117,19 +124,6 @@ proposed method will allow unqualified calls to `is_passed_directly_in_onedpl_de Both options will also have access to any user defined customizations defined in the same namespace of the type. With access to c++17, we will use `inline constexpr` to avoid issues with ODR, rather than his described method. -### Drawbacks -#### Unavailable For SFINAE -While `is_passed_directly_in_onedpl_device_policies` is defined to be `constexpr`, and all user customizations must also -be `constexpr`, they will be unavailable for `std::enable_if` or other SFINAE checks. These checks only have access to -the type of the template parameter, and do not have access to any named instance of that type. Therefore, without -imposing a requirement like default constructibility on types, we cannot use -`is_passed_directly_in_onedpl_device_policies` in this context, as we have no instance to use as the argument to our -Argument Dependant Lookup (ADL) function. This is an inconvenience, and it will require some refactoring of the code -which processes input sequences, but it should only impact internal usage of -`is_passed_directly_in_onedpl_device_policies`. I don't anticipate users wanting to incorporate this function into -their own SFINAE checks. Alternatives below do not have such drawbacks, but I still believe this to be the superior -option for users. - ## Alternatives Considered ### Public Trait Struct Explicit Specialization oneDPL could make public our internal structure `oneapi::dpl::__ranges::is_passed_directly` as From 5c45e7dfc20fc0c24e376c879f5245101f4effbe Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Wed, 15 Jan 2025 15:53:28 -0500 Subject: [PATCH 16/31] improvements to language and strategy only offering customization point and trait as public API Signed-off-by: Dan Hoeflinger --- .../README.md | 68 ++++++++----------- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 30d85364a4f..ec91dd6eea3 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -23,16 +23,16 @@ details which are not part of oneDPL's specified interface. ## Proposal -Create a customization point `oneapi::dpl::is_passed_directly_in_onedpl_device_policies` which allows users to mark +Create a customization point `is_passed_directly_in_onedpl_device_policies` which allows users to mark their types as "passed directly" by returning either `std::true_type{}` or as "non passed directly" by returning -`std::false_type{}`. Also create also a helper `oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` -which is an `inline constexpr bool` indicating the passed directly status of the templated type `T`.`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` is very useful in `std::enable_if` and other SFINAE +`std::false_type{}`. Also create also a public trait `oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` +which is an `inline constexpr bool` indicating the "passed directly" status of the template type `T`.`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` is very useful in `std::enable_if` and other SFINAE contexts, since we only require the typename rather than a named instance of the type to determine if the type is passed directly. It is also useful in the context of defining customizations for user types which are conditionally passed directly, as shown in an example below. -The default implementation will defer to the existing `oneapi::dpl::__ranges::is_passed_directly` structure, which is -already used internally to oneDPL. +The default implementation of the customization point `is_passed_directly_in_onedpl_device_policies` will defer to the +existing `oneapi::dpl::__ranges::is_passed_directly` structure as shown, which is already used internally to oneDPL. ``` template @@ -44,17 +44,32 @@ is_passed_directly_in_onedpl_device_policies(const T&) } ``` +User who want to customize this function for their own type must override it in the same namespace as their defined +type. + `oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` will rely upon the customization point `is_passed_directly_in_onedpl_device_policies`, as well as `std::declval()` and `decltype` to determine the passed directly status of a type. oneDPL will use this customization point internally when determining how to handle incoming data, picking up any user defined customizations. When using device policies, oneDPL will run compile time checks on argument iterator types by calling -`is_passed_directly_in_onedpl_device_policies` as a `constexpr`. If `std::true_type` is returned, oneDPL will pass the -iterator directly to sycl kernels rather than copying the data into sycl buffers and using accessors to those buffers in -the kernel. Users may also call `oneapi::dpl::is_passed_directly_in_onedpl_device_policies[_v]` themselves to check how -the oneDPL internals will treat any iterator types. This may be useful to ensure that no extra overhead occurs in device -policy calls. +`is_passed_directly_in_onedpl_device_policies`. If `std::true_type{}` is returned, oneDPL will pass the iterator +directly to sycl kernels rather than copying the data into `sycl::buffers` and using those buffers to transfer data to +sycl kernels. Users may also use `oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` themselves to check +how the oneDPL internals will treat any iterator types. This may be useful to ensure that no extra overhead occurs in +device policy calls of oneDPL APIs. + +This option can exist in concert with existing methods, the legacy `is_passed_directly` typedef in types, the internal +`oneapi::dpl::__ranges::is_passed_directly` trait specializations. It would be possible to simplify the internal +implementation away from explicit specializations and instead using the customization point, but that is not required +at first implementation. + +`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` will be defined in `oneapi/dpl/execution`. + +The specification and implementation will be prepared once this RFC is accepted as "proposed", we do not intend to offer +this first as experimental. This RFC will target "Supported" once we have implementation solidified its details. + +### Examples Below is a simple example of a type and customization point definition which is always passed directly. @@ -76,9 +91,9 @@ namespace user } //namespace user ``` -Users can use any constexpr logic based on their type to determine if the type can be passed directly into a SYCL kernel -without any processing. Below is an example of a type which contains a pair of iterators, and should be treated as -passed directly if and only if both base iterators are also passed directly. +Users can use any `constexpr` logic based on their type to determine if the type can be passed directly into a SYCL +kernel without any processing. Below is an example of a type which contains a pair of iterators, and should be treated +as passed directly if and only if both base iterators are also passed directly. ``` namespace user @@ -103,27 +118,6 @@ namespace user } //namespace user ``` -This allows the user to provide rules for their types next to their implementation, without cluttering the -implementation of the type itself with extra typedefs, etc. - -This option can exist in concert with existing methods, the legacy `is_passed_directly` typedef in types, the internal -`oneapi::dpl::__ranges::is_passed_directly` trait specializations. It would be possible to simplify the internal -implementation away from explicit specializations of the trait to the customization point, but that is not required -at first implementation. - -`oneapi::dpl::is_passed_directly_in_onedpl_device_policies[_v]` will be defined in `oneapi/dpl/execution` which must be -included prior to calling or overriding `oneapi::dpl::is_passed_directly_in_onedpl_device_policies()` with -customizations. - -### Implementation Details -We will follow a C++17 updated version of what is discussed in -[Eric Niebler's Post](https://ericniebler.com/2014/10/21/customization-point-design-in-c11-and-beyond/). Using his -proposed method will allow unqualified calls to `is_passed_directly_in_onedpl_device_policies()` after a -`using oneapi::dpl::is_passed_directly_in_onedpl_device_policies;` statement, as well as qualified calls to -`oneapi::dpl::is_passed_directly_in_onedpl_device_policies()` to find the default implementation provided by oneDPL. -Both options will also have access to any user defined customizations defined in the same namespace of the type. -With access to c++17, we will use `inline constexpr` to avoid issues with ODR, rather than his described method. - ## Alternatives Considered ### Public Trait Struct Explicit Specialization oneDPL could make public our internal structure `oneapi::dpl::__ranges::is_passed_directly` as @@ -166,12 +160,10 @@ more opportunity to cause problems. ## Testing We will need a detailed test checking both positive and negative responses to -`is_passed_directly_in_onedpl_device_policies` have the expected result, with custom types and combinations of -iterators, usm pointers etc. +`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` have the expected result, with custom types and +combinations of iterators, usm pointers etc. ## Open Questions * Is there a better, more concise name than `is_passed_directly_in_onedpl_device_policies` that properly conveys the meaning to the users? -* Should we be targeting Experimental or fully Supported with this proposal? -(Do we think user feedback is required to solidify an interface / experience?) From 8bcc334dcf0c530803e93ca9f5a108c09f217775 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Thu, 16 Jan 2025 09:58:37 -0500 Subject: [PATCH 17/31] change header to type_traits Signed-off-by: Dan Hoeflinger --- rfcs/proposed/iterators_pass_directly_customization/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index ec91dd6eea3..db7ceaf91ca 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -64,7 +64,7 @@ This option can exist in concert with existing methods, the legacy `is_passed_di implementation away from explicit specializations and instead using the customization point, but that is not required at first implementation. -`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` will be defined in `oneapi/dpl/execution`. +`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` will be defined in `oneapi/dpl/type_traits`. The specification and implementation will be prepared once this RFC is accepted as "proposed", we do not intend to offer this first as experimental. This RFC will target "Supported" once we have implementation solidified its details. From 27dc9ba222b8a6351019111e997a73a05d0f3487 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 10 Mar 2025 14:21:42 -0400 Subject: [PATCH 18/31] reverting accidental gitignore checkin Signed-off-by: Dan Hoeflinger --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index c1db052baad..8ce10921d74 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ # -------- CMake -------- -build*/* +build/* # -------- IDE -------- .vscode/* From 60cd1a92c71b9a5b15aa7686e50513d7483cfe3e Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 11 Mar 2025 09:03:15 -0400 Subject: [PATCH 19/31] expand open questions Signed-off-by: Dan Hoeflinger --- .../iterators_pass_directly_customization/README.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index db7ceaf91ca..0421dda8603 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -165,5 +165,11 @@ combinations of iterators, usm pointers etc. ## Open Questions -* Is there a better, more concise name than `is_passed_directly_in_onedpl_device_policies` that properly conveys the -meaning to the users? +* Is there a better, more concise name than `oneapi::dpl::is_passed_directly_in_onedpl_device_policies[_v]` that +properly conveys the meaning to the users? + * Other names proposed: + * `oneapi::dpl::onedpl_is_iterator_device_ready[_v]` + * `oneapi::dpl::is_passed_directly_to_sycl_backend[_v]` + * `oneapi::dpl::requires_explicit_data_transfer_onedpl_device_policies[_v]` (inverted) +* Where should this be located? + * Possible options include `oneapi/dpl/iterator`, `oneapi/dpl/type_traits`. \ No newline at end of file From 2ddaf9df475f75540e3905c8f1e28814f5d843b2 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 11 Mar 2025 09:55:45 -0400 Subject: [PATCH 20/31] revisions for clarity and specificity Signed-off-by: Dan Hoeflinger --- .../README.md | 89 +++++++++---------- 1 file changed, 42 insertions(+), 47 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 0421dda8603..944df3f8b4b 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -22,78 +22,73 @@ Without something like this users are forced to only rely upon our provided iter details which are not part of oneDPL's specified interface. ## Proposal - -Create a customization point `is_passed_directly_in_onedpl_device_policies` which allows users to mark -their types as "passed directly" by returning either `std::true_type{}` or as "non passed directly" by returning -`std::false_type{}`. Also create also a public trait `oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` -which is an `inline constexpr bool` indicating the "passed directly" status of the template type `T`.`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` is very useful in `std::enable_if` and other SFINAE -contexts, since we only require the typename rather than a named instance of the type to determine if the type is passed -directly. It is also useful in the context of defining customizations for user types which are conditionally passed -directly, as shown in an example below. - -The default implementation of the customization point `is_passed_directly_in_onedpl_device_policies` will defer to the -existing `oneapi::dpl::__ranges::is_passed_directly` structure as shown, which is already used internally to oneDPL. - -``` -template -constexpr -auto -is_passed_directly_in_onedpl_device_policies(const T&) -{ - return oneapi::dpl::__ranges::is_passed_directly{}; -} -``` - -User who want to customize this function for their own type must override it in the same namespace as their defined -type. - -`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` will rely upon the customization point -`is_passed_directly_in_onedpl_device_policies`, as well as `std::declval()` and `decltype` to determine the passed -directly status of a type. oneDPL will use this customization point internally when determining how to handle incoming -data, picking up any user defined customizations. +### High Level Proposal + +We will create an Argument-Dependant Lookup (ADL) customization point which defines if iterators are "passed +directly" `is_passed_directly_in_onedpl_device_policies`. Users may define if their iterator types should be "passed +directly" by onedpl by defining a constexpr function in the same namespace where the iterator is defined. +`is_passed_directly_in_onedpl_device_policies` must accept a const lvalue reference to the iterator type being +specialized and return `std::true_type{}` if the iterator type is "passed directly" and `std::false_type{}` otherwise. + +Additionaly, oneDPL will provide a public trait, +`inline constexpr bool oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v`, indicating if the iterator type +`T` is "passed directly". This public trait is intended to be used to help define +`is_passed_directly_in_onedpl_device_policies` for wrapper iterator types which depend on the "passed directly" status +of their base iterator(s) using only the base iterator type(s) rather than a named instance. It may also be used to +confirm user iterator types "passed directly" traits are as intended to prevent unnecessary overhead in oneDPL calls. + +### Additional Information + +The default implementation of the customization point `is_passed_directly_in_onedpl_device_policies` will be used to +explicitly mark the following iterators as "passed directly": +* pointers (assumes usm pointers) +* iterators containing the legacy `using is_passed_directly = true` trait defined within the type definition +* iterators to usm shared allocated `std::vectors` (when knowable) +* `std::reverse_iterator` when `Iter` is also "passed directly" + +oneDPL will define the "passed directly" definitions of its custom iterators as follows: +* `zip_iterator` is "passed directly" when all base iterators are "passed directly" +* `counting_iterator` and `discard_iterators` are always "passed directly" +* `transform_iterator` is "passed directly" if its source iterator is "passed directly" +* `permutation_iterator` is "passed directly" if both its source iterator and its index map are "passed directly" + +### Implementation Details When using device policies, oneDPL will run compile time checks on argument iterator types by calling `is_passed_directly_in_onedpl_device_policies`. If `std::true_type{}` is returned, oneDPL will pass the iterator directly to sycl kernels rather than copying the data into `sycl::buffers` and using those buffers to transfer data to -sycl kernels. Users may also use `oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` themselves to check -how the oneDPL internals will treat any iterator types. This may be useful to ensure that no extra overhead occurs in -device policy calls of oneDPL APIs. - -This option can exist in concert with existing methods, the legacy `is_passed_directly` typedef in types, the internal -`oneapi::dpl::__ranges::is_passed_directly` trait specializations. It would be possible to simplify the internal -implementation away from explicit specializations and instead using the customization point, but that is not required -at first implementation. - -`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` will be defined in `oneapi/dpl/type_traits`. +SYCL kernels. The specification and implementation will be prepared once this RFC is accepted as "proposed", we do not intend to offer -this first as experimental. This RFC will target "Supported" once we have implementation solidified its details. +this first as experimental. This RFC will target "Supported" once specification and implementation are accepted. ### Examples -Below is a simple example of a type and customization point definition which is always passed directly. +Below is a simple example of an iterator and ADL customization point definition which is always "passed directly". ``` namespace user { - struct my_passed_directly_type + struct my_passed_directly_iterator { /* unspecified user definition */ }; constexpr auto - is_passed_directly_in_onedpl_device_policies(const my_passed_directly_type&) + is_passed_directly_in_onedpl_device_policies(const my_passed_directly_iterator&) { return std::true_type{}; } } //namespace user ``` -Users can use any `constexpr` logic based on their type to determine if the type can be passed directly into a SYCL -kernel without any processing. Below is an example of a type which contains a pair of iterators, and should be treated -as passed directly if and only if both base iterators are also passed directly. +Users can use any `constexpr` logic based on their iterator to determine if the iterator can be passed directly into a +SYCL kernel without any processing. Below is an example of a type which contains a pair of iterators, and should be +treated as passed directly if and only if both base iterators are also passed directly. Note that the use of the public +trait enables use of the base iterator type alone without creating a named instance of the base iterator to pass into +the ADL customization point. ``` namespace user @@ -171,5 +166,5 @@ properly conveys the meaning to the users? * `oneapi::dpl::onedpl_is_iterator_device_ready[_v]` * `oneapi::dpl::is_passed_directly_to_sycl_backend[_v]` * `oneapi::dpl::requires_explicit_data_transfer_onedpl_device_policies[_v]` (inverted) -* Where should this be located? +* Where should this be located? * Possible options include `oneapi/dpl/iterator`, `oneapi/dpl/type_traits`. \ No newline at end of file From 497b8c6805e61ac84e1bbb33a486a91609c17c2f Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 11 Mar 2025 10:12:51 -0400 Subject: [PATCH 21/31] spelling grammar, minor edits Signed-off-by: Dan Hoeflinger --- .../README.md | 102 +++++++++--------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 944df3f8b4b..b65855a8118 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -2,37 +2,37 @@ ## Introduction -oneDPL handles some types of input data automatically as input to its dpcpp (sycl-based) backend as described +oneDPL handles some types of input data automatically as input to its device backend (SYCL-based) as described [here](https://uxlfoundation.github.io/oneDPL/parallel_api/pass_data_algorithms.html). Unified Shared Memory (USM) -pointers refer to data which is device accessible inherently, so no processing is required to pass this type of input -data to SYCL kernels, we refer to this trait as "passed directly". oneDPL also defines some rules for its provided +pointers refer to data that is inherently device accessible, so no processing is required to pass this type of input +data to SYCL kernels. We refer to this trait as "passed directly". oneDPL also defines some rules for its provided [iterator types](https://uxlfoundation.github.io/oneDPL/parallel_api/iterators.html) to be passed directly to SYCL -under some circumstances (based usually on their base types). +under some circumstances (usually based on their base types). Internally, these rules are defined with a trait `oneapi::dpl::__ranges::is_passed_directly` which evaluates to -`std::true_type` or `std::false_type` to indicate whether the iterator type `T` should be passed directly to sycl +`std::true_type` or `std::false_type` to indicate whether the iterator type `T` should be passed directly to SYCL kernels. There exists an unofficial legacy `is_passed_directly` trait which types can define like this: `using is_passed_directly = std::true_type;` which is supported within oneDPL. This method is currently used for a -number of helper types within the SYCLomatic compatibility headers, (`device_pointer`, `device_iterator`, +number of helper types within the SYCLomatic compatibility headers (`device_pointer`, `device_iterator`, `tagged_pointer`, `constant_iterator`, `iterator_adaptor`). There is no official public API for users who want to -create their own iterator types which could be passed directly to SYCL kernels, this is a gap that should be filled +create their own iterator types that could be passed directly to SYCL kernels. This is a gap that should be filled with an official public API. -Without something like this users are forced to only rely upon our provided iterator types, or reach into implementation -details which are not part of oneDPL's specified interface. +Without something like this, users are forced to rely only upon our provided iterator types or reach into implementation +details that are not part of oneDPL's specified interface. ## Proposal ### High Level Proposal -We will create an Argument-Dependant Lookup (ADL) customization point which defines if iterators are "passed -directly" `is_passed_directly_in_onedpl_device_policies`. Users may define if their iterator types should be "passed -directly" by onedpl by defining a constexpr function in the same namespace where the iterator is defined. +We will create an Argument-Dependent Lookup (ADL) customization point which defines if iterators are "passed directly" +`is_passed_directly_in_onedpl_device_policies`. Users may define if their iterator types should be "passed directly" by +oneDPL by defining a constexpr function in the same namespace where the iterator is defined. `is_passed_directly_in_onedpl_device_policies` must accept a const lvalue reference to the iterator type being specialized and return `std::true_type{}` if the iterator type is "passed directly" and `std::false_type{}` otherwise. -Additionaly, oneDPL will provide a public trait, +Additionally, oneDPL will provide a public trait, `inline constexpr bool oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v`, indicating if the iterator type -`T` is "passed directly". This public trait is intended to be used to help define +`T` is "passed directly". This public trait is intended to be used to help define `is_passed_directly_in_onedpl_device_policies` for wrapper iterator types which depend on the "passed directly" status of their base iterator(s) using only the base iterator type(s) rather than a named instance. It may also be used to confirm user iterator types "passed directly" traits are as intended to prevent unnecessary overhead in oneDPL calls. @@ -41,26 +41,26 @@ confirm user iterator types "passed directly" traits are as intended to prevent The default implementation of the customization point `is_passed_directly_in_onedpl_device_policies` will be used to explicitly mark the following iterators as "passed directly": -* pointers (assumes usm pointers) -* iterators containing the legacy `using is_passed_directly = true` trait defined within the type definition -* iterators to usm shared allocated `std::vectors` (when knowable) +* Pointers (assumes USM pointers) +* Iterators containing the legacy `using is_passed_directly = true` trait defined within the type definition +* Iterators to USM shared allocated `std::vectors` (when knowable) * `std::reverse_iterator` when `Iter` is also "passed directly" oneDPL will define the "passed directly" definitions of its custom iterators as follows: * `zip_iterator` is "passed directly" when all base iterators are "passed directly" -* `counting_iterator` and `discard_iterators` are always "passed directly" +* `counting_iterator` and `discard_iterator` are always "passed directly" * `transform_iterator` is "passed directly" if its source iterator is "passed directly" * `permutation_iterator` is "passed directly" if both its source iterator and its index map are "passed directly" ### Implementation Details -When using device policies, oneDPL will run compile time checks on argument iterator types by calling +When using device policies, oneDPL will run compile-time checks on argument iterator types by calling `is_passed_directly_in_onedpl_device_policies`. If `std::true_type{}` is returned, oneDPL will pass the iterator -directly to sycl kernels rather than copying the data into `sycl::buffers` and using those buffers to transfer data to -SYCL kernels. +directly to SYCL kernels rather than copying the data into `sycl::buffers` and using those buffers to transfer data to +SYCL kernels. -The specification and implementation will be prepared once this RFC is accepted as "proposed", we do not intend to offer -this first as experimental. This RFC will target "Supported" once specification and implementation are accepted. +The specification and implementation will be prepared once this RFC is accepted as "proposed". We do not intend to offer +this first as experimental. This RFC will target "Supported" once the specification and implementation are accepted. ### Examples @@ -85,10 +85,10 @@ namespace user ``` Users can use any `constexpr` logic based on their iterator to determine if the iterator can be passed directly into a -SYCL kernel without any processing. Below is an example of a type which contains a pair of iterators, and should be +SYCL kernel without any processing. Below is an example of a type which contains a pair of iterators and should be treated as passed directly if and only if both base iterators are also passed directly. Note that the use of the public -trait enables use of the base iterator type alone without creating a named instance of the base iterator to pass into -the ADL customization point. +trait enables the use of the base iterator type alone without creating a named instance of the base iterator to pass +into the ADL. ``` namespace user @@ -118,23 +118,23 @@ namespace user oneDPL could make public our internal structure `oneapi::dpl::__ranges::is_passed_directly` as `oneapi::dpl::is_passed_directly` for users to specialize to define rules for their types. This would be a similar mechanism to `sycl::is_device_copyable`. The implementation details of this option should avoid some complexity required -to properly implement the customization point. + to properly implement the customization point. However, as we have learned from experience within oneDPL, explicit specialization of a structure in another library's -namespace makes for maintenance problems. It either requires lots of closing of nested namespaces, opening of the -external library's namespace for the specialization or it requires separating these specializations to a separate -location removed from the types they are specializing for. oneDPL has chosen to use the later, which can be seen in -`include/oneapi/dpl/pstl/hetero/dpcpp/sycl_traits.h`. This has made for several errors where changes to structures -should have included changes to sycl_traits, but did not, and needed to be fixed later. +namespace creates maintenance problems. It either requires lots of closing of nested namespaces, opening of the external + library's namespace for the specialization, or it requires separating these specializations to a separate location + removed from the types they are specializing for. oneDPL has chosen to use the latter, which can be seen in + `include/oneapi/dpl/pstl/hetero/dpcpp/sycl_traits.h`. This has led to several errors where changes to structures should + have included changes to sycl_traits but did not, and needed to be fixed later. In an effort to avoid this same issue for our users, we propose a similar method but instead with a constexpr customization point, allowing the user to override that customization point within their own namespace as a free function. ### Require Specifically Named Typedef / Using in User's Type -oneDPL could make official our requirements for user's types to include a typedef or using statement to define if the -type is passed directly like `using is_passed_directly = std::true_type;`, where the absence of this would be equivalent -to a `std::false_type`. +oneDPL could make official our requirements for users' types to include a typedef or using statement to define if the +type is "passed directly" like `using is_passed_directly = std::true_type;`, where the absence of this would be +equivalent to a `std::false_type`. However, this clutters the user type definitions with specifics of oneDPL. It also may not be as clear what this signifies for maintenance of user code without appropriate comments describing the details of oneDPL and SYCL. Users @@ -142,29 +142,29 @@ have expressed that this is undesirable. ### Wrapper Class oneDPL could provide some wrapper iterator `direct_iterator` which wraps an arbitrary base iterator and marks it as -passed directly. `direct_iterator` could utilize either of the above alternatives to accomplish this, and signal -that the iterator should be passed directly. It would need to pass through all operations to the wrapped base iterator, -and make sure no overhead is added in its usage. +"passed directly". `direct_iterator` could utilize either of the above alternatives to accomplish this and signal that +the iterator should be "passed directly". It would need to pass through all operations to the wrapped base iterator and +make sure no overhead is added in its usage. There is some complexity in adding such a wrapper iterator, and it would need to be considered carefully to make sure no -problems would be introduced. This wrapper class may obfuscate users types, and make them more unwieldy to use. It is -also less expressive than the other options in that it only has the ability to unilaterally mark a type as passed -directly. There is no logic that can be used to express some iterator type which may be conditionally passed directly, -other than to have logic to conditionally apply the wrapper in the first place. This option seems less clear and has -more opportunity to cause problems. + problems would be introduced. This wrapper class may obfuscate users' types and make them more unwieldy to use. It is + also less expressive than the other options in that it only has the ability to unilaterally mark a type as "passed + directly". There is no logic that can be used to express some iterator type which may be conditionally "passed + directly", other than to have logic to conditionally apply the wrapper in the first place. This option seems less clear + and has more opportunity to cause problems. ## Testing We will need a detailed test checking both positive and negative responses to -`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` have the expected result, with custom types and -combinations of iterators, usm pointers etc. +`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` to ensure they have the expected result. This should +include tests with custom types, combinations of iterators, USM pointers, etc. ## Open Questions * Is there a better, more concise name than `oneapi::dpl::is_passed_directly_in_onedpl_device_policies[_v]` that -properly conveys the meaning to the users? - * Other names proposed: - * `oneapi::dpl::onedpl_is_iterator_device_ready[_v]` - * `oneapi::dpl::is_passed_directly_to_sycl_backend[_v]` - * `oneapi::dpl::requires_explicit_data_transfer_onedpl_device_policies[_v]` (inverted) +* properly conveys the meaning to the users? + * Other names proposed: + * `oneapi::dpl::onedpl_is_iterator_device_ready[_v]` + * `oneapi::dpl::is_passed_directly_to_sycl_backend[_v]` + * `oneapi::dpl::requires_explicit_data_transfer_onedpl_device_policies[_v]` (inverted) * Where should this be located? - * Possible options include `oneapi/dpl/iterator`, `oneapi/dpl/type_traits`. \ No newline at end of file + * Possible options include `oneapi/dpl/iterator`, `oneapi/dpl/type_traits`. From 7837319957a7893072fc448748ee81877aa21810 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 11 Mar 2025 11:40:57 -0400 Subject: [PATCH 22/31] typo Signed-off-by: Dan Hoeflinger --- rfcs/proposed/iterators_pass_directly_customization/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index b65855a8118..e64122d7064 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -161,7 +161,7 @@ include tests with custom types, combinations of iterators, USM pointers, etc. ## Open Questions * Is there a better, more concise name than `oneapi::dpl::is_passed_directly_in_onedpl_device_policies[_v]` that -* properly conveys the meaning to the users? +properly conveys the meaning to the users? * Other names proposed: * `oneapi::dpl::onedpl_is_iterator_device_ready[_v]` * `oneapi::dpl::is_passed_directly_to_sycl_backend[_v]` From 79adf346bfc3e8ac87d8c2d498fd8407f03facd6 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Thu, 20 Mar 2025 11:05:47 -0400 Subject: [PATCH 23/31] edits for clarity Signed-off-by: Dan Hoeflinger --- .../README.md | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index e64122d7064..3207734d0cc 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -2,16 +2,21 @@ ## Introduction -oneDPL handles some types of input data automatically as input to its device backend (SYCL-based) as described -[here](https://uxlfoundation.github.io/oneDPL/parallel_api/pass_data_algorithms.html). Unified Shared Memory (USM) -pointers refer to data that is inherently device accessible, so no processing is required to pass this type of input -data to SYCL kernels. We refer to this trait as "passed directly". oneDPL also defines some rules for its provided -[iterator types](https://uxlfoundation.github.io/oneDPL/parallel_api/iterators.html) to be passed directly to SYCL -under some circumstances (usually based on their base types). - -Internally, these rules are defined with a trait `oneapi::dpl::__ranges::is_passed_directly` which evaluates to -`std::true_type` or `std::false_type` to indicate whether the iterator type `T` should be passed directly to SYCL -kernels. There exists an unofficial legacy `is_passed_directly` trait which types can define like this: +oneDPL processes data from some `std::vector::iterator` types automatically as input to its device backend (SYCL-based) +by implicitly copying data between host and device as described +[here](https://uxlfoundation.github.io/oneDPL/parallel_api/pass_data_algorithms.html). Other iterator types, like +Unified Shared Memory (USM) pointers, refer to data that is inherently device accessible, so no data transfers are +required from oneDPL to pass this data into oneDPL APIs with a device policy. When iterator types do not require oneDPL +to copy data between host and device as preparation for being passed into SYCL kernels, we refer to these iterator types +as having the trait "passed directly". oneDPL also defines some rules for its provided +[iterator types](https://uxlfoundation.github.io/oneDPL/parallel_api/iterators.html) to be "passed directly" to SYCL +under some circumstances (usually based on their base types). When iterators which are not "passed directly" are passed +in to oneDPL APIs with a device policy, there is an overhead involved to transfer data to and from the device. +Accurately marking iterator types which are "passed directly" is necessary to avoid this overhead. + +Internally, these rules are currently defined with a trait `oneapi::dpl::__ranges::is_passed_directly` which +evaluates to `std::true_type` or `std::false_type` to indicate whether the iterator type `T` should be passed directly +to SYCL kernels. There exists an unofficial legacy `is_passed_directly` trait which types can define like this: `using is_passed_directly = std::true_type;` which is supported within oneDPL. This method is currently used for a number of helper types within the SYCLomatic compatibility headers (`device_pointer`, `device_iterator`, `tagged_pointer`, `constant_iterator`, `iterator_adaptor`). There is no official public API for users who want to From 5d704ab0105cccb341a9d875e6e4fa62938bfcf8 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Thu, 20 Mar 2025 11:07:21 -0400 Subject: [PATCH 24/31] edits for clarity Signed-off-by: Dan Hoeflinger --- .../README.md | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 3207734d0cc..26db22dbced 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -1,30 +1,29 @@ # Passed Directly Customization Point for User Defined Types ## Introduction - -oneDPL processes data from some `std::vector::iterator` types automatically as input to its device backend (SYCL-based) -by implicitly copying data between host and device as described +oneDPL processes data from some `std::vector::iterator` types automatically as input to its SYCL-based device backend by +copying data between host and device, as described [here](https://uxlfoundation.github.io/oneDPL/parallel_api/pass_data_algorithms.html). Other iterator types, like -Unified Shared Memory (USM) pointers, refer to data that is inherently device accessible, so no data transfers are -required from oneDPL to pass this data into oneDPL APIs with a device policy. When iterator types do not require oneDPL -to copy data between host and device as preparation for being passed into SYCL kernels, we refer to these iterator types -as having the trait "passed directly". oneDPL also defines some rules for its provided -[iterator types](https://uxlfoundation.github.io/oneDPL/parallel_api/iterators.html) to be "passed directly" to SYCL -under some circumstances (usually based on their base types). When iterators which are not "passed directly" are passed -in to oneDPL APIs with a device policy, there is an overhead involved to transfer data to and from the device. -Accurately marking iterator types which are "passed directly" is necessary to avoid this overhead. - -Internally, these rules are currently defined with a trait `oneapi::dpl::__ranges::is_passed_directly` which -evaluates to `std::true_type` or `std::false_type` to indicate whether the iterator type `T` should be passed directly -to SYCL kernels. There exists an unofficial legacy `is_passed_directly` trait which types can define like this: -`using is_passed_directly = std::true_type;` which is supported within oneDPL. This method is currently used for a -number of helper types within the SYCLomatic compatibility headers (`device_pointer`, `device_iterator`, -`tagged_pointer`, `constant_iterator`, `iterator_adaptor`). There is no official public API for users who want to -create their own iterator types that could be passed directly to SYCL kernels. This is a gap that should be filled -with an official public API. - -Without something like this, users are forced to rely only upon our provided iterator types or reach into implementation -details that are not part of oneDPL's specified interface. +Unified Shared Memory (USM) pointers, refer to data inherently accessible by the device, so no data transfers are +required when passed into oneDPL APIs with a device policy. Iterators that do not require such data transfers are said +to have the "passed directly" trait. + +oneDPL also defines rules for its provided +[iterator types](https://uxlfoundation.github.io/oneDPL/parallel_api/iterators.html) to be "passed directly" under +certain conditions, often based on their base types. When iterators lacking the "passed directly" trait are passed into +oneDPL APIs with a device policy, additional overhead is incurred to transfer data to and from the device. Properly +marking iterator types as "passed directly" is essential to avoid this overhead. + +Internally, these rules are defined using the trait `oneapi::dpl::__ranges::is_passed_directly`, which evaluates to +`std::true_type` or `std::false_type` to indicate whether the iterator type `T` should be passed directly to SYCL +kernels, rather than copied to a SYCL buffer first to enable a transfer to and from the device. A legacy +`is_passed_directly` trait exists, allowing types to define `using is_passed_directly = std::true_type;`. This legacy +method is used for helper types in SYCLomatic compatibility headers (`device_pointer`, `device_iterator`, +`tagged_pointer`, `constant_iterator`, `iterator_adaptor`). However, there is no official public API for users to define +custom iterator types that can be passed directly to SYCL kernels. + +This gap should be addressed with an official public API. Without it, users must rely on oneDPL's provided iterator +types or access implementation details outside the specified interface. ## Proposal ### High Level Proposal From 91b77a2842d1a59815e1d55762035e58832d6fc3 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Thu, 20 Mar 2025 11:36:01 -0400 Subject: [PATCH 25/31] added proposed names Signed-off-by: Dan Hoeflinger --- rfcs/proposed/iterators_pass_directly_customization/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 26db22dbced..63aadd77e5f 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -170,5 +170,7 @@ properly conveys the meaning to the users? * `oneapi::dpl::onedpl_is_iterator_device_ready[_v]` * `oneapi::dpl::is_passed_directly_to_sycl_backend[_v]` * `oneapi::dpl::requires_explicit_data_transfer_onedpl_device_policies[_v]` (inverted) + * `oneapi::dpl::is_passed_directly_to_device[_v]` + * `oneapi::dpl::is_passed_directly[_v]` * Where should this be located? * Possible options include `oneapi/dpl/iterator`, `oneapi/dpl/type_traits`. From 0474d92894bf7b49809799b38ec493eee46abd04 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Thu, 20 Mar 2025 11:38:36 -0400 Subject: [PATCH 26/31] adding note about naming Signed-off-by: Dan Hoeflinger --- rfcs/proposed/iterators_pass_directly_customization/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 63aadd77e5f..7815e82b58f 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -172,5 +172,8 @@ properly conveys the meaning to the users? * `oneapi::dpl::requires_explicit_data_transfer_onedpl_device_policies[_v]` (inverted) * `oneapi::dpl::is_passed_directly_to_device[_v]` * `oneapi::dpl::is_passed_directly[_v]` + * We could consider also not using the same name for the public trait as the ADL customization point. The + customization point function will be used in the user's namespace, so it has need to "identify" onedpl with its + name, but in the public trait, adding a similar identifier will be redundant with `oneapi::dpl`. * Where should this be located? * Possible options include `oneapi/dpl/iterator`, `oneapi/dpl/type_traits`. From 29a8374774948519a151e4ad4484e190afa8a520 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Fri, 21 Mar 2025 09:03:59 -0400 Subject: [PATCH 27/31] separating trait name and ADL name, other feedback addressed Signed-off-by: Dan Hoeflinger --- .../README.md | 74 ++++++++++++------- 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 7815e82b58f..56c0606e812 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -30,13 +30,25 @@ types or access implementation details outside the specified interface. We will create an Argument-Dependent Lookup (ADL) customization point which defines if iterators are "passed directly" `is_passed_directly_in_onedpl_device_policies`. Users may define if their iterator types should be "passed directly" by -oneDPL by defining a constexpr function in the same namespace where the iterator is defined. +oneDPL by defining a `constexpr` function in the same namespace where the iterator is defined. `is_passed_directly_in_onedpl_device_policies` must accept a const lvalue reference to the iterator type being specialized and return `std::true_type{}` if the iterator type is "passed directly" and `std::false_type{}` otherwise. Additionally, oneDPL will provide a public trait, -`inline constexpr bool oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v`, indicating if the iterator type -`T` is "passed directly". This public trait is intended to be used to help define +```cpp +namespace oneapi +{ +namespace dpl +{ +template +struct is_passed_directly_to_device; // std::true_type or std::false_type + +template +inline constexpr bool is_passed_directly_to_device_v = oneapi::dpl::is_passed_directly_to_device::value; +} // dpl +} // oneapi +``` +indicating if the iterator type `T` is "passed directly". This public trait is intended to be used to help define `is_passed_directly_in_onedpl_device_policies` for wrapper iterator types which depend on the "passed directly" status of their base iterator(s) using only the base iterator type(s) rather than a named instance. It may also be used to confirm user iterator types "passed directly" traits are as intended to prevent unnecessary overhead in oneDPL calls. @@ -58,10 +70,10 @@ oneDPL will define the "passed directly" definitions of its custom iterators as ### Implementation Details -When using device policies, oneDPL will run compile-time checks on argument iterator types by calling -`is_passed_directly_in_onedpl_device_policies`. If `std::true_type{}` is returned, oneDPL will pass the iterator -directly to SYCL kernels rather than copying the data into `sycl::buffers` and using those buffers to transfer data to -SYCL kernels. +When using device policies, oneDPL will run compile-time checks on argument iterator types by using `decltype` to +determine the return type of `is_passed_directly_in_onedpl_device_policies` when called with iterator as argument. If +`std::true_type{}` is returned, oneDPL will pass the iterator directly to SYCL kernels rather than copying the data into +`sycl::buffers` and using those buffers to transfer data to SYCL kernels. The specification and implementation will be prepared once this RFC is accepted as "proposed". We do not intend to offer this first as experimental. This RFC will target "Supported" once the specification and implementation are accepted. @@ -70,7 +82,7 @@ this first as experimental. This RFC will target "Supported" once the specificat Below is a simple example of an iterator and ADL customization point definition which is always "passed directly". -``` +```cpp namespace user { @@ -88,13 +100,13 @@ namespace user } //namespace user ``` -Users can use any `constexpr` logic based on their iterator to determine if the iterator can be passed directly into a -SYCL kernel without any processing. Below is an example of a type which contains a pair of iterators and should be -treated as passed directly if and only if both base iterators are also passed directly. Note that the use of the public -trait enables the use of the base iterator type alone without creating a named instance of the base iterator to pass -into the ADL. +Users can use any logic based on their iterator type to determine if it can be "passed directly", but the logic must be +`constexpr`. Commonly, this will involve some check of base template types and their iterator type's "passed directly" +status. Below is an example of a type that contains a pair of iterators and should be treated as "passed directly" if +and only if both base iterators are also "passed directly". Note that the use of the public trait enables the use of the +base iterator type alone without creating a named instance of the base iterator to pass into the ADL. -``` +```cpp namespace user { template @@ -108,14 +120,23 @@ namespace user constexpr auto is_passed_directly_in_onedpl_device_policies(const iterator_pair&) { - if constexpr (oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v && - oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v) + if constexpr (oneapi::dpl::is_passed_directly_to_device_v && + oneapi::dpl::is_passed_directly_to_device_v) return std::true_type{}; else return std::false_type{}; } } //namespace user ``` +It is also possible to write overloads without a body using an `auto` trailing return type. The following is an +alternative written in this way for the previous example: +```cpp + template + constexpr + auto is_passed_directly_in_onedpl_device_policies(const iterator_pair&) -> + std::bool_constant<(oneapi::dpl::is_passed_directly_to_device_v && + oneapi::dpl::is_passed_directly_to_device_v)>; +``` ## Alternatives Considered ### Public Trait Struct Explicit Specialization @@ -159,21 +180,22 @@ There is some complexity in adding such a wrapper iterator, and it would need to ## Testing We will need a detailed test checking both positive and negative responses to -`oneapi::dpl::is_passed_directly_in_onedpl_device_policies_v` to ensure they have the expected result. This should +`oneapi::dpl::is_passed_directly_to_device_v` to ensure they have the expected result. This should include tests with custom types, combinations of iterators, USM pointers, etc. ## Open Questions -* Is there a better, more concise name than `oneapi::dpl::is_passed_directly_in_onedpl_device_policies[_v]` that +* How should we name the (1) ADL customization point and (2) public trait and value in a way that is concise and properly conveys the meaning to the users? * Other names proposed: - * `oneapi::dpl::onedpl_is_iterator_device_ready[_v]` - * `oneapi::dpl::is_passed_directly_to_sycl_backend[_v]` - * `oneapi::dpl::requires_explicit_data_transfer_onedpl_device_policies[_v]` (inverted) - * `oneapi::dpl::is_passed_directly_to_device[_v]` - * `oneapi::dpl::is_passed_directly[_v]` - * We could consider also not using the same name for the public trait as the ADL customization point. The - customization point function will be used in the user's namespace, so it has need to "identify" onedpl with its - name, but in the public trait, adding a similar identifier will be redundant with `oneapi::dpl`. + * `oneapi::dpl::onedpl_is_iterator_device_ready` + * `oneapi::dpl::is_passed_directly_to_sycl_backend` + * `oneapi::dpl::requires_explicit_data_transfer_onedpl_device_policies` (inverted) + * `oneapi::dpl::is_passed_directly_to_device` + * `oneapi::dpl::is_passed_directly` + * This RFC provides different names for the (1) and (2). (1) will be used in the user's namespace, so it needs to + identify onedpl with its name, but (2) will be used from `oneapi::dpl` so including onedpl in the name is + redundant. Additionally, separating the names of (1) and (2) allows us to provide both a struct and value version + of the public trait, which is the norm. These decisions could be reconsidered in the specification PR. * Where should this be located? * Possible options include `oneapi/dpl/iterator`, `oneapi/dpl/type_traits`. From d45e799da9d0289cae08772c49ed1805cbb7ca5e Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 24 Mar 2025 16:59:42 -0400 Subject: [PATCH 28/31] Address feedback, add comments on derived classes Signed-off-by: Dan Hoeflinger --- .../README.md | 85 ++++++++++++------- 1 file changed, 55 insertions(+), 30 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 56c0606e812..945789dee18 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -30,21 +30,23 @@ types or access implementation details outside the specified interface. We will create an Argument-Dependent Lookup (ADL) customization point which defines if iterators are "passed directly" `is_passed_directly_in_onedpl_device_policies`. Users may define if their iterator types should be "passed directly" by -oneDPL by defining a `constexpr` function in the same namespace where the iterator is defined. +oneDPL by defining a function in the same namespace where the iterator is defined. `is_passed_directly_in_onedpl_device_policies` must accept a const lvalue reference to the iterator type being specialized and return `std::true_type{}` if the iterator type is "passed directly" and `std::false_type{}` otherwise. +oneDPL will not call the `is_passed_directly_in_onedpl_device_policies` function at runtime. It will merely capture its +return type based upon the argument iterator type passed. -Additionally, oneDPL will provide a public trait, +Additionally, oneDPL will provide a public trait, `is_passed_directly_to_device[_v]` as follows: ```cpp namespace oneapi { namespace dpl { template -struct is_passed_directly_to_device; // std::true_type or std::false_type +struct is_passed_directly_to_device; // must have the characteristics of std::true_type or of std::false_type template -inline constexpr bool is_passed_directly_to_device_v = oneapi::dpl::is_passed_directly_to_device::value; +inline constexpr bool is_passed_directly_to_device_v = oneapi::dpl::is_passed_directly_to_device::value; } // dpl } // oneapi ``` @@ -68,6 +70,12 @@ oneDPL will define the "passed directly" definitions of its custom iterators as * `transform_iterator` is "passed directly" if its source iterator is "passed directly" * `permutation_iterator` is "passed directly" if both its source iterator and its index map are "passed directly" +If a "passed directly" customization point is defined for a type, any derived type will also match the existing +ADL customization point function if a more specific one is not defined. This can be a feature, as multiple derived types +can inherit "passed directly" traits from their parent. However, users must be aware that this is the case. A user could +incorrectly assume that the lack of a customization point for the derived class would mean the new type will use the +default overload of `is_passed_directly_in_onedpl_device_policies` (and return `std::false_type`). + ### Implementation Details When using device policies, oneDPL will run compile-time checks on argument iterator types by using `decltype` to @@ -91,8 +99,7 @@ namespace user /* unspecified user definition */ }; - constexpr - auto + std::true_type is_passed_directly_in_onedpl_device_policies(const my_passed_directly_iterator&) { return std::true_type{}; @@ -100,11 +107,11 @@ namespace user } //namespace user ``` -Users can use any logic based on their iterator type to determine if it can be "passed directly", but the logic must be -`constexpr`. Commonly, this will involve some check of base template types and their iterator type's "passed directly" -status. Below is an example of a type that contains a pair of iterators and should be treated as "passed directly" if -and only if both base iterators are also "passed directly". Note that the use of the public trait enables the use of the -base iterator type alone without creating a named instance of the base iterator to pass into the ADL. +Users can use any compile-time logic based on their iterator type to determine if it can be "passed directly" to define +the ADL function's return type. Commonly, this will involve some check of base template types and their "passed +directly" trait status. Below is an example of a type that contains a pair of iterators and should be treated as "passed +directly" if and only if both base iterators are also "passed directly". Note that the use of the public trait enables +the use of the base iterator type alone without creating a named instance of the base iterator to pass into the ADL. ```cpp namespace user @@ -117,25 +124,40 @@ namespace user }; template - constexpr auto is_passed_directly_in_onedpl_device_policies(const iterator_pair&) { - if constexpr (oneapi::dpl::is_passed_directly_to_device_v && - oneapi::dpl::is_passed_directly_to_device_v) - return std::true_type{}; - else - return std::false_type{}; + return std::conjunction, + oneapi::dpl::is_passed_directly_to_device>{}; } } //namespace user ``` + It is also possible to write overloads without a body using an `auto` trailing return type. The following is an alternative written in this way for the previous example: + ```cpp template - constexpr + auto is_passed_directly_in_onedpl_device_policies(const iterator_pair&) -> - std::bool_constant<(oneapi::dpl::is_passed_directly_to_device_v && - oneapi::dpl::is_passed_directly_to_device_v)>; + std::conjunction, + oneapi::dpl::is_passed_directly_to_device>>; +``` +Finally, it is possible to write overloads using the "hidden friend" idiom as functions with or without a body inside +the scope of the iterator definition itself. This option may be preferred when a user wants to ensure that this "passed +directly" trait of their iterator is coupled tightly with the definition itself for maintenance. It also has the +advantage of only being making available for ADL the combinations of template arguments for this iterator which are +explicitly instantiated in the code already. + +```cpp + template + struct iterator_pair + { + It1 first; + It2 second; + auto is_passed_directly_in_onedpl_device_policies(const iterator_pair&) -> + std::conjunction, + oneapi::dpl::is_passed_directly_to_device>; + }; ``` ## Alternatives Considered @@ -143,14 +165,14 @@ alternative written in this way for the previous example: oneDPL could make public our internal structure `oneapi::dpl::__ranges::is_passed_directly` as `oneapi::dpl::is_passed_directly` for users to specialize to define rules for their types. This would be a similar mechanism to `sycl::is_device_copyable`. The implementation details of this option should avoid some complexity required - to properly implement the customization point. +to properly implement the customization point. However, as we have learned from experience within oneDPL, explicit specialization of a structure in another library's namespace creates maintenance problems. It either requires lots of closing of nested namespaces, opening of the external - library's namespace for the specialization, or it requires separating these specializations to a separate location - removed from the types they are specializing for. oneDPL has chosen to use the latter, which can be seen in - `include/oneapi/dpl/pstl/hetero/dpcpp/sycl_traits.h`. This has led to several errors where changes to structures should - have included changes to sycl_traits but did not, and needed to be fixed later. +library's namespace for the specialization, or it requires separating these specializations to a separate location +removed from the types they are specializing for. oneDPL has chosen to use the latter, which can be seen in +`include/oneapi/dpl/pstl/hetero/dpcpp/sycl_traits.h`. This has led to several errors where changes to structures should +have included changes to sycl_traits but did not, and needed to be fixed later. In an effort to avoid this same issue for our users, we propose a similar method but instead with a constexpr customization point, allowing the user to override that customization point within their own namespace as a free @@ -172,11 +194,11 @@ the iterator should be "passed directly". It would need to pass through all oper make sure no overhead is added in its usage. There is some complexity in adding such a wrapper iterator, and it would need to be considered carefully to make sure no - problems would be introduced. This wrapper class may obfuscate users' types and make them more unwieldy to use. It is - also less expressive than the other options in that it only has the ability to unilaterally mark a type as "passed - directly". There is no logic that can be used to express some iterator type which may be conditionally "passed - directly", other than to have logic to conditionally apply the wrapper in the first place. This option seems less clear - and has more opportunity to cause problems. +problems would be introduced. This wrapper class may obfuscate users' types and make them more unwieldy to use. It is +also less expressive than the other options in that it only has the ability to unilaterally mark a type as "passed +directly". There is no logic that can be used to express some iterator type which may be conditionally "passed +directly", other than to have logic to conditionally apply the wrapper in the first place. This option seems less clear +and has more opportunity to cause problems. ## Testing We will need a detailed test checking both positive and negative responses to @@ -199,3 +221,6 @@ properly conveys the meaning to the users? of the public trait, which is the norm. These decisions could be reconsidered in the specification PR. * Where should this be located? * Possible options include `oneapi/dpl/iterator`, `oneapi/dpl/type_traits`. +* What possibilities for problems / advantages exist for ADL matching for derived-from types, etc.? + * This can lead to ambiguity when deriving from multiple classes, but that can then be differentiated by implementing + a more specific ADL customization point function for the derived class. From 5305507b2e749a36c57e7733a89a33f64ad8a34a Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 24 Mar 2025 17:06:04 -0400 Subject: [PATCH 29/31] improve language Signed-off-by: Dan Hoeflinger --- .../iterators_pass_directly_customization/README.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 945789dee18..5f5eecb1c91 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -71,10 +71,13 @@ oneDPL will define the "passed directly" definitions of its custom iterators as * `permutation_iterator` is "passed directly" if both its source iterator and its index map are "passed directly" If a "passed directly" customization point is defined for a type, any derived type will also match the existing -ADL customization point function if a more specific one is not defined. This can be a feature, as multiple derived types -can inherit "passed directly" traits from their parent. However, users must be aware that this is the case. A user could -incorrectly assume that the lack of a customization point for the derived class would mean the new type will use the -default overload of `is_passed_directly_in_onedpl_device_policies` (and return `std::false_type`). +customization point function unless explicitly overridden. Users can override this behavior by implementing a more +specific ADL customization point function for the derived class. This is particularly useful in cases of multiple +inheritance or ambiguous ADL matches, where the default behavior may not align with the intended design. + +Users must be aware of this behavior. A user might incorrectly assume that the absence of a customization point +specifically for the derived class would cause the derived iterator type to use the default overload of +`is_passed_directly_in_onedpl_device_policies` (and return `std::false_type`). ### Implementation Details From 942b19c15a0c2d99d55bcf34556b9bb993b316a0 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 24 Mar 2025 17:24:30 -0400 Subject: [PATCH 30/31] fix missing `friend` Signed-off-by: Dan Hoeflinger --- rfcs/proposed/iterators_pass_directly_customization/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 5f5eecb1c91..49deb19ba60 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -157,7 +157,7 @@ explicitly instantiated in the code already. { It1 first; It2 second; - auto is_passed_directly_in_onedpl_device_policies(const iterator_pair&) -> + friend auto is_passed_directly_in_onedpl_device_policies(const iterator_pair&) -> std::conjunction, oneapi::dpl::is_passed_directly_to_device>; }; From 73a803856db68a5659f58dfe74ab70635cb30dc0 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Tue, 25 Mar 2025 08:24:16 -0400 Subject: [PATCH 31/31] removing extra word Signed-off-by: Dan Hoeflinger --- rfcs/proposed/iterators_pass_directly_customization/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/proposed/iterators_pass_directly_customization/README.md b/rfcs/proposed/iterators_pass_directly_customization/README.md index 49deb19ba60..db35b6af1ce 100644 --- a/rfcs/proposed/iterators_pass_directly_customization/README.md +++ b/rfcs/proposed/iterators_pass_directly_customization/README.md @@ -148,8 +148,8 @@ alternative written in this way for the previous example: Finally, it is possible to write overloads using the "hidden friend" idiom as functions with or without a body inside the scope of the iterator definition itself. This option may be preferred when a user wants to ensure that this "passed directly" trait of their iterator is coupled tightly with the definition itself for maintenance. It also has the -advantage of only being making available for ADL the combinations of template arguments for this iterator which are -explicitly instantiated in the code already. +advantage of only making available for ADL the combinations of template arguments for this iterator which are explicitly +instantiated in the code already. ```cpp template