Skip to content

build: wire TEST_SUBPROJECTS for math-libs and ml-libs components - #6646

Closed
endurthiabhilash wants to merge 1 commit into
ROCm:mainfrom
endurthiabhilash:users/aendurth/add-test-subprojects-rocm-libs
Closed

endurthiabhilash wants to merge 1 commit into
ROCm:mainfrom
endurthiabhilash:users/aendurth/add-test-subprojects-rocm-libs

Conversation

@endurthiabhilash

@endurthiabhilash endurthiabhilash commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

As part of enabling multi-arch on rocm-libraries #3343, adding TEST_SUBPROJECTS entries to therock_cmake_subproject_declare blocks so downstream consumers are tracked for testing:

  • hipBLASLt: hipblasltprovider, MIOpen, rocBLAS
  • rocBLAS: hipblasltprovider, hipSPARSE, MIOpen, rocSOLVER, rocSPARSE, rocWMMA
  • rocSPARSE: rocPRIM, rocBLAS
  • hipBLAS: MIOpen, rocWMMA, hipSOLVER, hipSPARSE, rocSPARSE
  • rocPRIM: rocSOLVER
  • libhipcxx: rocThrust
  • composable_kernel: MIOpen
  • MIOpen: hipDNN_samples, hipdnn_integration_tests
  • miopenprovider: hipDNN_samples

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

Add TEST_SUBPROJECTS entries to therock_cmake_subproject_declare blocks
so downstream consumers are tracked for testing:

- hipBLASLt: hipblasltprovider, MIOpen, rocBLAS
- rocBLAS: hipblasltprovider, hipSPARSE, MIOpen, rocSOLVER, rocSPARSE, rocWMMA
- rocSPARSE: rocPRIM, rocBLAS
- hipBLAS: MIOpen, rocWMMA, hipSOLVER, hipSPARSE, rocSPARSE
- rocPRIM: rocSOLVER
- libhipcxx: rocThrust
- composable_kernel: MIOpen
- MIOpen: hipDNN_samples, hipdnn_integration_tests
- miopenprovider: hipDNN_samples

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
@therock-pr-bot

therock-pr-bot Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
🌿 Branch Name ✅ Pass —
📝 PR Title/Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

@therock-pr-bot therock-pr-bot Bot added the Not ready to Review PR has unresolved policy failures — reviews blocked label Jul 17, 2026
@therock-pr-bot

therock-pr-bot Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@therock-pr-bot therock-pr-bot Bot removed the Not ready to Review PR has unresolved policy failures — reviews blocked label Jul 17, 2026

@geomin12 geomin12 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm, but let's get component teams to review and approve as well pls

@geomin12 geomin12 added the ci:skip Skip all CI builds/tests for this PR label Jul 17, 2026
@ScottTodd

Copy link
Copy Markdown
Member

Wait why do we need a dedicated variable for this? Can't we get this information from the build graph?

If A depends on B, we should build/test A when B changes. We don't need B to be aware that A depends on it.

@geomin12

geomin12 commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

Wait why do we need a dedicated variable for this? Can't we get this information from the build graph?

If A depends on B, we should build/test A when B changes. We don't need B to be aware that A depends on it.

purpose of the variable is to get granular component and their dependencies. I wanted to use build_topology.toml, but it was too broad.

