Skip to content

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.

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