Skip to content

fix(rocprofiler-compute): set RPATH origin for pc-sampling test binary - #6386

Merged
vedithal-amd merged 1 commit into
mainfrom
users/vedithal-amd/rocprofiler-compute-pc-sampling-test-rpath
Jul 7, 2026
Merged

vedithal-amd merged 1 commit into
mainfrom
users/vedithal-amd/rocprofiler-compute-pc-sampling-test-rpath

Conversation

@vedithal-amd

@vedithal-amd vedithal-amd commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

test-pc-sampling-collector fails at load with error while loading shared libraries: librocm_sysdeps_dw.so.1: cannot open shared object file. The unified test runner no longer adds sysdeps to LD_LIBRARY_PATH, so installed test binaries must resolve sysdeps via RPATH.

This test installs to libexec/rocprofiler-compute/tests/, not the default bin/. Without a correct THEROCK_INSTALL_RPATH_ORIGIN, TheRock computes its sysdeps RPATH relative to bin/, so librocm_sysdeps_dw.so.1 (in lib/rocm_sysdeps/lib) is not found.

Extend the post-hook to set THEROCK_INSTALL_RPATH_ORIGIN = libexec/rocprofiler-compute/tests for test-pc-sampling-collector, mirroring the existing handling of test-rocprofiler-compute-tool (#5108).

ISSUE ID: ROCm/rocm-systems#8186

Testing Summary

  • Verified against an existing build that test-rocprofiler-compute-tool RUNPATH already includes $ORIGIN/../../../lib/rocm_sysdeps/lib and ldd resolves librocm_sysdeps_dw.so.1 cleanly; this applies the same origin to the pc-sampling test.
  • CI to confirm test-pc-sampling-collector loads and passes.

Paired rocm-systems change (ROCm/rocm-systems#8191) removes the now-redundant hardcoded INSTALL_RPATH blocks from both test CMakeLists.

- Extend the post-hook to set THEROCK_INSTALL_RPATH_ORIGIN on
  test-pc-sampling-collector, matching test-rocprofiler-compute-tool.
- Both tests install to libexec/rocprofiler-compute/tests/, so TheRock
  can now compute correct relative RPATHs (including lib/rocm_sysdeps/lib)
  and resolve librocm_sysdeps_dw.so.1 at load time.

Co-Authored-By: Claude Opus 4
Copilot AI review requested due to automatic review settings July 7, 2026 15:49
@vedithal-amd vedithal-amd changed the title [rocprofiler-compute] Fix RPATH origin for pc-sampling test binary fix(rocprofiler-compute): set RPATH origin for pc-sampling test binary Jul 7, 2026
@vedithal-amd

Copy link
Copy Markdown
Contributor Author

Fixes ROCm/rocm-systems#8186

@therock-pr-bot

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

therock-pr-bot Bot commented Jul 7, 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 fixes runtime loading of the installed test-pc-sampling-collector binary by ensuring TheRock computes relative RPATH entries from the test’s actual install location (libexec/rocprofiler-compute/tests/) instead of the default executable origin (bin/). This aligns the pc-sampling test with the existing handling for test-rocprofiler-compute-tool, so sysdeps libraries under lib/rocm_sysdeps/lib can be found without relying on LD_LIBRARY_PATH.

Changes:

  • Extend the rocprofiler-compute post-hook to apply THEROCK_INSTALL_RPATH_ORIGIN=libexec/rocprofiler-compute/tests to both test-rocprofiler-compute-tool and test-pc-sampling-collector.
  • Refactor the per-target logic into a small foreach() loop with a TARGET existence guard.

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

@therock-pr-bot therock-pr-bot Bot removed the Not ready to Review PR has unresolved policy failures — reviews blocked label Jul 7, 2026
vedithal-amd added a commit to ROCm/rocm-systems that referenced this pull request Jul 7, 2026
- TheRock's post-process computes the authoritative RPATH for these test
  binaries (including lib/rocm_sysdeps/lib) relative to their install
  origin, overriding any local INSTALL_RPATH. The hardcoded blocks were
  no-ops under TheRock and omitted the sysdeps path.
- Origin handling for both tests now lives in TheRock's post-hook
  (ROCm/TheRock#6386).

Co-Authored-By: Claude Opus 4

@rahulc-gh rahulc-gh 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

@vedithal-amd

Copy link
Copy Markdown
Contributor Author

Since this is just a small change to rocprofiler-compute tool install hook - i am not waiting for windows and linux checks for compiler and runtime libs since those should not be affected by this

@vedithal-amd
vedithal-amd merged commit f6b9f70 into main Jul 7, 2026
54 of 60 checks passed
@vedithal-amd
vedithal-amd deleted the users/vedithal-amd/rocprofiler-compute-pc-sampling-test-rpath branch July 7, 2026 16:17
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Jul 7, 2026
abchoudh-amd added a commit to ROCm/rocm-systems that referenced this pull request Jul 9, 2026
…8191)

## Motivation

Bugfix: `test-pc-sampling-collector` fails in CI with `error while
loading shared libraries: librocm_sysdeps_dw.so.1: cannot open shared
object file`. The hardcoded `INSTALL_RPATH` on the test targets does not
fix this: TheRock's post-process overrides any local `INSTALL_RPATH`,
and the hardcoded path omitted `lib/rocm_sysdeps/lib` where the missing
library lives.

## Technical Details

- Remove the redundant `INSTALL_RPATH` blocks from
`pc_sampling_collector/tests/CMakeLists.txt` and
`rocprofiler_compute_tool/tests/CMakeLists.txt`.
- TheRock computes the authoritative RPATH (including sysdeps) for these
test binaries relative to their install origin. Origin handling for both
tests lives in TheRock's post-hook (paired PR ROCm/TheRock#6386).

JIRA ID - AIPROFCOMP-710

## Test Plan

- [ ] Run CI

## Test Result

- [ ] CI passes

## Submission Checklist

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

---------

Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com>
davidd-amd pushed a commit that referenced this pull request Jul 9, 2026
#6386)

## Summary

`test-pc-sampling-collector` fails at load with `error while loading
shared libraries: librocm_sysdeps_dw.so.1: cannot open shared object
file`. The unified test runner no longer adds sysdeps to
`LD_LIBRARY_PATH`, so installed test binaries must resolve sysdeps via
RPATH.

This test installs to `libexec/rocprofiler-compute/tests/`, not the
default `bin/`. Without a correct `THEROCK_INSTALL_RPATH_ORIGIN`,
TheRock computes its sysdeps RPATH relative to `bin/`, so
`librocm_sysdeps_dw.so.1` (in `lib/rocm_sysdeps/lib`) is not found.

Extend the post-hook to set `THEROCK_INSTALL_RPATH_ORIGIN =
libexec/rocprofiler-compute/tests` for `test-pc-sampling-collector`,
mirroring the existing handling of `test-rocprofiler-compute-tool`
(#5108).

ISSUE ID: ROCm/rocm-systems#8186

## Testing Summary

- Verified against an existing build that
`test-rocprofiler-compute-tool` RUNPATH already includes
`$ORIGIN/../../../lib/rocm_sysdeps/lib` and `ldd` resolves
`librocm_sysdeps_dw.so.1` cleanly; this applies the same origin to the
pc-sampling test.
- CI to confirm `test-pc-sampling-collector` loads and passes.

Paired rocm-systems change (ROCm/rocm-systems#8191) removes the
now-redundant hardcoded `INSTALL_RPATH` blocks from both test
CMakeLists.
cfallows-amd pushed a commit to ROCm/rocm-systems that referenced this pull request Jul 24, 2026
…8191)

## Motivation

Bugfix: `test-pc-sampling-collector` fails in CI with `error while
loading shared libraries: librocm_sysdeps_dw.so.1: cannot open shared
object file`. The hardcoded `INSTALL_RPATH` on the test targets does not
fix this: TheRock's post-process overrides any local `INSTALL_RPATH`,
and the hardcoded path omitted `lib/rocm_sysdeps/lib` where the missing
library lives.

## Technical Details

- Remove the redundant `INSTALL_RPATH` blocks from
`pc_sampling_collector/tests/CMakeLists.txt` and
`rocprofiler_compute_tool/tests/CMakeLists.txt`.
- TheRock computes the authoritative RPATH (including sysdeps) for these
test binaries relative to their install origin. Origin handling for both
tests lives in TheRock's post-hook (paired PR ROCm/TheRock#6386).

JIRA ID - AIPROFCOMP-710

## Test Plan

- [ ] Run CI

## Test Result

- [ ] CI passes

## Submission Checklist

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

---------

Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com>
vedithal-amd added a commit to ROCm/rocm-systems that referenced this pull request Jul 27, 2026
…8191)

## Motivation

Bugfix: `test-pc-sampling-collector` fails in CI with `error while
loading shared libraries: librocm_sysdeps_dw.so.1: cannot open shared
object file`. The hardcoded `INSTALL_RPATH` on the test targets does not
fix this: TheRock's post-process overrides any local `INSTALL_RPATH`,
and the hardcoded path omitted `lib/rocm_sysdeps/lib` where the missing
library lives.

## Technical Details

- Remove the redundant `INSTALL_RPATH` blocks from
`pc_sampling_collector/tests/CMakeLists.txt` and
`rocprofiler_compute_tool/tests/CMakeLists.txt`.
- TheRock computes the authoritative RPATH (including sysdeps) for these
test binaries relative to their install origin. Origin handling for both
tests lives in TheRock's post-hook (paired PR ROCm/TheRock#6386).

JIRA ID - AIPROFCOMP-710

## Test Plan

- [ ] Run CI

## Test Result

- [ ] CI passes

## Submission Checklist

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

---------

Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com>
amd-josnarlo pushed a commit to ROCm/rocm-systems that referenced this pull request Jul 28, 2026
…8191)

## Motivation

Bugfix: `test-pc-sampling-collector` fails in CI with `error while
loading shared libraries: librocm_sysdeps_dw.so.1: cannot open shared
object file`. The hardcoded `INSTALL_RPATH` on the test targets does not
fix this: TheRock's post-process overrides any local `INSTALL_RPATH`,
and the hardcoded path omitted `lib/rocm_sysdeps/lib` where the missing
library lives.

## Technical Details

- Remove the redundant `INSTALL_RPATH` blocks from
`pc_sampling_collector/tests/CMakeLists.txt` and
`rocprofiler_compute_tool/tests/CMakeLists.txt`.
- TheRock computes the authoritative RPATH (including sysdeps) for these
test binaries relative to their install origin. Origin handling for both
tests lives in TheRock's post-hook (paired PR ROCm/TheRock#6386).

JIRA ID - AIPROFCOMP-710

## Test Plan

- [ ] Run CI

## Test Result

- [ ] CI passes

## Submission Checklist

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

---------

Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com>
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