Skip to content

fix(profiler): Drop removed rocprofiler-systems TBB dependency - #8834

Merged
sputhala-amd merged 2 commits into
mainfrom
users/sputhala/tbbFlagRemoval
Oct 9, 2026
Merged

sputhala-amd merged 2 commits into
mainfrom
users/sputhala/tbbFlagRemoval

Conversation

@sputhala-amd

@sputhala-amd sputhala-amd commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Rocprofiler-systems no longer depends on oneTBB. ROCm/rocm-systems#9876 removed the ROCPROFSYS_BUILD_TBB option after Dyninst stopped using TBB (ROCm/dyninst#26). TheRock still passed that flag and still suppressed vendored libtbb* RPM metadata for amdrocm-profiler (ROCM-28385).

The current rocm-systems pin already contains that removal, so the flag is unused and the RPM filter no longer matches anything that is installed.

JIRA ID : AIPROFSYST-545

Technical Details

  • Stop passing -DROCPROFSYS_BUILD_TBB=ON when configuring rocprofiler-systems. Dyninst is still built with -DROCPROFSYS_BUILD_DYNINST=ON.
  • Remove the amdrocm-profiler RPM __requires_exclude / __provides_exclude rules for libtbb.so.*, libtbbmalloc.so.*, and libtbbmalloc_proxy.so.*. Those libraries are no longer shipped, so the filter that kept vendored TBB from coupling the profiler package to distribution TBB is obsolete. This work was done as part of the PR: fix(packaging): Suppress vendored TBB provides on amdrocm-profiler RPM #7395

Test Plan

  • Confirm profiler/CMakeLists.txt no longer passes ROCPROFSYS_BUILD_TBB.
  • Confirm generated amdrocm-profiler RPM specs no longer exclude libtbb* Provides or Requires.
  • Let PR CI configure rocprofiler-systems against the current rocm-systems pin.

Test Result

Not run locally. Pending PR CI.

Submission Checklist

Copilot AI balanced review requested due to automatic review settings October 7, 2026 18:34
@therock-pr-bot

therock-pr-bot Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: build_tools/packaging/linux/rpm_package.py; no test file found
🔎 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.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Oct 7, 2026

Copy link
Copy Markdown

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

@sputhala-amd sputhala-amd changed the title fix(profiler): drop removed rocprofiler-systems TBB dependency fix(profiler): Drop removed rocprofiler-systems TBB dependency Oct 7, 2026

Copilot AI 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.

🟢 Approval recommended

The focused cleanup matches the current TBB-free rocprofiler-systems dependency model.

0 open findings

What changed in this PR

Removes obsolete oneTBB configuration and RPM filtering after rocprofiler-systems dropped the dependency.

Changes:

  • Stops passing the removed ROCPROFSYS_BUILD_TBB option.
  • Removes profiler-specific TBB RPM metadata exclusions.
  • Cleans up the unused template context variable.
File Description
profiler/​CMakeLists.txt Removes the obsolete CMake option.
build_tools/​packaging/​linux/​template/​rpm_specfile.j2 Removes TBB metadata filters.
build_tools/​packaging/​linux/​rpm_package.py Removes associated template configuration.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@dgaliffiAMD dgaliffiAMD 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.

Approving for the CMakeLists.txt change.
Can make one amendment to the description. Note that this reverts the workaround that was added with #7395.

@arvindcheru arvindcheru 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.

Approve. This drops -DROCPROFSYS_BUILD_TBB=ON and removes the #7395 RPM filter for vendored libtbb* on amdrocm-profiler. Dyninst stays enabled (-DROCPROFSYS_BUILD_DYNINST=ON).

Verified the pull-request build 37667903135 (https://therock-ci-artifacts.s3.amazonaws.com/37667903135-linux/packages/rpm/x86_64) of users/sputhala/tbbFlagRemoval at ec60493. amdrocm-profiler RPM has no tbb Provides, Requires, or filenames, and readelf -d on every packaged .so shows no TBB NEEDED entry. libdyninstAPI.so.13.0 is still in the package and does not link libtbb.

On OL 8.10 (Verified ROCM-28385 autoremove case) with kernel-uek-devel dnf install amdrocm-developer-tools10.2 from that repo succeeded, and dnf remove amdrocm-developer-tools10.2 removed Successfully - profiler and profiler-base with the rest of the stack.

Please wait for CI Tests results, and give time for other requested reviewers inputs. Thanks.

@arvindcheru arvindcheru 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.

The native installs (rhel8, rhel10, sles16, Ubuntu 24.04) all passed on the new run.

@sputhala-amd
sputhala-amd merged commit b1c1952 into main Oct 9, 2026
322 of 324 checks passed
@sputhala-amd
sputhala-amd deleted the users/sputhala/tbbFlagRemoval branch October 9, 2026 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants