Adding TBB to sysdeps#4410
Open
ajanicijamd wants to merge 25 commits intomainfrom
Open
Conversation
marbre
reviewed
Apr 8, 2026
Member
marbre
left a comment
marbre
left a comment
There was a problem hiding this comment.
Some early feedback even though this is still a draft. Bundled sysdep might not be the right place. Is this linked statically or dynamically to rocprofiler?
Contributor
|
Please update the description's "motivation" and "technical details". Refer to #4399 for guidance. |
dgaliffiAMD
reviewed
Apr 9, 2026
dgaliffiAMD
added
the
test:rocprofiler-systems
dgaliffiAMD
reviewed
Apr 17, 2026
dgaliffiAMD
force-pushed
the
users/ajanicijamd/therock-add-tbb-to-sysdeps-2
branch
from
147fb10 to
ebb1ffc
Compare
…source directory, and installation paths for ROCm sysdeps. Adjusted package provision path for TBB.
Contributor
|
I found an issue with the installation of the TBB and pushed a fix. We still have a problem with the installation rpaths, as the Dyninst libraries cannot find the TBB libraries in their new location. |
Add `../llvm/lib` and `../rocm_sysdeps/lib`, so libraries under `rocprofiler-systems/lib` can find their dependent libraries.
Contributor
|
Moving to draft to address requirements after rebasing. |
TBB was added as a core sysdep but should follow the optional sysdep pattern established in fa6c993: gated behind THEROCK_ENABLE_SYSDEPS_TBB, with its own artifact descriptor and BUILD_TOPOLOGY entry. - Move add_subdirectory(tbb) and therock_provide_artifact(sysdeps-tbb) inside an THEROCK_ENABLE_SYSDEPS_TBB guard in linux/CMakeLists.txt - Remove TBB from the core sysdeps artifact.toml; add artifact-tbb.toml - Gate THEROCK_BUNDLED_TBB in root CMakeLists.txt behind the same flag - Add sysdeps-tbb entry to BUILD_TOPOLOGY.toml (required by topology validator) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TBB's own anonymous .def version scripts are incompatible with a named version script; the linker rejects mixing the two. version.lds is not used. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
The tbb sub-project build does not use Python. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Prevents the glob loop from running on a literal pattern if no matching libraries exist. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Verify that OUTPUT_NAME was actually injected after each sed. If a future oneTBB version changes the CMake layout, the build will fail immediately with a clear message instead of producing wrong library names. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Dyninst links against librocm_sysdeps_tbb.so.12 at runtime. The sysdeps-tbb artifact is optional and not part of the core sysdeps bundle, so it must be fetched explicitly alongside rocprofiler-systems. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Contributor
|
Moving back to "open" now that I believe the concerns have been addressed. I have also updated the description and made TBB an optional sysdep, following the pattern of other such dependencies (ex., #4647) |
dgaliffiAMD
reviewed
May 6, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Motivation
lib/rocm_sysdeps, consistent with other bundled third-party sysdeps.ROCPROFSYS_BUILD_TBB=ON). That can duplicate work and does not align with TheRock’s portable layout. This PR turns that off and consumestherock-tbbvia${THEROCK_BUNDLED_TBB}.Technical Details
sysdeps-tbbinBUILD_TOPOLOGY.toml(optional third-party sysdep pattern).rocprofiler-systemsdepends onsysdeps-tbbsoROCPROFSYSimplicitly requiresTHEROCK_ENABLE_SYSDEPS_TBBwhen features are finalized.third-party/sysdeps/linux/tbb/fetches oneTBB 2022.3.0 (mirrored tarball +URL_HASH). Custom install uses Ninja,TBB_TEST=OFF, install underlib/rocm_sysdeps.patch_source.shrewrites upstream CMake so shared libraries userocm_sysdeps_*OUTPUT_NAMEto avoid clashing with system TBB.profiler/CMakeLists.txt—ROCPROFSYS_BUILD_TBB=OFF,${THEROCK_BUNDLED_TBB}inRUNTIME_DEPS,INSTALL_RPATH_DIRSincludelib/rocm_sysdeps/lib.post_hook_rocprofiler-systems.cmakesetsTHEROCK_INSTALL_RPATH_ORIGINfor Dyninst-related targets so$ORIGIN/../rocm_sysdeps/libresolves correctly fromlib/rocprofiler-systems.third-party/sysdeps/linux/artifact-tbb.tomldescribes stage components for packaging.build_tools/install_rocm_from_artifacts.pyaddssysdeps-tbbfor--rocprofiler-systemsto ensure optional dependencies are installed for testing.Test Plan
librocm_sysdeps_tbb*.so*/libtbb*.so*are present underdist/rocm/lib/rocm_sysdeps/lib.dist/rocm/lib/rocprofiler-systems(no TBB leaked from the profiler subtree).librocm_sysdeps_tbb*/libtbb*,lddmust resolve TBB fromdist/rocm/lib/rocm_sysdeps/lib.dist/rocm/lib/rocprofiler-systems/lib*anddist/rocm/lib/librocprof-sys*; each library must include$ORIGIN/../rocm_sysdeps/libor$ORIGIN/rocm_sysdeps/lib, respectively.Test Result
Built successful, tests pass, metadata is correct.
Submission Checklist