[SYCL] Pull oneDPL tuple to use in reduction implementation #3481
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
v-klochkov
force-pushed
the
public_vklochkov_reduction_detail_tuple
branch
2 times, most recently
from
bd85aa1 to
87eeacb
Compare
v-klochkov
force-pushed
the
public_vklochkov_reduction_detail_tuple
branch
3 times, most recently
from
eccc202 to
102ec23
Compare
Using sycl::detail::tuple is a temporary work-around for various problems caused by using std::tuple: a) reduction using std::tuple cannot be compiled on Windows because std::tuple cannot be copied to DEVICE. b) internal error in level_zero RT. The new sycl::detail::tuple class is a very limited version of oneDPL's implementation of tuple. It includes such functionality: - convert from std::tuple and to std::tuple - tie(), get<I>(), tuple_element, make_tuple This change enables parallel_for() with number of reductions more than 1 for level_zero and for Windows. Signed-off-by: Vyacheslav N Klochkov <[email protected]>
v-klochkov
force-pushed
the
public_vklochkov_reduction_detail_tuple
branch
from
102ec23 to
64aece7
Compare
v-klochkov
changed the title
Pennycook
reviewed
Apr 3, 2021
| const std::tuple<ReducerT...> &Reducers, | ||
| const std::tuple<ResultT...> Identities, | ||
| ReduTupleT<ResultT...> Identities, |
There was a problem hiding this comment.
Did you mean to drop const here?
| const std::tuple<ResultT...> Identities, | ||
| ReduTupleT<InputAccT...> LocalAccs, | ||
| ReduTupleT<LocalAccT...> InputAccs, | ||
| ReduTupleT<ResultT...> Identities, |
There was a problem hiding this comment.
Same question about const here.
There was a problem hiding this comment.
The tuple is passed by value here, so 'const' does not have any effect, right?
There was a problem hiding this comment.
Good question, I'm not sure! I just wanted to check if it was deliberate, because you did more than just change the type.
There was a problem hiding this comment.
The tuple is passed by value here, so 'const' does not have any effect, right?
Both versions are equivalent from the caller POV, but const protects from modifying the value within the callee scope if needed.
romanovvlad
approved these changes
Apr 5, 2021
| @@ -30,6 +31,15 @@ using cl::sycl::detail::is_sgeninteger; | |||
| using cl::sycl::detail::queue_impl; | |||
| using cl::sycl::detail::remove_AS; | |||
|
|
|||
| // std::tuple seems to be a) too heavy and b) not copyable to device now | |||
There was a problem hiding this comment.
Could you please clarify what specifically makes std::tuple heavy?
There was a problem hiding this comment.
I'll eventually have to do that. At this moment I only know that SPIR-V file for a reduction test with std::tuple is 34% bigger than SPIRV for the same reduction test with sycl::detail::tuple (i.e. with this change).
Pennycook
approved these changes
Apr 5, 2021
v-klochkov
added a commit
to v-klochkov/llvm-test-suite
that referenced
this pull request
Apr 7, 2021
…y property The corresponding changes in SYCL RT: intel/llvm#3410 intel/llvm#3481 Signed-off-by: Vyacheslav N Klochkov <[email protected]>
v-klochkov
added a commit
to intel/llvm-test-suite
that referenced
this pull request
Apr 8, 2021
* [SYCL] Add test cases for SYCL2020 reductions + initialize_to_identity property The corresponding changes in SYCL RT: intel/llvm#3410 intel/llvm#3481 This patch - adds checks for SYCL-2020 reductions with initialize_to_identity property (#3410) - enables the test reduction_nd_N_vars.cpp for level-zero (#3481) - reduces some of tests by eliminating unneeded/duplicated checks. - removes the test reduction_transparent.cpp as useless because transparent operators already checked in many other tests. - reduces runtime of reduction tests by about 200x in average by re-using queue created once instead of creating it in each of test-cases Signed-off-by: Vyacheslav N Klochkov <[email protected]>
aelovikov-intel
pushed a commit
to aelovikov-intel/llvm
that referenced
this pull request
Mar 27, 2023
) * [SYCL] Add test cases for SYCL2020 reductions + initialize_to_identity property The corresponding changes in SYCL RT: intel#3410 intel#3481 This patch - adds checks for SYCL-2020 reductions with initialize_to_identity property (intel/llvm-test-suite#3410) - enables the test reduction_nd_N_vars.cpp for level-zero (intel/llvm-test-suite#3481) - reduces some of tests by eliminating unneeded/duplicated checks. - removes the test reduction_transparent.cpp as useless because transparent operators already checked in many other tests. - reduces runtime of reduction tests by about 200x in average by re-using queue created once instead of creating it in each of test-cases Signed-off-by: Vyacheslav N Klochkov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using sycl::detail::tuple is a temporary work-around for various
problems caused by using std::tuple:
a) reduction using std::tuple cannot be compiled on Windows because
std::tuple cannot be copied to DEVICE.
b) internal error in level_zero RT.
The new sycl::detail::tuple class is a very limited version
of oneDPL's implementation of tuple. It includes such functionality:
This change enables parallel_for() with number of reductions
more than 1 for level_zero and for Windows.
The corresponding changes in LIT tests: intel/llvm-test-suite#194
Signed-off-by: Vyacheslav N Klochkov [email protected]