rocrtst: Disabled NUMA async copy build by shwetagkhatri · Pull Request #3044 · ROCm/rocm-systems

shwetagkhatri · 2026-02-03T19:52:09Z

Forced ENABLE_COPY_NUMA off and excluded memory_async_copy_numa.cc from build

Motivation

Technical Details

JIRA ID

Test Plan

Test Result

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

shwetagkhatri · 2026-02-03T19:57:10Z

@dayatsin-amd ,
The build errors with the latest hwloc 2.10.0 and above, is coming up due to only this one API - hwloc_set_membind_nodeset.
So guarding this under the #ifdef will resolve TheRock build errors.

We must ofcourse update rocrtst APIs to use latest hwloc APIs "eventually."

Forced ENABLE_COPY_NUMA off and excluded memory_async_copy_numa.cc from build

Copilot

Pull request overview

Disables building the NUMA async copy performance test in rocrtst, while also restructuring rocrtst’s CMake logic around NUMA/hwloc discovery and RPATH handling.

Changes:

Gated NUMA async copy test registration/includes behind ENABLE_COPY_NUMA.
Forced ENABLE_COPY_NUMA OFF and excluded memory_async_copy_numa.cc from the build.
Refactored CMake NUMA/hwloc detection and modified build/install RPATH settings.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
projects/rocr-runtime/rocrtst/suites/test_common/main.cc	Wraps NUMA async copy header include and gtest entry in `#if ENABLE_COPY_NUMA`.
projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt	Forces NUMA test off, filters NUMA source, and refactors NUMA/hwloc detection + RPATH configuration.
projects/rocr-runtime/rocrtst/suites/performance/memory_async_copy_numa.h	Wraps the NUMA test class definition behind `#if ENABLE_COPY_NUMA`.
projects/rocr-runtime/rocrtst/suites/performance/memory_async_copy_numa.cc	Wraps NUMA test implementation behind `#if ENABLE_COPY_NUMA` and gates a membind call behind `ENABLE_COPY_NUMA_MEMBIND_NODESET`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-04T17:23:19Z

projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt

+if(NOT HAVE_HWLOC AND PKG_CONFIG_FOUND)
+  pkg_check_modules(HWLOC_PC hwloc)
+  if(HWLOC_PC_FOUND)
+    # Try to find the actual library file for SHARED IMPORTED target
+    find_library(HWLOC_LIBRARY_FILE
+      NAMES hwloc
+      HINTS ${HWLOC_PC_LIBRARY_DIRS}
+      NO_DEFAULT_PATH
+    )


The hwloc pkg-config path only succeeds if find_library(... NO_DEFAULT_PATH HINTS ${HWLOC_PC_LIBRARY_DIRS}) finds a full .so file. If HWLOC_PC_LIBRARY_DIRS is empty (common when the lib is in a default system path) or the .so symlink isn’t present, HAVE_HWLOC stays FALSE and the later fallback searches only ${ROCRTST_ROOT}/thirdparty/lib (also NO_DEFAULT_PATH), leading to a hard build failure even when system hwloc is installed. Suggest adding an INTERFACE imported target fallback using ${HWLOC_PC_LIBRARIES}/${HWLOC_PC_LIBRARY_DIRS} (like the NUMA logic) and/or dropping NO_DEFAULT_PATH so system paths are searched.

Copilot · 2026-02-04T17:23:19Z

projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt

+  INSTALL_RPATH "$ORIGIN/../lib/rocrtst/lib;$ORIGIN/../lib/rocm_sysdeps/lib"
+  BUILD_RPATH "$ORIGIN/../../dist/lib/rocm_sysdeps/lib;$ORIGIN/../lib/rocrtst/lib"


INSTALL_RPATH/BUILD_RPATH paths look inconsistent with what this CMakeLists installs: install(DIRECTORY .../thirdparty/lib DESTINATION lib/rocrtst) places libhwloc.so.5 under $prefix/lib/rocrtst/, but the new RPATH points to $ORIGIN/../lib/rocrtst/lib (extra /lib). This will prevent the installed binary from finding the bundled hwloc when the fallback is used. Recommend adjusting the RPATH to include $ORIGIN/../lib/rocrtst (and, if needed, $ORIGIN/../lib/rocrtst/thirdparty/lib), or aligning the install destination with the RPATH.

Suggested change

INSTALL_RPATH "$ORIGIN/../lib/rocrtst/lib;$ORIGIN/../lib/rocm_sysdeps/lib"

