-
Notifications
You must be signed in to change notification settings - Fork 1.1k
refactor(rtcx): decouple librtcx from cudf and add standalone CMake build #22744
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
Changes from all commits
651d96b
07b43f1
17f476c
52756fe
0427310
24e8ee7
f5b458f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| # ============================================================================= | ||
| # cmake-format: off | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # cmake-format: on | ||
| # ============================================================================= | ||
|
|
||
| cmake_minimum_required(VERSION 4.0 FATAL_ERROR) | ||
|
|
||
| if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) | ||
| include(cmake/rapids_config.cmake) | ||
| include(rapids-cmake) | ||
| include(rapids-cpm) | ||
| rapids_cpm_init() | ||
| endif() | ||
|
|
||
| project( | ||
| rtcx | ||
| VERSION 0.1.0 | ||
| LANGUAGES CXX | ||
| ) | ||
|
|
||
| find_package(CUDAToolkit REQUIRED) | ||
|
|
||
| if(NOT TARGET zstd) | ||
| set(CPM_DOWNLOAD_zstd ON) | ||
| rapids_cpm_find( | ||
| zstd 1.5.7 | ||
| GLOBAL_TARGETS zstd | ||
| CPM_ARGS | ||
| GIT_REPOSITORY https://github.com/facebook/zstd.git | ||
| GIT_TAG v1.5.7 | ||
| GIT_SHALLOW FALSE SOURCE_SUBDIR build/cmake | ||
| OPTIONS "ZSTD_BUILD_STATIC ON" "ZSTD_BUILD_SHARED OFF" "ZSTD_BUILD_TESTS OFF" | ||
| "ZSTD_BUILD_PROGRAMS OFF" "BUILD_SHARED_LIBS OFF" | ||
| ) | ||
|
|
||
| if(zstd_ADDED) | ||
| # disable weak symbols support to hide tracing APIs as well | ||
| target_compile_definitions(libzstd_static PRIVATE ZSTD_HAVE_WEAK_SYMBOLS=0) | ||
| # expose experimental API | ||
| target_compile_definitions(libzstd_static PUBLIC ZSTD_STATIC_LINKING_ONLY=0N) | ||
| # suppress warnings from uninitialized variables and redefining ZSTD_STATIC_LINKING_ONLY | ||
| target_compile_options(libzstd_static PRIVATE -w) | ||
| add_library(zstd ALIAS libzstd_static) | ||
| endif() | ||
| endif() | ||
|
|
||
| if(NOT TARGET nvtx3::nvtx3-cpp) | ||
| include(${rapids-cmake-dir}/cpm/nvtx3.cmake) | ||
| rapids_cpm_nvtx3() | ||
| endif() | ||
|
|
||
| add_library(rtcx STATIC rtcx.cpp) | ||
| add_library(rtcx::rtcx ALIAS rtcx) | ||
|
|
||
| set_target_properties( | ||
| rtcx | ||
| PROPERTIES CXX_STANDARD 20 | ||
| CXX_STANDARD_REQUIRED YES | ||
| CXX_VISIBILITY_PRESET hidden | ||
| VISIBILITY_INLINES_HIDDEN YES | ||
| POSITION_INDEPENDENT_CODE ON | ||
| ) | ||
|
|
||
| target_include_directories( | ||
| rtcx | ||
| PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}> | ||
| PRIVATE ${CUDAToolkit_INCLUDE_DIRS} | ||
| ) | ||
|
|
||
| target_link_libraries(rtcx PRIVATE zstd ${CMAKE_DL_LIBS} nvtx3::nvtx3-cpp) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| # ============================================================================= | ||
| # cmake-format: off | ||
| # SPDX-FileCopyrightText: Copyright (c) 2021-2026, NVIDIA CORPORATION. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # cmake-format: on | ||
| # ============================================================================= | ||
| # | ||
| # This is the preferred entry point for projects using rapids-cmake | ||
| # | ||
| # Enforce the minimum required CMake version for all users | ||
| cmake_minimum_required(VERSION 4.0 FATAL_ERROR) | ||
|
|
||
| # Allow users to control which version is used | ||
| if(NOT (rapids-cmake-branch OR rapids-cmake-version)) | ||
| message( | ||
| FATAL_ERROR "The CMake variable `rapids-cmake-branch` or `rapids-cmake-version` must be defined" | ||
| ) | ||
| endif() | ||
|
|
||
| # Allow users to control which GitHub repo is fetched | ||
| if(NOT rapids-cmake-repo) | ||
| # Define a default repo if the user doesn't set one | ||
| set(rapids-cmake-repo rapidsai/rapids-cmake) | ||
| endif() | ||
|
|
||
| # Allow users to control which branch is fetched | ||
| if(NOT rapids-cmake-branch) | ||
| # Define a default branch if the user doesn't set one | ||
| set(rapids-cmake-branch "release/${rapids-cmake-version}") | ||
| endif() | ||
|
|
||
| # Allow users to control the exact URL passed to FetchContent | ||
| if(NOT rapids-cmake-url) | ||
| # Construct a default URL if the user doesn't set one | ||
| set(rapids-cmake-url "https://github.com/${rapids-cmake-repo}/") | ||
|
|
||
| # In order of specificity | ||
| if(rapids-cmake-fetch-via-git) | ||
| if(rapids-cmake-sha) | ||
| # An exact git SHA takes precedence over anything | ||
| set(rapids-cmake-value-to-clone "${rapids-cmake-sha}") | ||
| elseif(rapids-cmake-tag) | ||
| # Followed by a git tag name | ||
| set(rapids-cmake-value-to-clone "${rapids-cmake-tag}") | ||
| else() | ||
| # Or if neither of the above two were defined, use a branch | ||
| set(rapids-cmake-value-to-clone "${rapids-cmake-branch}") | ||
| endif() | ||
| else() | ||
| if(rapids-cmake-sha) | ||
| # An exact git SHA takes precedence over anything | ||
| set(rapids-cmake-value-to-clone "archive/${rapids-cmake-sha}.zip") | ||
| elseif(rapids-cmake-tag) | ||
| # Followed by a git tag name | ||
| set(rapids-cmake-value-to-clone "archive/refs/tags/${rapids-cmake-tag}.zip") | ||
| else() | ||
| # Or if neither of the above two were defined, use a branch | ||
| set(rapids-cmake-value-to-clone "archive/refs/heads/${rapids-cmake-branch}.zip") | ||
| endif() | ||
| endif() | ||
| endif() | ||
|
|
||
| include(FetchContent) | ||
| if(rapids-cmake-fetch-via-git) | ||
| FetchContent_Declare( | ||
| rapids-cmake | ||
| GIT_REPOSITORY "${rapids-cmake-url}" | ||
| GIT_TAG "${rapids-cmake-value-to-clone}" | ||
| ) | ||
| else() | ||
| string(APPEND rapids-cmake-url "${rapids-cmake-value-to-clone}") | ||
| FetchContent_Declare(rapids-cmake URL "${rapids-cmake-url}") | ||
| endif() | ||
| FetchContent_GetProperties(rapids-cmake) | ||
| if(rapids-cmake_POPULATED) | ||
| # Something else has already populated rapids-cmake, only thing we need to do is setup the | ||
| # CMAKE_MODULE_PATH | ||
| if(NOT "${rapids-cmake-dir}" IN_LIST CMAKE_MODULE_PATH) | ||
| list(APPEND CMAKE_MODULE_PATH "${rapids-cmake-dir}") | ||
| endif() | ||
| else() | ||
| FetchContent_MakeAvailable(rapids-cmake) | ||
| endif() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # ============================================================================= | ||
| # cmake-format: off | ||
| # SPDX-FileCopyrightText: Copyright (c) 2018-2026, NVIDIA CORPORATION. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # cmake-format: on | ||
| # ============================================================================= | ||
| file(READ "${CMAKE_CURRENT_LIST_DIR}/../../../VERSION" _rapids_version) | ||
| if(_rapids_version MATCHES [[^([0-9][0-9])\.([0-9][0-9])\.([0-9][0-9])]]) | ||
| set(RAPIDS_VERSION_MAJOR "${CMAKE_MATCH_1}") | ||
| set(RAPIDS_VERSION_MINOR "${CMAKE_MATCH_2}") | ||
| set(RAPIDS_VERSION_PATCH "${CMAKE_MATCH_3}") | ||
| set(RAPIDS_VERSION_MAJOR_MINOR "${RAPIDS_VERSION_MAJOR}.${RAPIDS_VERSION_MINOR}") | ||
| set(RAPIDS_VERSION "${RAPIDS_VERSION_MAJOR}.${RAPIDS_VERSION_MINOR}.${RAPIDS_VERSION_PATCH}") | ||
| else() | ||
| string(REPLACE "\n" "\n " _rapids_version_formatted " ${_rapids_version}") | ||
| message( | ||
| FATAL_ERROR | ||
| "Could not determine RAPIDS version. Contents of VERSION file:\n${_rapids_version_formatted}" | ||
| ) | ||
| endif() | ||
|
|
||
| file(STRINGS "${CMAKE_CURRENT_LIST_DIR}/../../../RAPIDS_BRANCH" RAPIDS_BRANCH) | ||
| if(NOT RAPIDS_BRANCH) | ||
| message( | ||
| FATAL_ERROR | ||
| "Could not determine branch name to use for checking out rapids-cmake. The file \"${CMAKE_CURRENT_LIST_DIR}/../../../RAPIDS_BRANCH\" is missing." | ||
| ) | ||
| endif() | ||
|
|
||
| if(NOT rapids-cmake-version) | ||
| set(rapids-cmake-version "${RAPIDS_VERSION_MAJOR_MINOR}") | ||
| endif() | ||
|
|
||
| if(NOT rapids-cmake-branch) | ||
| set(rapids-cmake-branch "${RAPIDS_BRANCH}") | ||
| endif() | ||
|
|
||
| include("${CMAKE_CURRENT_LIST_DIR}/RAPIDS.cmake") |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,8 +3,6 @@ | |||||
| * SPDX-License-Identifier: Apache-2.0 | ||||||
| */ | ||||||
|
|
||||||
| #include <cudf/logger.hpp> | ||||||
|
|
||||||
| #include <cuda.h> | ||||||
| #include <cuda_runtime_api.h> | ||||||
| #include <nvtx3/nvtx3.hpp> | ||||||
|
|
@@ -107,7 +105,7 @@ namespace rtcx { | |||||
| namespace { | ||||||
|
|
||||||
| struct nvtx_domain { | ||||||
| static constexpr char const* name = "rtcx"; | ||||||
| static constexpr char const* name [[maybe_unused]] = "rtcx"; | ||||||
| }; | ||||||
|
|
||||||
| enum class object_type : std::uint8_t { LIBRARY, BLOB }; | ||||||
|
|
@@ -154,12 +152,12 @@ std::string join_strings(std::span<StringType> strings, std::string_view separat | |||||
|
|
||||||
| void log_warning(std::string_view msg) | ||||||
| { | ||||||
| CUDF_LOG_WARN("%.*s", static_cast<std::int32_t>(msg.size()), msg.data()); | ||||||
| std::fprintf(stderr, "[rtcx] warn: %.*s\n", static_cast<int>(msg.size()), msg.data()); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Optional] Can/should this use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good question. I will take note of both this and your other comment to open as new issues on the new repo for next steps in this work.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Optional] Actually, just realized, if we're on a newer C++ standard, could this use
Suggested change
Also in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good suggestion. For this PR I kept fprintf to minimize the diff (just swapping out the cudf logger macro for a direct stderr call), but switching to |
||||||
| } | ||||||
|
Comment on lines
153
to
156
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optionally, in the future, we might want to turn this into a function/virtual object hook that can be set at runtime/compile time.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lamarrr Just to clarify, by "this" are you referring to the logging destination, a part of the log message, or something else?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed — a pluggable logging backend (e.g. a function pointer or virtual interface that consumers can set at init time) would be the right long-term approach. For this PR the goal was just to sever the cudf header dependency with the simplest possible replacement. I'll open an issue on the new repo to track making the logging destination configurable. |
||||||
|
|
||||||
| void log_error(std::string_view msg) | ||||||
| { | ||||||
| CUDF_LOG_ERROR("%.*s", static_cast<std::int32_t>(msg.size()), msg.data()); | ||||||
| std::fprintf(stderr, "[rtcx] error: %.*s\n", static_cast<int>(msg.size()), msg.data()); | ||||||
| } | ||||||
|
|
||||||
| #define FOR_EACH_CUDA_FUNC(DO_IT) \ | ||||||
|
|
||||||
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.
[Optional] Depending on the C++ standard you're targeting, this could be
static constexpr std::string_viewinstead… But I guess this is potentially a more pervasive change.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.
Yes, that is a potential change we could make. I think we'll be going through multiple rounds of iteration on librtcx once the code has been moved into its long-term home in a new repo.