Skip to content

[SYCL] Do not set CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON for plugins #10702

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

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

smaslov-intel
Copy link
Contributor

@smaslov-intel smaslov-intel commented Aug 4, 2023

We just don't need it:

  1. PI interfaces are specifically marked with __SYCL_EXPORT, e.g.
    __SYCL_EXPORT pi_result piPluginInit(pi_plugin *plugin_info);
  2. We call to PI interfaces (except for piPluginInit) via a pointer from a table set up during piPluginInit:
    inline FuncPtrT getFuncPtr(PiPlugin MPlugin) { \

@smaslov-intel smaslov-intel requested review from a team as code owners August 4, 2023 22:18
@smaslov-intel smaslov-intel requested a review from bso-intel August 4, 2023 22:18
@smaslov-intel smaslov-intel temporarily deployed to aws August 4, 2023 22:28 — with GitHub Actions Inactive
@smaslov-intel smaslov-intel temporarily deployed to aws August 4, 2023 23:08 — with GitHub Actions Inactive
@smaslov-intel
Copy link
Contributor Author

Waiting on code owner review from @intel/dpcpp-l0-pi-reviewers and/or @intel/llvm-reviewers-runtime.

@againull againull merged commit a8cc71e into intel:sycl Aug 7, 2023
mdtoguchi pushed a commit to mdtoguchi/llvm that referenced this pull request Oct 18, 2023
…tel#10702)

We just don't need it:
1) PI interfaces are specifically marked with __SYCL_EXPORT, e.g.
https://github.com/intel/llvm/blob/8e0cc4b7a845df9389a1313a3e680babc4d87782/sycl/include/sycl/detail/pi.h#L1148
2) We call to PI interfaces (except for `piPluginInit`) via a pointer
from a table set up during
`piPluginInit`:https://github.com/intel/llvm/blob/8e0cc4b7a845df9389a1313a3e680babc4d87782/sycl/include/sycl/detail/pi.hpp#L220

Signed-off-by: smaslov <[email protected]>
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.

3 participants