[rocprofiler-systems] Add third-party dependencies #2657
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -146,6 +146,7 @@ if(THEROCK_ENABLE_PRIM) | |
| rocm-cmake | ||
| rocPRIM | ||
| therock-googletest | ||
| therock-tbb | ||
| RUNTIME_DEPS | ||
| hip-clr | ||
| ) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbonnell-amd marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,19 @@ | ||
| # Installs a limited subset of boost sufficient to build ROCM subprojects. | ||
| # Currently, this includes: | ||
| # atomic | ||
| # chrono | ||
| # date_time | ||
| # filesystem | ||
| # multi_index | ||
| # system | ||
| # thread | ||
| # timer | ||
| # Static libraries only | ||
| # Release build | ||
| # Multithreaded | ||
| # See cmake_project/CMakeLists.txt for details. | ||
|
|
||
|
|
||
| set(_source_dir "${CMAKE_CURRENT_BINARY_DIR}/source") | ||
| set(_download_stamp "${_source_dir}/download.stamp") | ||
|
|
||
|
|
@@ -29,7 +35,7 @@ therock_cmake_subproject_declare(therock-boost | |
| OUTPUT_ON_FAILURE | ||
| CMAKE_ARGS | ||
| "-DBOOST_SOURCE_DIR=${_source_dir}" | ||
| "-DTHEROCK_BOOST_LIBRARIES=atomic,chrono,date_time,filesystem,multi_index,system,thread,timer" | ||
|
Comment on lines
-32
to
+38
Member
There was a problem hiding this comment.
We really shouldn't be expanding our dependency on boost, especially not for relatively simple utilities like these. This is especially problematic if the dependency is indirect, as that could make migrating away more difficult. Should we consider these "grandfathered in" and allow them to be added here, or draw a line and reject them? Either way, the boost dependency must be removed eventually - is the rocprofiler-systems team prioritizing that?
Collaborator
There was a problem hiding this comment. I wish we weren't. But we need to get the whole gang in place before giving any of them a haircut. |
||
| EXTRA_DEPENDS | ||
| "${_download_stamp}" | ||
| ) | ||
|
|
@@ -41,9 +47,13 @@ set(_boost_version 1.87.0) | |
| # kept in sync with above library list. | ||
| therock_cmake_subproject_provide_package(therock-boost Boost "lib/cmake/Boost-${_boost_version}") | ||
| therock_cmake_subproject_provide_package(therock-boost boost_atomic "lib/cmake/boost_atomic-${_boost_version}") | ||
| therock_cmake_subproject_provide_package(therock-boost boost_chrono "lib/cmake/boost_chrono-${_boost_version}") | ||
| therock_cmake_subproject_provide_package(therock-boost boost_date_time "lib/cmake/boost_date_time-${_boost_version}") | ||
| therock_cmake_subproject_provide_package(therock-boost boost_filesystem "lib/cmake/boost_filesystem-${_boost_version}") | ||
| therock_cmake_subproject_provide_package(therock-boost boost_headers "lib/cmake/boost_headers-${_boost_version}") | ||
| therock_cmake_subproject_provide_package(therock-boost boost_system "lib/cmake/boost_system-${_boost_version}") | ||
| therock_cmake_subproject_provide_package(therock-boost boost_thread "lib/cmake/boost_thread-${_boost_version}") | ||
| therock_cmake_subproject_provide_package(therock-boost boost_timer "lib/cmake/boost_timer-${_boost_version}") | ||
| therock_cmake_subproject_activate(therock-boost) | ||
|
|
||
| add_dependencies(therock-third-party therock-boost) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| therock_subproject_fetch(therock-dyninst-sources | ||
| CMAKE_PROJECT | ||
| # Originally mirrored from: https://github.com/dyninst/dyninst/archive/refs/tags/v13.0.0.tar.gz | ||
| URL https://rocm-third-party-deps.s3.us-east-2.amazonaws.com/dyninst-13.0.0.tar.gz | ||
| URL_HASH SHA256=1bc48d26478b677a6c090c25586a447507bd1b4cf88d369bd61820005ce1be39 | ||
| ) | ||
|
|
||
| therock_cmake_subproject_declare(therock-dyninst | ||
| BACKGROUND_BUILD | ||
| EXCLUDE_FROM_ALL | ||
| NO_MERGE_COMPILE_COMMANDS | ||
| OUTPUT_ON_FAILURE | ||
| EXTERNAL_SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}/source" | ||
| RUNTIME_DEPS | ||
| therock-boost | ||
| therock-tbb | ||
| ${THEROCK_BUNDLED_BINUTILS} | ||
| ${THEROCK_BUNDLED_ELFUTILS} | ||
| INSTALL_RPATH_DIRS | ||
| lib/rocm_sysdeps/lib | ||
| ) | ||
| therock_cmake_subproject_provide_package(therock-dyninst Dyninst lib/cmake) | ||
| therock_cmake_subproject_activate(therock-dyninst) | ||
|
|
||
| add_dependencies(therock-third-party therock-dyninst) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| if(NOT CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) | ||
| # When included in TheRock, we download sources and set up the sub-project. | ||
| set(_source_dir "${CMAKE_CURRENT_BINARY_DIR}/source") | ||
| set(_download_stamp "${_source_dir}/download.stamp") | ||
|
|
||
| therock_subproject_fetch(therock-binutils-sources | ||
| SOURCE_DIR "${_source_dir}" | ||
| # Originally mirrored from: "http://ftpmirror.gnu.org/gnu/binutils/binutils-2.45.tar.gz" | ||
| URL https://rocm-third-party-deps.s3.us-east-2.amazonaws.com/binutils-2.45.tar.gz | ||
| URL_HASH "SHA512=5c71ec80884c0b4c0f7a4f8600946f1f6feebe584261b63185b0942cba4062d5b6cf2337539132d0ca03505c0a74f3c2760bc62ed08843c3b7db03df7f5798ad" | ||
| TOUCH "${_download_stamp}" | ||
| ) | ||
|
|
||
| therock_cmake_subproject_declare(therock-binutils | ||
| USE_DIST_AMDGPU_TARGETS | ||
| EXTERNAL_SOURCE_DIR . | ||
| FPRINT_SOURCE_DIR "${_source_dir}" | ||
| FPRINT_FILE_GLOBS "${CMAKE_CURRENT_LIST_DIR}/*" | ||
| BINARY_DIR build | ||
| NO_MERGE_COMPILE_COMMANDS | ||
| BACKGROUND_BUILD | ||
| OUTPUT_ON_FAILURE | ||
| CMAKE_ARGS | ||
| "-DSOURCE_DIR=${_source_dir}" | ||
| "-DPATCHELF=${PATCHELF}" | ||
| "-DPython3_EXECUTABLE=${Python3_EXECUTABLE}" | ||
| INSTALL_DESTINATION | ||
| lib/rocm_sysdeps | ||
| INTERFACE_LINK_DIRS | ||
| lib/rocm_sysdeps/lib | ||
| INTERFACE_INSTALL_RPATH_DIRS | ||
| lib/rocm_sysdeps/lib | ||
| INTERFACE_PKG_CONFIG_DIRS | ||
| lib/rocm_sysdeps/lib/pkgconfig | ||
| EXTRA_DEPENDS | ||
| "${_download_stamp}" | ||
| ) | ||
|
|
||
| therock_cmake_subproject_provide_package(therock-binutils LibIberty lib/rocm_sysdeps/lib/cmake/libiberty) | ||
| therock_cmake_subproject_activate(therock-binutils) | ||
|
|
||
| return() | ||
| endif() | ||
|
|
||
| # Otherwise, this is the sub-project build. | ||
| cmake_minimum_required(VERSION 3.25) | ||
| project(BINUTILS_BUILD) | ||
|
|
||
| include(ProcessorCount) | ||
| ProcessorCount(PAR_JOBS) | ||
|
|
||
| if(NOT PATCHELF) | ||
| message(FATAL_ERROR "Missing PATCHELF from super-project") | ||
| endif() | ||
|
|
||
| set(EXTRA_CPPFLAGS) | ||
| string(APPEND EXTRA_CPPFLAGS " -fPIC") | ||
| message(STATUS "EXTRA_CPPFLAGS=${EXTRA_CPPFLAGS}") | ||
|
|
||
| add_custom_target( | ||
| build ALL | ||
| WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" | ||
| COMMAND | ||
| "${CMAKE_COMMAND}" -E rm -rf -- "${CMAKE_INSTALL_PREFIX}" "${CMAKE_CURRENT_BINARY_DIR}/s" | ||
| COMMAND | ||
| # We have to patch the sources so make a fresh copy. | ||
| "${CMAKE_COMMAND}" -E copy_directory "${SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}/s" | ||
| COMMAND | ||
| "${CMAKE_COMMAND}" -E env | ||
| "CFLAGS=${EXTRA_CPPFLAGS}" | ||
| "CPPFLAGS=${EXTRA_CPPFLAGS}" | ||
| -- | ||
| "${CMAKE_CURRENT_BINARY_DIR}/s/configure" | ||
| --prefix "${CMAKE_INSTALL_PREFIX}" | ||
| --enable-install-libiberty | ||
| --disable-shared | ||
|
Collaborator
There was a problem hiding this comment. Add a comment that this is only providing binaries, and even if GPL conditions are satisfied since nothing links to these. |
||
| --enable-static | ||
| --libdir="${CMAKE_INSTALL_PREFIX}/lib" | ||
| --includedir="${CMAKE_INSTALL_PREFIX}/include" | ||
| --disable-nls | ||
| --disable-werror | ||
| COMMAND | ||
| make -j 1 V=1 | ||
|
||
| COMMAND | ||
| make install | ||
| COMMAND | ||
| "${CMAKE_COMMAND}" -E env | ||
| "PATCHELF=${PATCHELF}" | ||
| "THEROCK_SOURCE_DIR=${THEROCK_SOURCE_DIR}" | ||
| "Python3_EXECUTABLE=${Python3_EXECUTABLE}" -- | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/patch_install.sh" ${CMAKE_INSTALL_PREFIX} | ||
|
|
||
| DEPENDS | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/patch_install.sh" | ||
| ) | ||
|
|
||
| # This matches the conventions of the FindLibIberty.cmake module. | ||
| # Other than that, there is so source of truth for this. | ||
| configure_file( | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/libiberty-config.cmake.in | ||
| ${CMAKE_CURRENT_BINARY_DIR}/libiberty-config.cmake | ||
| @ONLY | ||
| ) | ||
| install(FILES "${CMAKE_CURRENT_BINARY_DIR}/libiberty-config.cmake" DESTINATION lib/cmake/libiberty) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Traverse from lib/cmake/FOO -> the directory holding lib | ||
| get_filename_component(_IMPORT_PREFIX "${CMAKE_CURRENT_LIST_DIR}" PATH) | ||
| get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH) | ||
| get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH) | ||
| if(_IMPORT_PREFIX STREQUAL "/") | ||
| set(_IMPORT_PREFIX "") | ||
| endif() | ||
|
|
||
| if(NOT TARGET LibIberty::LibIberty) | ||
| add_library(LibIberty::LibIberty STATIC IMPORTED) | ||
|
Collaborator
There was a problem hiding this comment. libiberty is LGPL licensed. We have to use it as a shared library or not at all.
Member
There was a problem hiding this comment. @jbonnell-amd please ping once these comments are resolved and you want another review. |
||
| set_target_properties(LibIberty::LibIberty PROPERTIES | ||
| INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include;${_IMPORT_PREFIX}/include/libiberty" | ||
| IMPORTED_LOCATION "${_IMPORT_PREFIX}/lib/libiberty.a" | ||
| ) | ||
| endif() | ||
|
|
||
| set(_IMPORT_PREFIX) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| #!/bin/bash | ||
| # Patches installed binaries from the external build system. | ||
| # Args: install_dir patchelf_binary | ||
| set -e | ||
|
|
||
| PREFIX="${1:?Expected install prefix argument}" | ||
| PATCHELF="${PATCHELF:-patchelf}" | ||
| THEROCK_SOURCE_DIR="${THEROCK_SOURCE_DIR:?THEROCK_SOURCE_DIR not defined}" | ||
| Python3_EXECUTABLE="${Python3_EXECUTABLE:?Python3_EXECUTABLE not defined}" | ||
|
|
||
| echo "binutils::patch_install.sh - prefix - $PREFIX" | ||
|
|
||
| # Copy lib64 to lib if it exists | ||
| if [ -d "$PREFIX/lib64" ]; then | ||
| cp -r $PREFIX/lib64/* $PREFIX/lib/ | ||
| rm -rf $PREFIX/lib64 | ||
| fi | ||
|
|
||
| # We don't want library descriptors or binaries. | ||
| rm -f $PREFIX/lib/*.la | ||
|
|
||
| if [ -d "$PREFIX/lib/bfd-plugins" ]; then | ||
| rm -rf $PREFIX/lib/bfd-plugins | ||
| fi | ||
| if [ -d "$PREFIX/lib/gprofng" ]; then | ||
| rm -rf $PREFIX/lib/gprofng | ||
| fi | ||
| if [ -d "$PREFIX/bin" ]; then | ||
| rm -rf $PREFIX/bin | ||
| fi | ||
| if [ -d "$PREFIX/share" ]; then | ||
| rm -rf $PREFIX/share | ||
| fi | ||
| if [ -d "$PREFIX/etc" ]; then | ||
| rm -rf $PREFIX/etc | ||
| fi | ||
| if [ -d "$PREFIX/x86_64-pc-linux-gnu" ]; then | ||
| rm -rf $PREFIX/x86_64-pc-linux-gnu | ||
| fi |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
For some reason, rocthrust configure kept failing due to this error:
Project contains find_package(TBB) for a package available in the super-project but not declared: Add a BUILD_DEPS or RUNTIME_DEPS appropriatelyNot sure where this is coming from but added this for now as a workaround. Would appreciate some insight on what the root cause of this might be
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 am pretty sure that thrust must have an optional dependency on TBB and is probing for it. TheRock doesn't let that happen (for this reason). You can add TBB to thrust to unblock BUILD_DEPS to unblock this PR but we should really have an explicit flag to enable/disable the TBB dep there vs having implicit behavior like this.
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 dread TBB more than Boost 🗡️
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.
@jbonnell-amd can you file an issue and link it 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.
Done #2837