Skip to content

Commit

Permalink
[Libomptarget] Build plugins with protected visibility by default
Browse files Browse the repository at this point in the history
The plugins all define the same interface symbols. This is generally not
a problem when calling the plugin directly from the dynamic library's
handle. However, when calling from within the plugin itself it is
possible for another plugin's symbols to preempt the symbols. This was
observed with the `__tgt_rtl_is_valid_binary` call in the
`__tgt_rtl_is_valid_binary_info` function being mapped to the x86_64
plugin.

This patch changes the default visibility to `protected` intead. This
visibility ensures that these symbols are all externally visible from
the plugin, but ensures their definitions are fixed within the shared
library. Having protected visiiblity makes such symbol preemption
impossible.

Reviewed By: tianshilei1992

Differential Revision: https://reviews.llvm.org/D136365
  • Loading branch information
jhuber6 committed Oct 20, 2022
1 parent 62eae83 commit 429d3d4
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 4 deletions.
3 changes: 2 additions & 1 deletion openmp/libomptarget/plugins/CMakeLists.txt
Expand Up @@ -52,7 +52,8 @@ if(CMAKE_SYSTEM_PROCESSOR MATCHES "${tmachine}$")
install(TARGETS "omptarget.rtl.${tmachine_libname}"
LIBRARY DESTINATION "${OPENMP_INSTALL_LIBDIR}")
set_target_properties("omptarget.rtl.${tmachine_libname}" PROPERTIES
INSTALL_RPATH "$ORIGIN" BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/..")
INSTALL_RPATH "$ORIGIN" BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/.."
CXX_VISIBILITY_PRESET protected)

target_include_directories( "omptarget.rtl.${tmachine_libname}" PRIVATE
${LIBOMPTARGET_INCLUDE_DIR}
Expand Down
3 changes: 2 additions & 1 deletion openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
Expand Up @@ -102,7 +102,8 @@ target_include_directories(
# Install plugin under the lib destination folder.
install(TARGETS omptarget.rtl.amdgpu LIBRARY DESTINATION "${OPENMP_INSTALL_LIBDIR}")
set_target_properties(omptarget.rtl.amdgpu PROPERTIES
INSTALL_RPATH "$ORIGIN" BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/..")
INSTALL_RPATH "$ORIGIN" BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/.."
CXX_VISIBILITY_PRESET protected)

# Report to the parent scope that we are building a plugin for hsa.
# This controls whether tests are run for the nvptx offloading target
Expand Down
3 changes: 2 additions & 1 deletion openmp/libomptarget/plugins/cuda/CMakeLists.txt
Expand Up @@ -87,7 +87,8 @@ add_dependencies(omptarget.rtl.cuda omptarget.devicertl.nvptx)
# Install plugin under the lib destination folder.
install(TARGETS omptarget.rtl.cuda LIBRARY DESTINATION "${OPENMP_INSTALL_LIBDIR}")
set_target_properties(omptarget.rtl.cuda PROPERTIES
INSTALL_RPATH "$ORIGIN" BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/..")
INSTALL_RPATH "$ORIGIN" BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/.."
CXX_VISIBILITY_PRESET protected)

target_include_directories(omptarget.rtl.cuda PRIVATE
${LIBOMPTARGET_INCLUDE_DIR}
Expand Down
3 changes: 2 additions & 1 deletion openmp/libomptarget/plugins/ve/CMakeLists.txt
Expand Up @@ -44,7 +44,8 @@ if(${LIBOMPTARGET_DEP_VEO_FOUND})
# Install plugin under the lib destination folder.
install(TARGETS "omptarget.rtl.${tmachine_libname}" LIBRARY DESTINATION "${OPENMP_INSTALL_LIBDIR}")
set_target_properties("omptarget.rtl.${tmachine_libname}" PROPERTIES
INSTALL_RPATH "$ORIGIN" BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/..")
INSTALL_RPATH "$ORIGIN" BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/.."
CXX_VISIBILITY_PRESET protected)

target_include_directories("omptarget.rtl.${tmachine_libname}" PRIVATE
${LIBOMPTARGET_INCLUDE_DIR}
Expand Down

0 comments on commit 429d3d4

Please sign in to comment.