Skip to content
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

[NFC][SYCL] Break include's cycle #1770

Merged
merged 8 commits into from
Jun 2, 2020

Conversation

s-kanaev
Copy link
Contributor

@s-kanaev s-kanaev commented May 28, 2020

This changes is required for #1747.

Suppose we add #include <CL/sycl/accessor.hpp> in interop_handle.hpp . Furthermore we will add #include <CL/sycl/interop_handle.hpp> to include/CL/sycl/handler.hpp. Then, we will have the following cycle (here -> says which includes):

handler.hpp -> interop_handle.hpp -> accessor.hpp -> handler.hpp (cycle here)

Also, this patch eliminates use of reinterpret_cast (either as it is or C-style) when converting accessor to AccessorBaseHost at host.

Sergey Kanaev added 4 commits May 28, 2020 11:08
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
turinevgeny
turinevgeny previously approved these changes May 28, 2020
Copy link
Contributor

@turinevgeny turinevgeny left a comment

Choose a reason for hiding this comment

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

LGTM.

sycl/CMakeLists.txt Outdated Show resolved Hide resolved
sycl/include/CL/sycl/detail/cg_types.hpp Show resolved Hide resolved
sycl/include/CL/sycl/detail/handler_proxy.hpp Show resolved Hide resolved
sycl/source/detail/handler_proxy.cpp Show resolved Hide resolved
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
@v-klochkov
Copy link
Contributor

Overall Looks Good To Me. I'll mark approved (as reduction.hpp owner) after additional fixes for Alexander's comments.
Please also respond to my question regarding __SYCL_EXPORT.

Sergey Kanaev added 3 commits May 28, 2020 21:12
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
@s-kanaev
Copy link
Contributor Author

s-kanaev commented Jun 1, 2020

@turinevgeny @romanovvlad any comments from you?

@romanovvlad
Copy link
Contributor

romanovvlad commented Jun 2, 2020

@turinevgeny @romanovvlad any comments from you?

No, the patch is OK to me.

@bader bader merged commit 0473316 into intel:sycl Jun 2, 2020
@s-kanaev s-kanaev deleted the private/s-kanaev/break-include-cycle branch June 2, 2020 10:12
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 15, 2022
After intel#1770 some uses of associateWithHandler
have to be guarded by "#ifndef __SYCL_DEVICE_ONLY__" as the accessor's
inheritance chain differs between host/device compilation.

Put the burden of that into the handler's implementation instead of paying the
price at uses.
againull pushed a commit that referenced this pull request Jul 18, 2022
After #1770 some uses of associateWithHandler
have to be guarded by "#ifndef __SYCL_DEVICE_ONLY__" as the accessor's
inheritance chain differs between host/device compilation.

Put the burden of that into the handler's implementation instead of paying the
price at uses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants