-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[OpenMP][Offload] Support standalone build #88957
Conversation
With this commit, users need to enable the new LLVM/Offload subproject as a runtime in their cmake configuration. No further changes are expected for downstream code. Tests and other components still depend on OpenMP and have also not been renamed. The results below are for a build in which OpenMP and Offload are enabled runtimes. In addition to the pure git mv, I needed to adjust some CMake files. Nothing is intended to change semantics. ``` ninja check-offload ``` Works with the X86 and AMDGPU offload tests ``` ninja check-openmp ``` Still works but doesn't build offload tests anymore. ``` ls install/lib ``` Shows all expected libraries, incl. - `libomptarget.devicertl.a` - `libomptarget-nvptx-sm_90.bc` - `libomptarget.rtl.amdgpu.so` -> `libomptarget.rtl.amdgpu.so.19git` - `libomptarget.so` -> `libomptarget.so.19git` Fixes: llvm#75124
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused here. I thought all that's required is
- Detect support for
--version-script
in the linker - Find
libomp.so
to link against it - Find
omp.h
andomp-tools.h
to include.
What's all this other stuff for?
offload/CMakeLists.txt
Outdated
else() | ||
option(LIBOMPTARGET_ENABLE_DEBUG "Allow debug output with the environment variable LIBOMPTARGET_DEBUG=1" OFF) | ||
endif() | ||
if(LIBOMPTARGET_ENABLE_DEBUG) | ||
add_definitions(-DOMPTARGET_DEBUG) | ||
endif() | ||
|
||
# OMPD support for libomptarget (currently only with cuda) | ||
set(LIBOMPTARGET_OMPD_SUPPORT FALSE CACHE BOOL "OMPD-support?") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need OMPD
?
offload/CMakeLists.txt
Outdated
@@ -159,13 +168,20 @@ set (LIBOMPTARGET_TESTED_PLUGINS "") | |||
string( TOLOWER "${CMAKE_BUILD_TYPE}" LIBOMPTARGET_CMAKE_BUILD_TYPE) | |||
if(LIBOMPTARGET_CMAKE_BUILD_TYPE MATCHES debug) | |||
option(LIBOMPTARGET_ENABLE_DEBUG "Allow debug output with the environment variable LIBOMPTARGET_DEBUG=1" ON) | |||
add_definitions(-DDEBUG) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this for? I don't think we want a global definition DEBUG
set for every compilation.
offload/CMakeLists.txt
Outdated
if (OPENMP_STANDALONE_BUILD OR "${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}") | ||
set(OPENMP_STANDALONE_BUILD TRUE) | ||
project(omptarget C CXX ASM) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why "OpenMP?" Shouldn't it of offload
? This is definitely required for standalone compilation however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the libomptarget folder was just renamed offload, all of the checks were not renamed. So we stuck with OPENMP_STANDALONE_BUILD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant the project
name as well.
What is this pattern
if (OPENMP_STANDALONE_BUILD)
set(OPENMP_STANDALONE_BUILD TRUE)
endif()
Seems wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, the project could probably change. If the offload/CMakeLists.txt is invoked directly without setting OPENMP_STANDALONE_BUILD
in the cmake configuration then OPENMP_STANDALONE_BUILD
will need to be set here so the rest of the checks will work.
# Check if OMPT support is available | ||
# Currently, __builtin_frame_address() is required for OMPT | ||
# Weak attribute is required for Unices (except Darwin), LIBPSAPI is used for Windows | ||
if(NOT LIBOMP_ARCH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need LIBOMP_ARCH here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because right below LIBOMP_HAVE_OMPT_SUPPORT needs it and the standalone build of offload is not aware of LIBOMP_ARCH.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't there some header that's only built if we have OMPT support? Presumably the presence of that would indicate that whoever built OpenMP had OMPT support, or we could just make it an option, there's no explicit need for this to be automatically detected since a standalone build is supposed to be manual.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first thought this could work. If the ompt header is found offload standalone would set the OMPT support to ON. This would allow us to drop the majority of this top level cmake module. Then that makes the argument for removing it entirely and just put LIBOMP_HAVE_VERSION_SCRIPT_FLAG
and pythonize_bool
directly into offload/CMakeLists.txt.
offload/CMakeLists.txt
Outdated
endif() | ||
include_directories(${LIBOMPTARGET_INCLUDE_DIR} ${LIBOMP_OMP_TOOLS_INCLUDE_DIR}) | ||
|
||
# Various LLVM_TOOLS are needed to build libomptarget |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this stuff coming from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this code below should not be necessary.
bd42137
to
e82f077
Compare
@llvm/pr-subscribers-offload @llvm/pr-subscribers-backend-amdgpu Author: Saiyedul Islam (saiislam) ChangesMoved some of the common functionality in a top-level common Depends on #75125 Patch is 121.46 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/88957.diff 355 Files Affected:
diff --git a/cmake/Modules/OffloadOpenmpCommon.cmake b/cmake/Modules/OffloadOpenmpCommon.cmake
new file mode 100644
index 00000000000000..b4e1737e7ef78c
--- /dev/null
+++ b/cmake/Modules/OffloadOpenmpCommon.cmake
@@ -0,0 +1,68 @@
+# Check if OMPT support is available
+# Currently, __builtin_frame_address() is required for OMPT
+# Weak attribute is required for Unices (except Darwin), LIBPSAPI is used for Windows
+if(NOT LIBOMP_ARCH)
+ include(LibompGetArchitecture)
+ libomp_get_architecture(LIBOMP_DETECTED_ARCH)
+ set(LIBOMP_ARCH ${LIBOMP_DETECTED_ARCH})
+endif()
+
+check_c_source_compiles("int main(int argc, char** argv) {
+ void* p = __builtin_frame_address(0);
+ return 0;}" LIBOMP_HAVE___BUILTIN_FRAME_ADDRESS)
+check_c_source_compiles("__attribute__ ((weak)) int foo(int a) { return a*a; }
+ int main(int argc, char** argv) {
+ return foo(argc);}" LIBOMP_HAVE_WEAK_ATTRIBUTE)
+set(CMAKE_REQUIRED_LIBRARIES psapi)
+check_c_source_compiles("#include <windows.h>
+ #include <psapi.h>
+ int main(int artc, char** argv) {
+ return EnumProcessModules(NULL, NULL, 0, NULL);
+ }" LIBOMP_HAVE_PSAPI)
+set(CMAKE_REQUIRED_LIBRARIES)
+if(NOT LIBOMP_HAVE___BUILTIN_FRAME_ADDRESS)
+ set(LIBOMP_HAVE_OMPT_SUPPORT FALSE)
+else()
+ if( # hardware architecture supported?
+ ((LIBOMP_ARCH STREQUAL x86_64) OR
+ (LIBOMP_ARCH STREQUAL i386) OR
+# (LIBOMP_ARCH STREQUAL arm) OR
+ (LIBOMP_ARCH STREQUAL aarch64) OR
+ (LIBOMP_ARCH STREQUAL aarch64_32) OR
+ (LIBOMP_ARCH STREQUAL aarch64_a64fx) OR
+ (LIBOMP_ARCH STREQUAL ppc64le) OR
+ (LIBOMP_ARCH STREQUAL ppc64) OR
+ (LIBOMP_ARCH STREQUAL riscv64) OR
+ (LIBOMP_ARCH STREQUAL loongarch64) OR
+ (LIBOMP_ARCH STREQUAL s390x))
+ AND # OS supported?
+ ((WIN32 AND LIBOMP_HAVE_PSAPI) OR APPLE OR
+ (NOT (WIN32 OR ${CMAKE_SYSTEM_NAME} MATCHES "AIX") AND LIBOMP_HAVE_WEAK_ATTRIBUTE)))
+ set(LIBOMP_HAVE_OMPT_SUPPORT TRUE)
+ else()
+ set(LIBOMP_HAVE_OMPT_SUPPORT FALSE)
+ endif()
+endif()
+
+# OMPT-support defaults to ON for OpenMP 5.0+ and if the requirements in
+# cmake/config-ix.cmake are fulfilled.
+set(OMPT_DEFAULT FALSE)
+if ((LIBOMP_HAVE_OMPT_SUPPORT) AND (NOT WIN32))
+ set(OMPT_DEFAULT TRUE)
+endif()
+set(LIBOMP_OMPT_SUPPORT ${OMPT_DEFAULT} CACHE BOOL
+ "OMPT-support?")
+
+# Check LIBOMP_HAVE_VERSION_SCRIPT_FLAG
+include(LLVMCheckCompilerLinkerFlag)
+if(NOT APPLE)
+ llvm_check_compiler_linker_flag(C "-Wl,--version-script=${CMAKE_CURRENT_LIST_DIR}/../../openmp/runtime/src/exports_test_so.txt" LIBOMP_HAVE_VERSION_SCRIPT_FLAG)
+endif()
+
+macro(pythonize_bool var)
+if (${var})
+ set(${var} True)
+else()
+ set(${var} False)
+endif()
+endmacro()
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 6f5647d70d8bc1..f9b953185d3e01 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -147,7 +147,7 @@ endif()
# As we migrate runtimes to using the bootstrapping build, the set of default runtimes
# should grow as we remove those runtimes from LLVM_ENABLE_PROJECTS above.
set(LLVM_DEFAULT_RUNTIMES "libcxx;libcxxabi;libunwind")
-set(LLVM_SUPPORTED_RUNTIMES "libc;libunwind;libcxxabi;pstl;libcxx;compiler-rt;openmp;llvm-libgcc")
+set(LLVM_SUPPORTED_RUNTIMES "libc;libunwind;libcxxabi;pstl;libcxx;compiler-rt;openmp;llvm-libgcc;offload")
set(LLVM_ENABLE_RUNTIMES "" CACHE STRING
"Semicolon-separated list of runtimes to build, or \"all\" (${LLVM_DEFAULT_RUNTIMES}). Supported runtimes are ${LLVM_SUPPORTED_RUNTIMES}.")
if(LLVM_ENABLE_RUNTIMES STREQUAL "all")
diff --git a/openmp/libomptarget/CMakeLists.txt b/offload/CMakeLists.txt
similarity index 59%
rename from openmp/libomptarget/CMakeLists.txt
rename to offload/CMakeLists.txt
index 531198fae01699..0c20aa01f34b1b 100644
--- a/openmp/libomptarget/CMakeLists.txt
+++ b/offload/CMakeLists.txt
@@ -10,12 +10,106 @@
#
##===----------------------------------------------------------------------===##
-if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
- message(FATAL_ERROR "Direct configuration not supported, please use parent directory!")
+cmake_minimum_required(VERSION 3.20.0)
+
+if (OPENMP_STANDALONE_BUILD OR "${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
+ set(OPENMP_STANDALONE_BUILD TRUE)
+ project(omptarget C CXX ASM)
+endif()
+
+set(ENABLE_LIBOMPTARGET ON)
+# Currently libomptarget cannot be compiled on Windows or MacOS X.
+# Since the device plugins are only supported on Linux anyway,
+# there is no point in trying to compile libomptarget on other OSes.
+# 32-bit systems are not supported either.
+if (APPLE OR WIN32 OR NOT "cxx_std_17" IN_LIST CMAKE_CXX_COMPILE_FEATURES OR NOT CMAKE_SIZEOF_VOID_P EQUAL 8)
+ set(ENABLE_LIBOMPTARGET OFF)
endif()
-# Add cmake directory to search for custom cmake functions.
-set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules ${CMAKE_MODULE_PATH})
+option(OPENMP_ENABLE_LIBOMPTARGET "Enable building libomptarget for offloading."
+ ${ENABLE_LIBOMPTARGET})
+if (OPENMP_ENABLE_LIBOMPTARGET)
+ # Check that the library can actually be built.
+ if (APPLE OR WIN32)
+ message(FATAL_ERROR "libomptarget cannot be built on Windows and MacOS X!")
+ elseif (NOT "cxx_std_17" IN_LIST CMAKE_CXX_COMPILE_FEATURES)
+ message(FATAL_ERROR "Host compiler must support C++17 to build libomptarget!")
+ elseif (NOT CMAKE_SIZEOF_VOID_P EQUAL 8)
+ message(FATAL_ERROR "libomptarget on 32-bit systems are not supported!")
+ endif()
+endif()
+
+# TODO: Leftover from the move, could probably be just LLVM_LIBDIR_SUFFIX everywhere.
+set(OFFLOAD_INSTALL_LIBDIR "lib${LLVM_LIBDIR_SUFFIX}")
+
+set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+ "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
+ "${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules"
+ "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+ )
+
+if (OPENMP_STANDALONE_BUILD)
+ # CMAKE_BUILD_TYPE was not set, default to Release.
+ if (NOT CMAKE_BUILD_TYPE)
+ set(CMAKE_BUILD_TYPE Release)
+ endif()
+
+ # Group common settings.
+ set(OPENMP_ENABLE_WERROR FALSE CACHE BOOL
+ "Enable -Werror flags to turn warnings into errors for supporting compilers.")
+ set(OPENMP_LIBDIR_SUFFIX "" CACHE STRING
+ "Suffix of lib installation directory, e.g. 64 => lib64")
+ # Do not use OPENMP_LIBDIR_SUFFIX directly, use OPENMP_INSTALL_LIBDIR.
+ set(OPENMP_INSTALL_LIBDIR "lib${OPENMP_LIBDIR_SUFFIX}")
+
+ # Group test settings.
+ set(OPENMP_TEST_C_COMPILER ${CMAKE_C_COMPILER} CACHE STRING
+ "C compiler to use for testing OpenMP runtime libraries.")
+ set(OPENMP_TEST_CXX_COMPILER ${CMAKE_CXX_COMPILER} CACHE STRING
+ "C++ compiler to use for testing OpenMP runtime libraries.")
+ set(OPENMP_TEST_Fortran_COMPILER ${CMAKE_Fortran_COMPILER} CACHE STRING
+ "FORTRAN compiler to use for testing OpenMP runtime libraries.")
+ set(OPENMP_LLVM_TOOLS_DIR "" CACHE PATH "Path to LLVM tools for testing.")
+
+ set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
+ set(CMAKE_CXX_STANDARD_REQUIRED NO)
+ set(CMAKE_CXX_EXTENSIONS NO)
+else()
+ set(OPENMP_ENABLE_WERROR ${LLVM_ENABLE_WERROR})
+ # If building in tree, we honor the same install suffix LLVM uses.
+ set(OPENMP_INSTALL_LIBDIR "lib${LLVM_LIBDIR_SUFFIX}")
+
+ if (NOT MSVC)
+ set(OPENMP_TEST_C_COMPILER ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang)
+ set(OPENMP_TEST_CXX_COMPILER ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++)
+ else()
+ set(OPENMP_TEST_C_COMPILER ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang.exe)
+ set(OPENMP_TEST_CXX_COMPILER ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++.exe)
+ endif()
+
+ # Check for flang
+ if (NOT MSVC)
+ set(OPENMP_TEST_Fortran_COMPILER ${LLVM_RUNTIME_OUTPUT_INTDIR}/flang-new)
+ else()
+ set(OPENMP_TEST_Fortran_COMPILER ${LLVM_RUNTIME_OUTPUT_INTDIR}/flang-new.exe)
+ endif()
+
+ # Set fortran test compiler if flang is found
+ if (EXISTS "${OPENMP_TEST_Fortran_COMPILER}")
+ message("Using local flang build at ${OPENMP_TEST_Fortran_COMPILER}")
+ else()
+ unset(OPENMP_TEST_Fortran_COMPILER)
+ endif()
+
+ # If not standalone, set CMAKE_CXX_STANDARD but don't set the global cache value,
+ # only set it locally for OpenMP.
+ set(CMAKE_CXX_STANDARD 17)
+ set(CMAKE_CXX_STANDARD_REQUIRED NO)
+ set(CMAKE_CXX_EXTENSIONS NO)
+endif()
# Set the path of all resulting libraries to a unified location so that it can
# be used for testing.
@@ -36,6 +130,9 @@ include(LibomptargetUtils)
# Get dependencies for the different components of the project.
include(LibomptargetGetDependencies)
+# Set up testing infrastructure.
+include(OpenMPTesting)
+
# LLVM source tree is required at build time for libomptarget
if (NOT LIBOMPTARGET_LLVM_INCLUDE_DIRS)
message(FATAL_ERROR "Missing definition for LIBOMPTARGET_LLVM_INCLUDE_DIRS")
@@ -101,6 +198,10 @@ if (LIBOMPTARGET_USE_LTO)
list(APPEND offload_link_flags ${CMAKE_CXX_COMPILE_OPTIONS_IPO})
endif()
+# Set LIBOMP_HAVE_OMPT_SUPPORT and LIBOMP_HAVE_VERSION_SCRIPT_FLAG using common offload/openmp cmake module.
+list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/../openmp/runtime/cmake")
+include(OffloadOpenmpCommon)
+
# OMPT support for libomptarget
# Follow host OMPT support and check if host support has been requested.
# LIBOMP_HAVE_OMPT_SUPPORT indicates whether host OMPT support has been implemented.
@@ -125,15 +226,35 @@ set(LIBOMPTARGET_GPU_LIBC_SUPPORT ${LLVM_LIBC_GPU_BUILD} CACHE BOOL
"Libomptarget support for the GPU libc")
pythonize_bool(LIBOMPTARGET_GPU_LIBC_SUPPORT)
+if(OPENMP_STANDALONE_BUILD)
+ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ find_package(LLVM HINTS ${CMAKE_INSTALL_PREFIX})
+ endif()
+ find_path (
+ LIBOMP_OMP_TOOLS_INCLUDE_DIR
+ NAMES
+ omp-tools.h
+ HINTS
+ ${LLVM_INCLUDE_DIRS}
+ ${LLVM_INCLUDE_DIRS}/../lib/clang/${LLVM_VERSION_MAJOR}/include
+ ${CMAKE_INSTALL_PREFIX}/include
+ REQUIRED
+ )
+ find_library (
+ LIBOMP_STANDALONE
+ NAMES
+ omp
+ HINTS
+ ${LLVM_LIBRARY_DIRS}
+ ${CMAKE_INSTALL_PREFIX}/lib
+ REQUIRED
+ )
+endif()
+
set(LIBOMPTARGET_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/include)
message(STATUS "OpenMP tools dir in libomptarget: ${LIBOMP_OMP_TOOLS_INCLUDE_DIR}")
include_directories(${LIBOMP_OMP_TOOLS_INCLUDE_DIR})
-# Definitions for testing, for reuse when testing libomptarget-nvptx.
-set(LIBOMPTARGET_OPENMP_HEADER_FOLDER "${LIBOMP_INCLUDE_DIR}" CACHE STRING
- "Path to folder containing omp.h")
-set(LIBOMPTARGET_OPENMP_HOST_RTL_FOLDER "${LIBOMP_LIBRARY_DIR}" CACHE STRING
- "Path to folder containing libomp.so, and libLLVMSupport.so with profiling enabled")
set(LIBOMPTARGET_LLVM_LIBRARY_DIR "${LLVM_LIBRARY_DIR}" CACHE STRING
"Path to folder containing llvm library libomptarget.so")
set(LIBOMPTARGET_LLVM_LIBRARY_INTDIR "${LIBOMPTARGET_INTDIR}" CACHE STRING
diff --git a/openmp/libomptarget/DeviceRTL/CMakeLists.txt b/offload/DeviceRTL/CMakeLists.txt
similarity index 99%
rename from openmp/libomptarget/DeviceRTL/CMakeLists.txt
rename to offload/DeviceRTL/CMakeLists.txt
index 2509f1276ccee6..c7a5dc5ccf5b67 100644
--- a/openmp/libomptarget/DeviceRTL/CMakeLists.txt
+++ b/offload/DeviceRTL/CMakeLists.txt
@@ -228,7 +228,7 @@ function(compileDeviceRTLLibrary target_cpu target_name target_triple)
set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${bclib_name} ${LIBOMPTARGET_LIBRARY_DIR}/${bclib_name})
# Install bitcode library under the lib destination folder.
- install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name} DESTINATION "${OPENMP_INSTALL_LIBDIR}")
+ install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name} DESTINATION "${OFFLOAD_INSTALL_LIBDIR}")
set(target_feature "")
if("${target_triple}" STREQUAL "nvptx64-nvidia-cuda")
@@ -307,4 +307,4 @@ set_target_properties(omptarget.devicertl PROPERTIES
)
target_link_libraries(omptarget.devicertl PRIVATE omptarget.devicertl.all_objs)
-install(TARGETS omptarget.devicertl ARCHIVE DESTINATION ${OPENMP_INSTALL_LIBDIR})
+install(TARGETS omptarget.devicertl ARCHIVE DESTINATION ${OFFLOAD_INSTALL_LIBDIR})
diff --git a/openmp/libomptarget/DeviceRTL/include/Allocator.h b/offload/DeviceRTL/include/Allocator.h
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/Allocator.h
rename to offload/DeviceRTL/include/Allocator.h
diff --git a/openmp/libomptarget/DeviceRTL/include/Configuration.h b/offload/DeviceRTL/include/Configuration.h
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/Configuration.h
rename to offload/DeviceRTL/include/Configuration.h
diff --git a/openmp/libomptarget/DeviceRTL/include/Debug.h b/offload/DeviceRTL/include/Debug.h
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/Debug.h
rename to offload/DeviceRTL/include/Debug.h
diff --git a/openmp/libomptarget/DeviceRTL/include/Interface.h b/offload/DeviceRTL/include/Interface.h
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/Interface.h
rename to offload/DeviceRTL/include/Interface.h
diff --git a/openmp/libomptarget/DeviceRTL/include/LibC.h b/offload/DeviceRTL/include/LibC.h
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/LibC.h
rename to offload/DeviceRTL/include/LibC.h
diff --git a/openmp/libomptarget/DeviceRTL/include/Mapping.h b/offload/DeviceRTL/include/Mapping.h
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/Mapping.h
rename to offload/DeviceRTL/include/Mapping.h
diff --git a/openmp/libomptarget/DeviceRTL/include/State.h b/offload/DeviceRTL/include/State.h
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/State.h
rename to offload/DeviceRTL/include/State.h
diff --git a/openmp/libomptarget/DeviceRTL/include/Synchronization.h b/offload/DeviceRTL/include/Synchronization.h
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/Synchronization.h
rename to offload/DeviceRTL/include/Synchronization.h
diff --git a/openmp/libomptarget/DeviceRTL/include/Types.h b/offload/DeviceRTL/include/Types.h
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/Types.h
rename to offload/DeviceRTL/include/Types.h
diff --git a/openmp/libomptarget/DeviceRTL/include/Utils.h b/offload/DeviceRTL/include/Utils.h
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/Utils.h
rename to offload/DeviceRTL/include/Utils.h
diff --git a/openmp/libomptarget/DeviceRTL/include/generated_microtask_cases.gen b/offload/DeviceRTL/include/generated_microtask_cases.gen
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/include/generated_microtask_cases.gen
rename to offload/DeviceRTL/include/generated_microtask_cases.gen
diff --git a/openmp/libomptarget/DeviceRTL/src/Allocator.cpp b/offload/DeviceRTL/src/Allocator.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Allocator.cpp
rename to offload/DeviceRTL/src/Allocator.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Configuration.cpp b/offload/DeviceRTL/src/Configuration.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Configuration.cpp
rename to offload/DeviceRTL/src/Configuration.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Debug.cpp b/offload/DeviceRTL/src/Debug.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Debug.cpp
rename to offload/DeviceRTL/src/Debug.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Kernel.cpp b/offload/DeviceRTL/src/Kernel.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Kernel.cpp
rename to offload/DeviceRTL/src/Kernel.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/LibC.cpp b/offload/DeviceRTL/src/LibC.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/LibC.cpp
rename to offload/DeviceRTL/src/LibC.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Mapping.cpp b/offload/DeviceRTL/src/Mapping.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Mapping.cpp
rename to offload/DeviceRTL/src/Mapping.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Misc.cpp b/offload/DeviceRTL/src/Misc.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Misc.cpp
rename to offload/DeviceRTL/src/Misc.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Parallelism.cpp b/offload/DeviceRTL/src/Parallelism.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Parallelism.cpp
rename to offload/DeviceRTL/src/Parallelism.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Reduction.cpp b/offload/DeviceRTL/src/Reduction.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Reduction.cpp
rename to offload/DeviceRTL/src/Reduction.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/State.cpp b/offload/DeviceRTL/src/State.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/State.cpp
rename to offload/DeviceRTL/src/State.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Stub.cpp b/offload/DeviceRTL/src/Stub.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Stub.cpp
rename to offload/DeviceRTL/src/Stub.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Synchronization.cpp b/offload/DeviceRTL/src/Synchronization.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Synchronization.cpp
rename to offload/DeviceRTL/src/Synchronization.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Tasking.cpp b/offload/DeviceRTL/src/Tasking.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Tasking.cpp
rename to offload/DeviceRTL/src/Tasking.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Utils.cpp b/offload/DeviceRTL/src/Utils.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Utils.cpp
rename to offload/DeviceRTL/src/Utils.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/Workshare.cpp b/offload/DeviceRTL/src/Workshare.cpp
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/Workshare.cpp
rename to offload/DeviceRTL/src/Workshare.cpp
diff --git a/openmp/libomptarget/DeviceRTL/src/exports b/offload/DeviceRTL/src/exports
similarity index 100%
rename from openmp/libomptarget/DeviceRTL/src/exports
rename to offload/DeviceRTL/src/exports
diff --git a/openmp/libomptarget/README.txt b/offload/README.txt
similarity index 100%
rename from openmp/libomptarget/README.txt
rename to offload/README.txt
diff --git a/openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake b/offload/cmake/Modules/LibomptargetGetDependencies.cmake
similarity index 100%
rename from openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake
rename to offload/cmake/Modules/LibomptargetGetDependencies.cmake
diff --git a/openmp/libomptarget/cmake/Modules/LibomptargetUtils.cmake b/offload/cmake/Modules/LibomptargetUtils.cmake
similarity index 100%
rename from openmp/libomptarget/cmake/Modules/LibomptargetUtils.cmake
rename to offload/cmake/Modules/LibomptargetUtils.cmake
diff --git a/offload/cmake/OpenMPTesting.cmake b/offload/cmake/OpenMPTesting.cmake
new file mode 100644
index 00000000000000..11eafeb764260f
--- /dev/null
+++ b/offload/cmake/OpenMPTesting.cmake
@@ -0,0 +1,238 @@
+# Keep track if we have all dependencies.
+set(ENABLE_CHECK_TARGETS TRUE)
+
+# Function to find required dependencies for testing.
+function(find_standalone_test_dependencies)
+ find_package (Python3 COMPONENTS Interpreter)
+
+ if (NOT Python3_Interpreter_FOUND)
+ message(STATUS "Could not find Python.")
+ message(WARNING "The check targets will not be available!")
+ set(ENABLE_CHECK_TARGETS FALSE PARENT_SCOPE)
+ return()
+ else()
+ set(Python3_EXECUTABLE ${Python3_EXECUTABLE} PARENT_SCOPE)
+ endif()
+
+ # Find executables.
+ find_program(OPENMP_LLVM_LIT_EXECUTABLE
+ NAMES llvm-lit.py llvm-lit lit.py lit
+ PATHS ${OPENMP_LLVM_TOOLS_DIR})
+ if (NOT OPENMP_LLVM_LIT_EXECUTABLE)
+ message(STATUS "Cannot find llvm-lit.")
+ message(STATUS "Please put llvm-lit in your PATH, set OPENMP_LLVM_LIT_EXECUTABLE to its full path, or point OPENMP_LLVM_TOOLS_DIR to its directory.")
+ message(WARNING "The check targets will not be available!")
+ set(ENABLE_CHECK_TARGETS FALSE PARENT_SCOPE)
+ return()
+ endif()
+
+ find_program(OPENMP_FILECHECK_EXECUTABLE
+ NAMES FileCheck
+ PATHS ${OPENMP_LLVM_TOOLS_DIR})
+ if (NOT OPENMP_FILECHECK_EXECUTABLE)
+ message(STATUS "Cannot find FileCheck.")
+ message(STATUS "Please put FileCheck in your PATH, set OPENMP_FILECHECK_E...
[truncated]
|
# Check if OMPT support is available | ||
# Currently, __builtin_frame_address() is required for OMPT | ||
# Weak attribute is required for Unices (except Darwin), LIBPSAPI is used for Windows | ||
if(NOT LIBOMP_ARCH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't there some header that's only built if we have OMPT support? Presumably the presence of that would indicate that whoever built OpenMP had OMPT support, or we could just make it an option, there's no explicit need for this to be automatically detected since a standalone build is supposed to be manual.
offload/CMakeLists.txt
Outdated
if (OPENMP_STANDALONE_BUILD OR "${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}") | ||
set(OPENMP_STANDALONE_BUILD TRUE) | ||
project(omptarget C CXX ASM) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant the project
name as well.
What is this pattern
if (OPENMP_STANDALONE_BUILD)
set(OPENMP_STANDALONE_BUILD TRUE)
endif()
Seems wrong.
@@ -35,11 +42,12 @@ endif() | |||
# TODO: Leftover from the move, could probably be just LLVM_LIBDIR_SUFFIX everywhere. | |||
set(OFFLOAD_INSTALL_LIBDIR "lib${LLVM_LIBDIR_SUFFIX}") | |||
|
|||
set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/cmake) | |||
set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't make sense to me, the whole point of a standalone build is that it's separate from LLVM, which would mean that this is the top level directory in the tree. Why are we just assuming that the LLVM modules directory is there?
I don't think putting this module stuff is necessary since we can just make the users pass in OMPTARGET_OMPT_ENABLED
or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok to assume the openmp sources are still inside the llvm-project, right? It is standalone in the sense that it will not be built during the same build as llvm. This change was more so a correction on the current offload patch ${CMAKE_CURRENT_SOURCE_DIR}/cmake
== offload/cmake
, which is not what LLVM_COMMON_CMAKE_UTILS
is supposed to reference.
I was just keeping the autodetection in for OMPT as the system configuration still needs checked to verify whether it can be supported, regardless if the user passes it in or not.
offload/CMakeLists.txt
Outdated
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/../openmp/runtime/cmake") | ||
include(OffloadOpenmpCommon) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, we're assuming that this is in an LLVM directory despite the fact that this is supposed to be "standalone".
offload/CMakeLists.txt
Outdated
omp-tools.h | ||
HINTS | ||
${LLVM_INCLUDE_DIRS} | ||
${LLVM_INCLUDE_DIRS}/../lib/clang/${LLVM_VERSION_MAJOR}/include |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LLVM_VERSION_MAJOR
won't be set if this doesn't go through the LLVM CMake. You'll need to call --print-resource-dir
if the compiler is clang.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be set from the LLVMConfig.cmake from the result of find_package(LLVM). I did query clang --print-resource-dir
in a version before this but thought this was a cleaner way.
offload/CMakeLists.txt
Outdated
@@ -214,6 +226,31 @@ set(LIBOMPTARGET_GPU_LIBC_SUPPORT ${LLVM_LIBC_GPU_BUILD} CACHE BOOL | |||
"Libomptarget support for the GPU libc") | |||
pythonize_bool(LIBOMPTARGET_GPU_LIBC_SUPPORT) | |||
|
|||
if(OPENMP_STANDALONE_BUILD) | |||
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we already do find_package(LLVM)
somewhere else? And why only Clang.
@@ -33,7 +33,7 @@ endif() | |||
|
|||
# Install plugin under the lib destination folder. | |||
install(TARGETS omptarget.rtl.${machine} | |||
LIBRARY DESTINATION "${OPENMP_INSTALL_LIBDIR}") | |||
LIBRARY DESTINATION "${OFFLOAD_INSTALL_LIBDIR}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this the only one that changed? In any case this will be completely gone if we land the static library patches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should have been changed in the original offload patch, just caught it here.
openmp/runtime/cmake/config-ix.cmake
Outdated
@@ -18,6 +18,7 @@ include(CheckIncludeFiles) | |||
include(CheckSymbolExists) | |||
include(LibompCheckFortranFlag) | |||
include(LLVMCheckCompilerLinkerFlag) | |||
include(OffloadOpenmpCommon) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't think this is necessary. Just check the version script independently in offload/
and make OMPT support opt-in.
The standalone offload build will just search for the ompt header and if found then OMPT support will be enabled. This caused the LIBOMP_HAVE_VERSION_SCRIPT_FLAG detection and pythonize_bool to move into offload/CMakeLists.txt. If the cmake compiler is clang, get the resource directory for installation of openmp headers. find_package(LLVM) is already called in LibomptargetGetDependencies. Depends on llvm#75125
e82f077
to
adf2b51
Compare
Thanks @jhuber6 for the detailed review and @estewart08 for working on the patch. @jdoerfert latest patch is much smaller and cleaner, and should be easy to squash with your patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks better now, if it works for ROCm's purposes then let's ship it.
@saiislam @estewart08 Does this work for ROCm? Can I pick it up? |
Yep it's good for us. Once you get that applied on top of yours it should be fine to merge. |
The content of this PR has been pulled-in into the libomptarget->offload PR. |
The standalone offload build will just search for
the ompt header and if found then OMPT support will
be enabled. This caused the LIBOMP_HAVE_VERSION_SCRIPT_FLAG
detection and pythonize_bool to move into offload/CMakeLists.txt.
If the cmake compiler is clang, get the resource directory for
installation of openmp headers. find_package(LLVM) is already
called in LibomptargetGetDependencies.
Depends on #75125