Skip to content

fix: [pytorch] Resolve ROCm deps via RUNPATHs#6452

Open
venkat1361 wants to merge 4 commits into
mainfrom
users/venkat1361/rocprofiler-preload-fix
Open

fix: [pytorch] Resolve ROCm deps via RUNPATHs#6452
venkat1361 wants to merge 4 commits into
mainfrom
users/venkat1361/rocprofiler-preload-fix

Conversation

@venkat1361

@venkat1361 venkat1361 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Motivation

Commit 6a55012 added rocprofiler-sdk to PyTorch’s Linux startup preload list to fix nightly smoke tests after PyTorch/Kineto gained a runtime dependency on librocprofiler-sdk.so.1.

When PyTorch is launched under an external rocprofv3, the profiler may already have loaded and initialized one rocprofiler-sdk provider. PyTorch startup can then explicitly load the wheel-bundled rocprofiler-sdk, creating a duplicate-provider path in the same process. In that scenario, the rocprofv3 tool configuration path can be invoked after rocprofiler-sdk has already closed its configuration window, leading to:

ROCPROFILER_STATUS_ERROR_CONFIGURATION_LOCKED
Configuration request occurred outside of valid rocprofiler configuration period

This PR treats the original issue as a packaging/runtime dependency issue instead of an import-time preload issue. PyTorch’s ROCm-linked shared libraries should resolve their DT_NEEDED dependencies through relocatable wheel RUNPATHs.

JIRA ID: AIPROFSDK-969

Technical Details

This PR removes rocprofiler-sdk from PyTorch’s generated _rocm_init.py preload list.

After pytorch build step unpacks the generated torch wheel, finds ROCm-linked torch shared objects, removes non-relocatable absolute RUNPATH entries, appends $ORIGIN-relative RUNPATH entries pointing to the installed ROCm Python package library directories, and repacks the wheel before copying.

This lets dependencies such as librocprofiler-sdk.so.1 resolve through the dynamic loader instead of through explicit ctypes/rocm_sdk.initialize_process() preloading. If an external rocprofv3 has already loaded a compatible SDK provider with the same SONAME, the loader can use the already-loaded provider instead of PyTorch forcing another absolute SDK load.

This PR also updates package promotion logic so torch ELF RUNPATHs stay valid when ROCm package versions are promoted. Text files such as _rocm_init.py, version.py, and metadata were already updated during promotion, but RUNPATH strings embedded in ELF files also need their ROCm version fragments updated when promoting from nightly/RC versions to release versions.

Test Plan

  • Added test in build_tools/packaging/tests/promote_keep_list_test.py

Test Result

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Jul 9, 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
🔎 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 9, 2026
@therock-pr-bot

therock-pr-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

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

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.

Pull request overview

This PR changes PyTorch wheel packaging to resolve ROCm shared-library dependencies via relocatable ELF RUNPATHs (embedded in torch shared objects) instead of preloading rocprofiler-sdk at import/startup time, avoiding duplicate-provider initialization when users run PyTorch under external rocprofv3.

Changes:

  • Removes rocprofiler-sdk from PyTorch’s generated Linux preload list.
  • Post-processes the built torch wheel on Linux to strip absolute RUNPATH entries and append $ORIGIN-relative RUNPATHs pointing at ROCm Python package library directories.
  • Extends package promotion to also rewrite ROCm version fragments embedded in torch ELF RUNPATHs (with unit tests for the string rewrite helper).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
external-builds/pytorch/build_prod_wheels.py Drops rocprofiler-sdk preload and adds wheel repack step that patches torch .so RUNPATHs via patchelf.
build_tools/packaging/promote_packages.py Updates torch ELF RUNPATHs during version promotion so promoted wheels keep valid ROCm package paths.
build_tools/packaging/tests/promote_keep_list_test.py Adds unit tests covering ROCm version substitution within RUNPATH strings.

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

Comment thread external-builds/pytorch/build_prod_wheels.py Outdated
Comment thread external-builds/pytorch/build_prod_wheels.py Outdated
@venkat1361 venkat1361 changed the title [pytorch] Resolve ROCm deps via wheel RUNPATHs instead of preloading rocprofiler-sdk [pytorch] Resolve ROCm deps via RUNPATHs instead of preloading rocprofiler-sdk Jul 9, 2026
@therock-pr-bot therock-pr-bot Bot removed the Not ready to Review PR has unresolved policy failures — reviews blocked label Jul 9, 2026
@venkat1361 venkat1361 changed the title [pytorch] Resolve ROCm deps via RUNPATHs instead of preloading rocprofiler-sdk fix: [pytorch] Resolve ROCm deps via RUNPATHs instead of preloading rocprofiler-sdk Jul 9, 2026
@venkat1361 venkat1361 marked this pull request as ready for review July 9, 2026 19:03
@venkat1361 venkat1361 changed the title fix: [pytorch] Resolve ROCm deps via RUNPATHs instead of preloading rocprofiler-sdk fix: [pytorch] Resolve ROCm deps via RUNPATHs Jul 9, 2026
@venkat1361 venkat1361 requested a review from geomin12 July 10, 2026 14:59

@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 as it is passing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: TODO

Development

Successfully merging this pull request may close these issues.

3 participants