BUILD_RPATH "$ORIGIN/../../dist/lib/rocm_sysdeps/lib;$ORIGIN/../lib/rocrtst/lib"

INSTALL_RPATH "$ORIGIN/../lib/rocrtst;$ORIGIN/../lib/rocm_sysdeps/lib"

BUILD_RPATH "$ORIGIN/../../dist/lib/rocm_sysdeps/lib;$ORIGIN/../lib/rocrtst"

Copilot · 2026-02-04T17:23:19Z

projects/rocr-runtime/rocrtst/suites/performance/memory_async_copy_numa.cc

          "hwloc: membind not supported or cannot be enforced. Check errno.");
+#else
+    if (verbosity() >= VERBOSE_STANDARD) {
+      std::cout << "hwloc: membind nodeset not available in linked libhwloc; skipping."


The log message in the #else branch is misleading: this branch is selected when ENABLE_COPY_NUMA_MEMBIND_NODESET is disabled at compile time, not necessarily because the linked hwloc lacks nodeset membind support. Recommend rewording to reflect the build-time option (or detect support at runtime and print a message based on the detection result).

Suggested change

std::cout << "hwloc: membind nodeset not available in linked libhwloc; skipping."

std::cout << "hwloc: membind nodeset support disabled at build time "

<< "(ENABLE_COPY_NUMA_MEMBIND_NODESET=0); skipping."

Copilot · 2026-02-04T17:23:20Z

projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt

+# Force disabled by default to avoid building the NUMA test source.
+set(ENABLE_COPY_NUMA OFF CACHE BOOL "Build Memory_Async_Copy_NUMA test" FORCE)
+list(FILTER performanceSources EXCLUDE REGEX ".*memory_async_copy_numa\\.cc$")
+
+# Optional: enable hwloc membind nodeset call in NUMA test
+set(ENABLE_COPY_NUMA_MEMBIND_NODESET OFF CACHE BOOL "Enable hwloc membind nodeset in NUMA test")
+


set(ENABLE_COPY_NUMA ... FORCE) makes the option impossible to enable from the command line/GUI, but the surrounding comments call it “Optional”. Also, ENABLE_COPY_NUMA/ENABLE_COPY_NUMA_MEMBIND_NODESET are used in C++ with #if but are never added as compile definitions for the rocrtst target, so turning the CMake option ON would not actually enable the code. Recommend: remove FORCE, keep a single option(), gate source filtering on it, and propagate both options via target_compile_definitions() (e.g., ENABLE_COPY_NUMA=$<BOOL:...>).

Suggested change

# Force disabled by default to avoid building the NUMA test source.

set(ENABLE_COPY_NUMA OFF CACHE BOOL "Build Memory_Async_Copy_NUMA test" FORCE)

list(FILTER performanceSources EXCLUDE REGEX ".*memory_async_copy_numa\\.cc$")

# Optional: enable hwloc membind nodeset call in NUMA test

set(ENABLE_COPY_NUMA_MEMBIND_NODESET OFF CACHE BOOL "Enable hwloc membind nodeset in NUMA test")

option(ENABLE_COPY_NUMA "Build Memory_Async_Copy_NUMA test" OFF)

if(NOT ENABLE_COPY_NUMA)

# Exclude NUMA test source when NUMA copy tests are disabled.

list(FILTER performanceSources EXCLUDE REGEX ".*memory_async_copy_numa\\.cc$")

endif()

# Optional: enable hwloc membind nodeset call in NUMA test

option(ENABLE_COPY_NUMA_MEMBIND_NODESET "Enable hwloc membind nodeset in NUMA test" OFF)

# Propagate NUMA options to C++ sources.

if(ENABLE_COPY_NUMA)

add_definitions(-DENABLE_COPY_NUMA=1)

else()

add_definitions(-DENABLE_COPY_NUMA=0)

endif()

if(ENABLE_COPY_NUMA_MEMBIND_NODESET)

add_definitions(-DENABLE_COPY_NUMA_MEMBIND_NODESET=1)

else()

add_definitions(-DENABLE_COPY_NUMA_MEMBIND_NODESET=0)

endif()

Copilot · 2026-02-04T17:23:20Z

projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt

+# Always exclude memory_async_copy_numa.cc since ENABLE_COPY_NUMA is forced OFF
+message(STATUS "ENABLE_COPY_NUMA is OFF - excluding memory_async_copy_numa.cc from build")
+list(FILTER performanceSources EXCLUDE REGEX ".*memory_async_copy_numa\\.cc$")


This unconditionally excludes memory_async_copy_numa.cc regardless of ENABLE_COPY_NUMA, which makes the option ineffective and can lead to link/compile mismatches if someone tries to enable the test. Recommend: only list(FILTER ...) when NOT ENABLE_COPY_NUMA (and only print the STATUS message in that case).

Suggested change

# Always exclude memory_async_copy_numa.cc since ENABLE_COPY_NUMA is forced OFF

message(STATUS "ENABLE_COPY_NUMA is OFF - excluding memory_async_copy_numa.cc from build")

list(FILTER performanceSources EXCLUDE REGEX ".*memory_async_copy_numa\\.cc$")

# Exclude memory_async_copy_numa.cc when ENABLE_COPY_NUMA is OFF

if(NOT ENABLE_COPY_NUMA)

message(STATUS "ENABLE_COPY_NUMA is OFF - excluding memory_async_copy_numa.cc from build")

list(FILTER performanceSources EXCLUDE REGEX ".*memory_async_copy_numa\\.cc$")

endif()

I concur with copilot's comment--it looks like the source file is unconditionally excluded.

I don't think we need to exclude the .cc file. I would just have
ENABLE_COPY_NUMA_MEMBIND_NODESET and ENABLE_COPY_NUMA set to 0,

But I do not feel strongly about it. Up to you..

Yeah, you are right.. its already a disabled test. I will work on these review comments in the next PR. I will also clean and eliminate some redundant code in this CMakeFile file as well.

Copilot · 2026-02-04T17:23:20Z

projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt

+    else()
+      message(WARNING "NUMA library not found. Building without numa support.")
+      message(WARNING "  Install libnuma-dev (Debian/Ubuntu) or numactl-devel (RHEL/CentOS)")
+    endif()


When NUMA is not found you only emit a warning, but the build still compiles suites/performance/memory_async_copy*.cc, which include <numa.h> and call libnuma APIs. On systems without libnuma headers this will fail at compile time. Either make NUMA a required dependency (FATAL_ERROR with install hint) or keep the previous behavior of filtering out the memory_async_copy*.cc sources when NUMA isn’t available.

jharryma

rocrtst passing on this test PR: ROCm/TheRock#3236

71 tests ran and passed: https://github.com/ROCm/TheRock/actions/runs/21723147025/job/62672812484?pr=3236#step:10:2404

…copy

shwetagkhatri · 2026-02-05T22:10:08Z

@dayatsin-amd @cfreehill - This patch excludes memory_async_copy_numa.cc from build.
It also includes the other bigger patch from - #2598 - which you have already approved.

This patch has been tested successfully in TheRock's integrated build process by Jessy as confirmed here -ROCm/TheRock#3236
https://github.com/ROCm/TheRock/actions/runs/21723147025/job/62672812484?pr=3236

So instead of PR - #2598 - I would like to submit this PR. Please help provide your approval and complete the review here. Thank you!

cfreeamd · 2026-02-06T15:35:41Z

projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt

+endif()
+set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
+
+# Force disable NUMA copy functionality


Is this comment (and the one below) correct, and the message()? It looks like it's optional.
Does the message() print out whether ENABLE_COPY_NUMA is on or off?

cfreeamd · 2026-02-06T15:49:23Z

projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt

+# Always exclude memory_async_copy_numa.cc since ENABLE_COPY_NUMA is forced OFF
+message(STATUS "ENABLE_COPY_NUMA is OFF - excluding memory_async_copy_numa.cc from build")
+list(FILTER performanceSources EXCLUDE REGEX ".*memory_async_copy_numa\\.cc$")


I concur with copilot's comment--it looks like the source file is unconditionally excluded.

cfreeamd · 2026-02-06T15:58:26Z

I just have minor questions/comments. If rocrtst builds and runs successfully (at least the test in question) in CI and TheRock, then I would approve since it seems to be somewhat urgent. We can address the other issues (like testing the configurability of the tests wrt numa/hwloc), if any, afterwards.

…copy

dayatsin-amd · 2026-02-06T18:38:04Z

projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt


-set(CMAKE_BUILD_WITH_INSTALL_RPATH ON)
+# Configure RPATH settings before creating executable target
+set(CMAKE_BUILD_WITH_INSTALL_RPATH OFF)


Why do we need to change the rpath settings?

With CMAKE_BUILD_WITH_INSTALL_RPATH ON, TheRock build system was complaining about runtime linker failures when testing from the build directory:

./build/rocrtst64: error while loading shared libraries: libnuma.so.1: cannot open shared object file: No such file or directory

The binary was looking for libs at build/../lib/rocm_sysdeps/lib (install paths)

But in TheRock's build tree, numa/hwloc are actually at dist/lib/rocm_sysdeps/lib

Same for bundled thirdparty libs - not at lib/rocrtst yet, they are only copied there during install

With OFF -BUILD_RPATH is pointing to ../../dist/lib/rocm_sysdeps/lib and finds TheRock's libs

Also when they switch to INSTALL_RPATH after make install its able to finds installed libs at /opt/rocm/lib/rocrtst

Seemed to have worked in both scenarios without needing to install first.

Let me know if you see any more concerns.. We did a lot of iterations on this PR.. And this seems to be the Goldilocks Cmakefile!

@shwetagkhatri

Around 2 years ago, there was a task group that spent a few weeks to research and refine the RPATH settings for all the components in ROCm. I was not part of that discussion, but there were people who were very experienced in packaging/deployment involved. The current RPATH settings were based on the recommendations from that group, so I am hesitant with making RPATH changes.
Given that these changes only apply to rocrtst, which is a executable and not a library, we are probably ok, so we can ignore get back to it if we start seeing problems from this.

dayatsin-amd · 2026-02-06T18:41:23Z

projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt

+# Always exclude memory_async_copy_numa.cc since ENABLE_COPY_NUMA is forced OFF
+message(STATUS "ENABLE_COPY_NUMA is OFF - excluding memory_async_copy_numa.cc from build")
+list(FILTER performanceSources EXCLUDE REGEX ".*memory_async_copy_numa\\.cc$")


I don't think we need to exclude the .cc file. I would just have
ENABLE_COPY_NUMA_MEMBIND_NODESET and ENABLE_COPY_NUMA set to 0,

But I do not feel strongly about it. Up to you..

shwetagkhatri · 2026-02-06T18:48:05Z

I just have minor questions/comments. If rocrtst builds and runs successfully (at least the test in question) in CI and TheRock, then I would approve since it seems to be somewhat urgent. We can address the other issues (like testing the configurability of the tests wrt numa/hwloc), if any, afterwards.

Thanks @cfreehill. I would like to see TheRock build get unblocked as Step 1, since rocrtst's hwloc API link errors are blocking it. I will follow up on your review comments in the subsequent PRs. Thank you.

Forced ENABLE_COPY_NUMA off and excluded memory_async_copy_numa.cc from build Co-authored-by: Shweta Khatri <shweta.khatri@amd.com> [rocm-systems] ROCm/rocm-systems#3044 (commit de8012a)

Forced ENABLE_COPY_NUMA off and excluded memory_async_copy_numa.cc from build Co-authored-by: Shweta Khatri <shweta.khatri@amd.com>

chiranjeevipattigidi · 2026-02-11T11:27:11Z

@shwetagkhatri
In rock, CI checks for rocm-systems bump PR ROCm/TheRock#3307 failing with cmake errors.
(added some observations in the comments of above PR)

[rocrtst configure] CMake Error at /__w/TheRock/TheRock/cmake/therock_subproject_dep_provider.cmake:64 (message):
[rocrtst configure]   Project contains find_package(hwloc) for a package availabe in the
[rocrtst configure]   super-project but not declared: Add a BUILD_DEPS or RUNTIME_DEPS
[rocrtst configure]   appropriately
[rocrtst configure] Call Stack (most recent call first):

seems like bug in declaring the dependecies

Copilot AI review requested due to automatic review settings February 3, 2026 19:52

shwetagkhatri requested a review from cfreeamd as a code owner February 3, 2026 19:52

shwetagkhatri requested review from dayatsin-amd and removed request for Copilot February 3, 2026 19:52

github-actions bot added the project: rocr-runtime label Feb 3, 2026

Copilot started reviewing on behalf of shwetagkhatri February 3, 2026 19:52 View session

systems-assistant bot added the organization: ROCm label Feb 3, 2026

rocrtst: Disabled NUMA async copy build

08a1ff2

Forced ENABLE_COPY_NUMA off and excluded memory_async_copy_numa.cc from build

Copilot AI review requested due to automatic review settings February 4, 2026 17:17

shwetagkhatri force-pushed the users/shwetak/Disable_rocrtst_NUMA_async_copy branch from ecc8499 to 08a1ff2 Compare February 4, 2026 17:17

Copilot started reviewing on behalf of shwetagkhatri February 4, 2026 17:18 View session

Copilot AI reviewed Feb 4, 2026

View reviewed changes

jharryma approved these changes Feb 5, 2026

View reviewed changes

Merge branch 'develop' into users/shwetak/Disable_rocrtst_NUMA_async_…

75e454e

…copy

cfreeamd reviewed Feb 6, 2026

View reviewed changes

cfreeamd approved these changes Feb 6, 2026

View reviewed changes

Merge branch 'develop' into users/shwetak/Disable_rocrtst_NUMA_async_…

3123112

…copy

dayatsin-amd reviewed Feb 6, 2026

View reviewed changes

jharryma merged commit de8012a into develop Feb 6, 2026
5 of 8 checks passed

jharryma deleted the users/shwetak/Disable_rocrtst_NUMA_async_copy branch February 6, 2026 18:48

amd-josnarlo pushed a commit that referenced this pull request Feb 9, 2026

rocrtst: Disabled NUMA async copy build (#3044)

33cfd33

Forced ENABLE_COPY_NUMA off and excluded memory_async_copy_numa.cc from build Co-authored-by: Shweta Khatri <shweta.khatri@amd.com>

radhaksri pushed a commit that referenced this pull request Feb 9, 2026

Cherry pick de8012a (PR #3044), rocrtst: Disabled NUMA async copy build

893f007

jharryma mentioned this pull request Feb 12, 2026

[CI] Build and Enable rocrtst for ROCR-Runtime on theRock ROCm/TheRock#2498

Open

shwetagkhatri mentioned this pull request Feb 17, 2026

Revisit rocrtst RPATH change (CMAKE_BUILD_WITH_INSTALL_RPATH OFF) #3311

Open

shwetagkhatri mentioned this pull request Feb 17, 2026

rocrtst CI failure: hwloc dependency not declared to TheRock dep provider #3312

Closed

		INSTALL_RPATH "$ORIGIN/../lib/rocrtst/lib;$ORIGIN/../lib/rocm_sysdeps/lib"
		BUILD_RPATH "$ORIGIN/../../dist/lib/rocm_sysdeps/lib;$ORIGIN/../lib/rocrtst/lib"

	std::cout << "hwloc: membind nodeset not available in linked libhwloc; skipping."
	std::cout << "hwloc: membind nodeset support disabled at build time "
	<< "(ENABLE_COPY_NUMA_MEMBIND_NODESET=0); skipping."

-# Force disabled by default to avoid building the NUMA test source.
-set(ENABLE_COPY_NUMA OFF CACHE BOOL "Build Memory_Async_Copy_NUMA test" FORCE)
-list(FILTER performanceSources EXCLUDE REGEX ".*memory_async_copy_numa\\.cc$")
-# Optional: enable hwloc membind nodeset call in NUMA test
-set(ENABLE_COPY_NUMA_MEMBIND_NODESET OFF CACHE BOOL "Enable hwloc membind nodeset in NUMA test")
+option(ENABLE_COPY_NUMA "Build Memory_Async_Copy_NUMA test" OFF)
+if(NOT ENABLE_COPY_NUMA)
+  # Exclude NUMA test source when NUMA copy tests are disabled.
+  list(FILTER performanceSources EXCLUDE REGEX ".*memory_async_copy_numa\\.cc$")
+endif()
+# Optional: enable hwloc membind nodeset call in NUMA test
+option(ENABLE_COPY_NUMA_MEMBIND_NODESET "Enable hwloc membind nodeset in NUMA test" OFF)
+# Propagate NUMA options to C++ sources.
+if(ENABLE_COPY_NUMA)
+  add_definitions(-DENABLE_COPY_NUMA=1)
+else()
+  add_definitions(-DENABLE_COPY_NUMA=0)
+endif()
+if(ENABLE_COPY_NUMA_MEMBIND_NODESET)
+  add_definitions(-DENABLE_COPY_NUMA_MEMBIND_NODESET=1)
+else()
+  add_definitions(-DENABLE_COPY_NUMA_MEMBIND_NODESET=0)
+endif()

Conversation

shwetagkhatri commented Feb 3, 2026

Motivation

Technical Details

JIRA ID

Test Plan

Test Result

Submission Checklist

Uh oh!

shwetagkhatri commented Feb 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

jharryma left a comment

Choose a reason for hiding this comment

Uh oh!

shwetagkhatri commented Feb 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cfreeamd commented Feb 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shwetagkhatri commented Feb 6, 2026

Uh oh!

Uh oh!

chiranjeevipattigidi commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

chiranjeevipattigidi commented Feb 11, 2026 •

edited

Loading