previously in rocm-libraries impl, for example, any blas (rocblas, hipblaslt, hipblas) update, we would build all of BLAS and test all of BLAS for all updates. however, ROCm devs let me know this is not efficient (ex: for rocblas updates, we don't need to test hipblaslt)! i looked around and couldn't find the granularity i wanted, so ended with this impl.

@endurthiabhilash and I are happy to place it somewhere else, or if there is a way to pull this data more dynamically, let me know. it's fairly hard-coded and minimal impact

@ScottTodd

Copy link
Copy Markdown
Member

Yeah, the build topology is not the right tool to use here.

In the CMake next to where TEST_SUBPROJECTS was added there are BUILD_DEPS and RUNTIME_DEPS lists. I think the inverse of this should work:

# For a list of targets, resolves several transitive properties relavent to build
# and runtime deps.
function(_therock_cmake_subproject_collect_build_deps
out_include_dirs
out_link_dirs
out_program_dirs
out_pkg_config_dirs
out_transitive_configure_depend_files)
set(_include_dirs)
set(_link_dirs)
set(_program_dirs)
set(_pkg_config_dirs)
set(_transitive_configure_depend_files)
foreach(target_name ${ARGN})
_therock_assert_is_cmake_subproject("${target_name}")
get_target_property(_declared_configure_depend_files "${target_name}" THEROCK_INTERFACE_CONFIGURE_DEPEND_FILES)
list(APPEND _transitive_configure_depend_files ${_declared_configure_depend_files})
get_target_property(_stamp_dir "${target_name}" THEROCK_STAMP_DIR)
# Include dirs.
get_target_property(_include_dir "${target_name}" THEROCK_INTERFACE_INCLUDE_DIRS)
list(APPEND _include_dirs ${_include_dir})
# Link dirs.
get_target_property(_link_dir "${target_name}" THEROCK_INTERFACE_LINK_DIRS)
list(APPEND _link_dirs ${_link_dir})
# Depend on stage installation.
get_target_property(_program_dir "${target_name}" THEROCK_INTERFACE_PROGRAM_DIRS)
if(_program_dir)
list(APPEND _program_dirs ${_program_dir})
list(APPEND _transitive_configure_depend_files "${_stamp_dir}/stage.stamp")
endif()
# PkgConfig dirs.
get_target_property(_pkg_config_dir "${target_name}" THEROCK_INTERFACE_PKG_CONFIG_DIRS)
if(_pkg_config_dir)
list(APPEND _pkg_config_dirs ${_pkg_config_dir})
endif()
endforeach()
set("${out_include_dirs}" "${_include_dirs}" PARENT_SCOPE)
set("${out_link_dirs}" "${_link_dirs}" PARENT_SCOPE)
set("${out_program_dirs}" "${_program_dirs}" PARENT_SCOPE)
set("${out_pkg_config_dirs}" "${_pkg_config_dirs}" PARENT_SCOPE)
set("${out_transitive_configure_depend_files}" "${_transitive_configure_depend_files}" PARENT_SCOPE)
endfunction()

that says "for a given project, what are all the transitive dependencies" when here you want "for a given project, what are all the transitive consumers". May need a subproject registry if one doesn't already exist to be able to compute that.

The explicit TEST_SUBPROJECTS list would let you do things like "even though rocBLAS depends on hipBLASLt, don't test it for changes to hipBLASLt", but IDK if we want that?

@geomin12

geomin12 commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

Yeah, the build topology is not the right tool to use here.

In the CMake next to where TEST_SUBPROJECTS was added there are BUILD_DEPS and RUNTIME_DEPS lists. I think the inverse of this should work:

# For a list of targets, resolves several transitive properties relavent to build
# and runtime deps.
function(_therock_cmake_subproject_collect_build_deps
out_include_dirs
out_link_dirs
out_program_dirs
out_pkg_config_dirs
out_transitive_configure_depend_files)
set(_include_dirs)
set(_link_dirs)
set(_program_dirs)
set(_pkg_config_dirs)
set(_transitive_configure_depend_files)
foreach(target_name ${ARGN})
_therock_assert_is_cmake_subproject("${target_name}")
get_target_property(_declared_configure_depend_files "${target_name}" THEROCK_INTERFACE_CONFIGURE_DEPEND_FILES)
list(APPEND _transitive_configure_depend_files ${_declared_configure_depend_files})
get_target_property(_stamp_dir "${target_name}" THEROCK_STAMP_DIR)
# Include dirs.
get_target_property(_include_dir "${target_name}" THEROCK_INTERFACE_INCLUDE_DIRS)
list(APPEND _include_dirs ${_include_dir})
# Link dirs.
get_target_property(_link_dir "${target_name}" THEROCK_INTERFACE_LINK_DIRS)
list(APPEND _link_dirs ${_link_dir})
# Depend on stage installation.
get_target_property(_program_dir "${target_name}" THEROCK_INTERFACE_PROGRAM_DIRS)
if(_program_dir)
list(APPEND _program_dirs ${_program_dir})
list(APPEND _transitive_configure_depend_files "${_stamp_dir}/stage.stamp")
endif()
# PkgConfig dirs.
get_target_property(_pkg_config_dir "${target_name}" THEROCK_INTERFACE_PKG_CONFIG_DIRS)
if(_pkg_config_dir)
list(APPEND _pkg_config_dirs ${_pkg_config_dir})
endif()
endforeach()
set("${out_include_dirs}" "${_include_dirs}" PARENT_SCOPE)
set("${out_link_dirs}" "${_link_dirs}" PARENT_SCOPE)
set("${out_program_dirs}" "${_program_dirs}" PARENT_SCOPE)
set("${out_pkg_config_dirs}" "${_pkg_config_dirs}" PARENT_SCOPE)
set("${out_transitive_configure_depend_files}" "${_transitive_configure_depend_files}" PARENT_SCOPE)
endfunction()

that says "for a given project, what are all the transitive dependencies" when here you want "for a given project, what are all the transitive consumers". May need a subproject registry if one doesn't already exist to be able to compute that.

The explicit TEST_SUBPROJECTS list would let you do things like "even though rocBLAS depends on hipBLASLt, don't test it for changes to hipBLASLt", but IDK if we want that?

ack! this hard-coded item is matching the therock-matrix.py (but with more guidance from ROCm component teams).

I'm planning to swap CI systems over for superrepos (so we get it done). this will work for now, then we can work on swapping this out this portion to dynamically select and make it more sophisticated

@endurthiabhilash and I will create GH issue

@geomin12 geomin12 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

as mentioned here (#6646 (comment)), this is not an ideal solution. However, this is a replicate of therock-matrix.py and this will apply to multi-arch.

After multi-arch migration happens, @endurthiabhilash and I are going to work to improve this retrieval to be dynamic. Especially if https://github.com/ROCm/TheRock/blob/main/docs/rfcs/RFC0003-Build-Tree-Normalization.md occurs, we want our solution to be workable for both CMakeLists scenarios

@endurthiabhilash

Copy link
Copy Markdown
Contributor Author

No longer needed as #6941 dynamically selects sub projects.

@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Oct 19, 2026
@endurthiabhilash
endurthiabhilash deleted the users/aendurth/add-test-subprojects-rocm-libs branch October 23, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:skip Skip all CI builds/tests for this PR

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants