Skip to content

[SYCL][ABI-Break] Hide getPlugin symbol #14616

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

Conversation

AlexeySachkov
Copy link
Contributor

This function is not called anywhere from SYCL headers and therefore there is no reason to have it exported.

It was made exported as part of ESIMD emulator plugin addition (#2963), but that plugin was since
then deprecated and removed (#13295).

This is patch is essentially a by-product of #14145 and it is done to simplify that change, i.e. PI plugins removal should not be ABI-breaking by itself, we just need to cleanup some of our exported symbols.

This function is not called anywhere from SYCL headers and therefore
there is no reason to have it exported.

It was made exported as part of ESIMD emulator plugin addition
(intel#2963), but that plugin was since
then deprecated and removed (intel#13295).
@AlexeySachkov AlexeySachkov requested a review from a team as a code owner July 18, 2024 08:07
@AlexeySachkov AlexeySachkov added the abi-break change that's breaking abi and waiting for the next window to be able to merge label Jul 18, 2024
@aelovikov-intel
Copy link
Contributor

We should drop

# pi.cpp:
# template <backend BE> const PluginPtr &getPlugin() {
# static const plugin *Plugin = nullptr;
# ...
# }
# // explicit dllexport instantiations.
#
# clang-cl generates exported symbols for the static variables Plugin. These are never referenced
# in the user's headers so cannot be used outside DLL and not exporting them should not affect any
# usage scenario.
#
# In general, the compiler doesn't know if the definition is in the DLL or in the header and inline
# dllexport/dllimport functions have to be supported, hence clang-cl's behavior.
#
# See also https://devblogs.microsoft.com/oldnewthing/20140109-00/?p=2123.
ignore_symbols += [
"?Plugin@?1???$getPlugin@$01@pi@detail@_V1@sycl@@YAAEBVplugin@234@XZ@4PEBV5234@EB",
"?Plugin@?1???$getPlugin@$00@pi@detail@_V1@sycl@@YAAEBVplugin@234@XZ@4PEBV5234@EB",
"?Plugin@?1???$getPlugin@$04@pi@detail@_V1@sycl@@YAAEBVplugin@234@XZ@4PEBV5234@EB",
"?Plugin@?1???$getPlugin@$02@pi@detail@_V1@sycl@@YAAEBVplugin@234@XZ@4PEBV5234@EB",
"?Plugin@?1???$getPlugin@$05@pi@detail@_V1@sycl@@YAAEBVplugin@234@XZ@4PEBV5234@EB",
]
as part of this, probably. On the other hand, given the abi breaking window closure we can address it in a separate PR.

@AlexeySachkov AlexeySachkov merged commit b23233f into intel:sycl Jul 18, 2024
16 checks passed
@AlexeySachkov AlexeySachkov deleted the private/asachkov/drop-get-plugin-symbol branch July 18, 2024 17:37
AlexeySachkov added a commit to AlexeySachkov/llvm that referenced this pull request Jul 18, 2024
This is a follow-up for intel#14616 to drop special-casing some of
SYCL RT functions that we don't export anymore.
@AlexeySachkov
Copy link
Contributor Author

We should drop

# pi.cpp:
# template <backend BE> const PluginPtr &getPlugin() {
# static const plugin *Plugin = nullptr;
# ...
# }
# // explicit dllexport instantiations.
#
# clang-cl generates exported symbols for the static variables Plugin. These are never referenced
# in the user's headers so cannot be used outside DLL and not exporting them should not affect any
# usage scenario.
#
# In general, the compiler doesn't know if the definition is in the DLL or in the header and inline
# dllexport/dllimport functions have to be supported, hence clang-cl's behavior.
#
# See also https://devblogs.microsoft.com/oldnewthing/20140109-00/?p=2123.
ignore_symbols += [
"?Plugin@?1???$getPlugin@$01@pi@detail@_V1@sycl@@YAAEBVplugin@234@XZ@4PEBV5234@EB",
"?Plugin@?1???$getPlugin@$00@pi@detail@_V1@sycl@@YAAEBVplugin@234@XZ@4PEBV5234@EB",
"?Plugin@?1???$getPlugin@$04@pi@detail@_V1@sycl@@YAAEBVplugin@234@XZ@4PEBV5234@EB",
"?Plugin@?1???$getPlugin@$02@pi@detail@_V1@sycl@@YAAEBVplugin@234@XZ@4PEBV5234@EB",
"?Plugin@?1???$getPlugin@$05@pi@detail@_V1@sycl@@YAAEBVplugin@234@XZ@4PEBV5234@EB",
]

as part of this, probably. On the other hand, given the abi breaking window closure we can address it in a separate PR.

#14637

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abi-break change that's breaking abi and waiting for the next window to be able to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants