Skip to content

Build Correctly In Presence of /opt/rocm #3825

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.

Sign up for GitHub

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

Open
linuxrocks123 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Build Correctly In Presence of /opt/rocm #3825

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions cmake/therock_subproject.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

include(ExternalProject)

set(ROCM_PATH ${THEROCK_BINARY_DIR}/dist/rocm)
Copy link
Copy Markdown
Member

@ScottTodd ScottTodd Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't generally work. Subproject builds are deliberately self-contained and isolated from one another. See https://github.com/ROCm/TheRock/blob/main/docs/development/build_system.md#build-directory-layout

  • dist/ : This is populated by the dist build phase by hard linking (or
    copying if hard-links are not possible) the cone of all runtime dep project's
    stage/ and this project's stage/ directory contents. In this way, each
    individual project's dist/ directory should consist of a self-contained
    slice that is relocatable and usable (for testing, etc). Top-level
    distributions are created in this same way by having runtime deps on all
    relevant sub-projects. Keeping sub-projects isolated in this way aids in
    testing, especially to ensure that dependencies are declared and layered
    properly.

Need to fix the subprojects that escape the sandbox and point at system paths.

Copy link
Copy Markdown
Contributor

@lumachad lumachad Mar 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we empty ROCM_PATH, ROCM_DIR, HIP_PATH and HIP_DIR on purpose so subprojects don't rely on those for their configs/builds?

For instance, one of out projects relied on ROCM_PATH for building tests, and I'd like that to fail rather than silently pass, as that gives us the opportunity to fix their behavior.

Copy link
Copy Markdown
Member

@marbre marbre Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do, for example at

TheRock/math-libs/CMakeLists.txt

Lines 15 to 17 in 65c64d0

-DHIP_PLATFORM=amd
-DROCM_PATH=
-DROCM_DIR=

but we might miss some places.


# Global properties.
# THEROCK_DEFAULT_CMAKE_VARS:
# List of CMake variables that will be injected by default into the
Expand All @@ -23,6 +25,10 @@ set_property(GLOBAL PROPERTY THEROCK_DEFAULT_CMAKE_VARS
THEROCK_ENABLE_LLVM_TESTS
LLVM_LIT_ARGS
THEROCK_USE_SAFE_DEPENDENCY_PROVIDER
HIP_DIR
HIP_PATH
ROCM_DIR
ROCM_PATH
ROCM_SYMLINK_LIBS

# RPATH handling.
Expand Down Expand Up @@ -705,10 +711,10 @@ function(therock_cmake_subproject_activate target_name)
# * Use `${HIP_PATH}` as a hint for `find_package()` calls
# We unset both the CMake and environment variables with these names.
# See also https://github.com/ROCm/TheRock/issues/670.
list(APPEND _build_env_pairs "--unset=ROCM_PATH")
list(APPEND _build_env_pairs "--unset=ROCM_DIR")
list(APPEND _build_env_pairs "--unset=HIP_PATH")
list(APPEND _build_env_pairs "--unset=HIP_DIR")
list(APPEND _build_env_pairs "ROCM_DIR=${THEROCK_BINARY_DIR}/dist/rocm")
list(APPEND _build_env_pairs "ROCM_PATH=${THEROCK_BINARY_DIR}/dist/rocm")
list(APPEND _build_env_pairs "HIP_PATH=${THEROCK_BINARY_DIR}/dist/rocm")
list(APPEND _build_env_pairs "HIP_DIR=${THEROCK_BINARY_DIR}/dist/rocm")

# Handle compiler toolchain.
set(_compiler_toolchain_addl_depends)
Expand Down