-
Notifications
You must be signed in to change notification settings - Fork 796
[SYCL] Add structure to pass references to ranges #19805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: sycl
Are you sure you want to change the base?
Changes from 5 commits
9571fec
2521805
1a90e48
38945f3
d4c267e
0d76be9
858bf21
95beb6a
f2e347d
71e2eee
7a3f269
e6ab5e7
d309440
c7bc868
2754978
d635399
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| //==---- ranges_ref_view.hpp --- SYCL iteration with reference to ranges ---==// | ||
| // | ||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <sycl/nd_range.hpp> | ||
|
|
||
| namespace sycl { | ||
| inline namespace _V1 { | ||
| namespace detail { | ||
|
|
||
| // The structure to keep dimension and references to ranges unified for | ||
| // all dimensions. | ||
| class ranges_ref_view { | ||
|
|
||
| public: | ||
| ranges_ref_view() = default; | ||
| ranges_ref_view(const ranges_ref_view &Desc) = default; | ||
| ranges_ref_view(ranges_ref_view &&Desc) = default; | ||
|
|
||
| template <int Dims_> | ||
| ranges_ref_view(sycl::range<Dims_> &GlobalSizes, | ||
| sycl::range<Dims_> &LocalSizes) | ||
| : GlobalSize(&(GlobalSizes[0])), LocalSize(&(LocalSizes[0])), | ||
| Dims{size_t(Dims_)} {} | ||
|
|
||
| // to support usage in sycl::ext::oneapi::experimental::submit_with_event() | ||
| template <int Dims_> | ||
| ranges_ref_view(sycl::nd_range<Dims_> &ExecutionRange) | ||
| : GlobalSize(&ExecutionRange.globalSize[0]), | ||
| LocalSize(&ExecutionRange.localSize[0]), | ||
| GlobalOffset(&ExecutionRange.offset[0]), Dims{size_t(Dims_)} {} | ||
|
|
||
| template <int Dims_> | ||
| ranges_ref_view(sycl::range<Dims_> &Range) | ||
| : GlobalSize(&(Range[0])), Dims{size_t(Dims_)} {} | ||
|
|
||
| ranges_ref_view &operator=(const ranges_ref_view &Desc) = default; | ||
| ranges_ref_view &operator=(ranges_ref_view &&Desc) = default; | ||
vinser52 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| const size_t *GlobalSize = nullptr; | ||
| const size_t *LocalSize = nullptr; | ||
| const size_t *GlobalOffset = nullptr; | ||
| size_t Dims = 0; | ||
| }; | ||
|
|
||
| } // namespace detail | ||
| } // namespace _V1 | ||
| } // namespace sycl | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| //==---- RangesRefViewUsage.cpp --- Check ranges_ref_view ------------------==// | ||
| // | ||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
| #include <sycl/detail/ranges_ref_view.hpp> | ||
|
|
||
| #include <gtest/gtest.h> | ||
|
|
||
| TEST(RangesRefUsage, RangesRefUsage) { | ||
|
||
| sycl::detail::ranges_ref_view r0; | ||
| ASSERT_EQ(r0.Dims, size_t{0}); | ||
|
|
||
| { | ||
| sycl::range<1> global_range{1024}; | ||
| sycl::range<1> local_range{64}; | ||
| sycl::id<1> offset{10}; | ||
| sycl::nd_range<1> nd_range{global_range, local_range, offset}; | ||
|
|
||
| { | ||
| sycl::detail::ranges_ref_view r{nd_range}; | ||
| ASSERT_EQ(r.Dims, size_t{1}); | ||
| ASSERT_EQ(*r.GlobalSize, global_range[0]); | ||
| ASSERT_EQ(*r.LocalSize, local_range[0]); | ||
| ASSERT_EQ(*r.GlobalOffset, offset[0]); | ||
| } | ||
| { | ||
| sycl::detail::ranges_ref_view r{global_range, local_range}; | ||
| ASSERT_EQ(r.Dims, size_t{1}); | ||
| ASSERT_EQ(*r.GlobalSize, global_range[0]); | ||
| ASSERT_EQ(*r.LocalSize, local_range[0]); | ||
| } | ||
| { | ||
| sycl::detail::ranges_ref_view r{global_range}; | ||
| ASSERT_EQ(r.Dims, size_t{1}); | ||
| ASSERT_EQ(*r.GlobalSize, global_range[0]); | ||
| ASSERT_EQ(r.LocalSize, nullptr); | ||
| } | ||
| } | ||
| { | ||
| sycl::range<2> global_range{1024, 512}; | ||
| sycl::range<2> local_range{64, 32}; | ||
| sycl::id<2> offset{10, 20}; | ||
| sycl::nd_range<2> nd_range{global_range, local_range, offset}; | ||
|
|
||
| { | ||
| sycl::detail::ranges_ref_view r{nd_range}; | ||
| ASSERT_EQ(r.Dims, size_t{2}); | ||
| ASSERT_EQ(r.GlobalSize[0], global_range[0]); | ||
| ASSERT_EQ(r.GlobalSize[1], global_range[1]); | ||
| ASSERT_EQ(r.LocalSize[0], local_range[0]); | ||
| ASSERT_EQ(r.LocalSize[1], local_range[1]); | ||
| ASSERT_EQ(r.GlobalOffset[0], offset[0]); | ||
| ASSERT_EQ(r.GlobalOffset[1], offset[1]); | ||
| } | ||
| { | ||
| sycl::detail::ranges_ref_view r{global_range, local_range}; | ||
| ASSERT_EQ(r.Dims, size_t{2}); | ||
| ASSERT_EQ(r.GlobalSize[0], global_range[0]); | ||
| ASSERT_EQ(r.GlobalSize[1], global_range[1]); | ||
| ASSERT_EQ(r.LocalSize[0], local_range[0]); | ||
| ASSERT_EQ(r.LocalSize[1], local_range[1]); | ||
| } | ||
| { | ||
| sycl::detail::ranges_ref_view r{global_range}; | ||
| ASSERT_EQ(r.Dims, size_t{2}); | ||
| ASSERT_EQ(r.GlobalSize[0], global_range[0]); | ||
| ASSERT_EQ(r.GlobalSize[1], global_range[1]); | ||
| ASSERT_EQ(r.LocalSize, nullptr); | ||
| } | ||
| } | ||
| { | ||
| sycl::range<3> global_range{1024, 512, 256}; | ||
| sycl::range<3> local_range{64, 32, 16}; | ||
| sycl::id<3> offset{10, 20, 30}; | ||
| sycl::nd_range<3> nd_range{global_range, local_range, offset}; | ||
|
|
||
| { | ||
| sycl::detail::ranges_ref_view r{nd_range}; | ||
| ASSERT_EQ(r.Dims, size_t{3}); | ||
| ASSERT_EQ(r.GlobalSize[0], global_range[0]); | ||
| ASSERT_EQ(r.GlobalSize[1], global_range[1]); | ||
| ASSERT_EQ(r.GlobalSize[2], global_range[2]); | ||
| ASSERT_EQ(r.LocalSize[0], local_range[0]); | ||
| ASSERT_EQ(r.LocalSize[1], local_range[1]); | ||
| ASSERT_EQ(r.LocalSize[2], local_range[2]); | ||
| ASSERT_EQ(r.GlobalOffset[0], offset[0]); | ||
| ASSERT_EQ(r.GlobalOffset[1], offset[1]); | ||
| ASSERT_EQ(r.GlobalOffset[2], offset[2]); | ||
| } | ||
| { | ||
| sycl::detail::ranges_ref_view r{global_range, local_range}; | ||
| ASSERT_EQ(r.Dims, size_t{3}); | ||
| ASSERT_EQ(r.GlobalSize[0], global_range[0]); | ||
| ASSERT_EQ(r.GlobalSize[1], global_range[1]); | ||
| ASSERT_EQ(r.GlobalSize[2], global_range[2]); | ||
| ASSERT_EQ(r.LocalSize[0], local_range[0]); | ||
| ASSERT_EQ(r.LocalSize[1], local_range[1]); | ||
| ASSERT_EQ(r.LocalSize[2], local_range[2]); | ||
| } | ||
| { | ||
| sycl::detail::ranges_ref_view r{global_range}; | ||
| ASSERT_EQ(r.Dims, size_t{3}); | ||
| ASSERT_EQ(r.GlobalSize[0], global_range[0]); | ||
| ASSERT_EQ(r.GlobalSize[1], global_range[1]); | ||
| ASSERT_EQ(r.GlobalSize[2], global_range[2]); | ||
| ASSERT_EQ(r.LocalSize, nullptr); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure that it is the best name. We have
sycl::rangeand someone might assume thatranges_ref_viewis a view to thesycl::range. Also it is not lear what_ref_means in the name. Why not justnd_range_view?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
View has strong connotation to views from C++20, that may be or may be not good.
Alternatively, it can be
nd_range_ref. I don't know.That's for "reference", I hope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote for
nd_range_view.@sergey-semenov, @aelovikov-intel, @slawekptak what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll copy my sketch from the offline group chat:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nd_range_view sounds good to me. Maybe we should change GlobalOffset to Offset, so the naming fully reflects the nd_range naming?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aelovikov-intel , Could you please clarify what is the problem are you solving? All current implementation can do is to consume
sycl::rangesornd_rangeand to producesycl::detail::NDRDescT. Probably, you are thinking about wider context.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, renamed to
nd_range_view.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More code unification, less than 4 pointer-sized data members for the "view", ability to pass existing
sycl::[nd_]range<N>asconst sycl::detail::[nd_]range_base &directly to all the helper function unifying all Dims handling.Do you have subsequent refactoring changes available? POC/draft quality would be fine. Are you going to switch all ABI entry points to accept the view?