Skip to content

fix(rocprofiler-sdk): TheRock fixes for SQLite3 include paths and rocm_sysdeps in RUNPATH - #9670

Merged
yhuiYH merged 4 commits into
developfrom
users/jrmadsen/rocprof-sdk-include-and-rpath-fixes
Oct 8, 2026
Merged

yhuiYH merged 4 commits into
developfrom
users/jrmadsen/rocprof-sdk-include-and-rpath-fixes

Conversation

@jrmadsen

@jrmadsen jrmadsen commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Motivation

Fix a few build system issues.

Technical Details

Issue Tracking

ROCM-28966

Test Plan

Test Result

Submission Checklist

Copilot AI lite review requested due to automatic review settings August 4, 2026 19:11
@jrmadsen
jrmadsen requested review from a team as code owners August 4, 2026 19:11
@therock-pr-bot

therock-pr-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown

✅ All Policy Checks Passed

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: projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/list-avail/rocprofv3_avail.cpp, projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/list-avail/rocprofv3_avail.hpp; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

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

🙋 Wish to Override Policy?

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 addresses build-system integration issues in rocprofiler-sdk by reorganizing the rocprofv3-list-avail tool library, improving runtime search paths to include rocm_sysdeps, and ensuring SQLite3 headers are available to the output library without forcing SQLite linkage.

Changes:

  • Move/build rocprofv3-list-avail under rocprofiler-sdk-tool/list-avail and adjust CMake subdirectories accordingly.
  • Extend several INSTALL_RPATH/default RPATH settings to include $ORIGIN/.../rocm_sysdeps/lib.
  • Improve SQLite3 dependency wiring and add a *-nolink interface target to propagate SQLite3 include paths without linking.

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/CMakeLists.txt Stops building the old rocprofiler-avail subdir from the core SDK lib tree.
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/list-avail/rocprofv3_avail.hpp Introduces a new exported C API header for the list-avail shared library.
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/list-avail/rocprofv3_avail.cpp Implements the exported C API for list-avail functionality (noted issues require fixes).
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/list-avail/CMakeLists.txt Adds build/install rules and updated INSTALL_RPATH for rocprofv3-list-avail.
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/CMakeLists.txt Adds list-avail subdirectory to the tool build.
projects/rocprofiler-sdk/source/lib/python/CMakeLists.txt Extends Python library RPATH defaults to include rocm_sysdeps.
projects/rocprofiler-sdk/source/lib/output/CMakeLists.txt Links output library to the SQLite3 nolink interface to obtain sqlite3.h include paths.
projects/rocprofiler-sdk/external/CMakeLists.txt Makes SQLite3 target detection more robust and configures a SQLite3 “nolink” interface target.
projects/rocprofiler-sdk/cmake/rocprofiler_interfaces.cmake Declares an imported rocprofiler-sdk-sqlite3-nolink interface target.

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

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

  • Add new commits
  • Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

@github-actions github-actions Bot added the Stale PR has no activity for 25+ days label Aug 30, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

This pull request has been automatically closed due to inactivity (30 days with no updates).

If you'd like to continue working on this, feel free to reopen the PR or create a new one.

@github-actions github-actions Bot closed this Sep 5, 2026
@jrmadsen jrmadsen reopened this Sep 30, 2026
@jrmadsen
jrmadsen force-pushed the users/jrmadsen/rocprof-sdk-include-and-rpath-fixes branch from c0e6c63 to 0fd2a39 Compare September 30, 2026 19:12
@jrmadsen jrmadsen removed the Stale PR has no activity for 25+ days label Sep 30, 2026
jrmadsen and others added 4 commits October 7, 2026 12:49
- this source should be in `source/lib/rocprofiler-sdk-tool/list-avail`
- important for Python bindings
- added to rocprofv3 tool and rocprofv3 list avail
- #6664 removed the include directory for rocprofiler-sdk-output-library
- detailed explanation in ROCm/TheRock#5868
- added rocprofiler-sdk-sqlite3-nolink target
Copilot AI lite review requested due to automatic review settings October 7, 2026 17:38
@yhuiYH
yhuiYH force-pushed the users/jrmadsen/rocprof-sdk-include-and-rpath-fixes branch from 0fd2a39 to a944aae Compare October 7, 2026 17:38

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.

🟡 Changes recommended

Unresolved critical SQLite linkage and missing API implementation, plus moderate correctness and resource-cleanup issues.

1 open finding

🧠 Review effort: Lite

@bernardogv

Copy link
Copy Markdown
Contributor

Tested this on a clean Ubuntu 26.04 build for TheRock #5868. It fixes it.

Setup: TheRock 4c6a8039 (rocm-systems d08190589c), Ubuntu 26.04.1, -DTHEROCK_AMDGPU_TARGETS=gfx1151.
libsqlite3-dev removed, so there is no sqlite3.h in /usr/include or /usr/local/include. Each
run is ninja rocprofiler-sdk+expunge followed by ninja rocprofiler-sdk+build:

rocprofiler-sdk result
without this PR fails: extract_data_type.hpp:27:10: fatal error: 'sqlite3.h' file not found (9 sources)
with this PR applied builds (rc=0), header found through TheRock's bundled SQLite

The diff applied cleanly to that older rocm-systems revision. I only checked the build, not the RUNPATH
changes or rocprofv3-list-avail at runtime.

@yhuiYH
yhuiYH merged commit 62a555a into develop Oct 8, 2026
241 of 248 checks passed
@yhuiYH
yhuiYH deleted the users/jrmadsen/rocprof-sdk-include-and-rpath-fixes branch October 8, 2026 13:43
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

bernardogv added a commit to bernardogv/TheRock that referenced this pull request Oct 10, 2026
Both fixes (ROCm/rocm-systems#9670 and #13075) are in the rocm-systems
submodule since ROCm#8862.
anujshuk-amd pushed a commit that referenced this pull request Oct 12, 2026
…m_sysdeps in RUNPATH (#9670)

## Motivation

<!-- Explain the purpose of this PR and the goals it aims to achieve.
-->
Fix a few build system issues.

## Technical Details

<!-- Explain the changes along with any relevant GitHub links. -->
- Minor reorganization to location of rocprofv3-list-avail tool library
- Include `$ORIGIN/.../rocm_sysdeps/lib` in various RUNPATH
- Ensure `rocprofiler-sdk-output-library` target is built with include
path to `sqlite3.h`
  - details in ROCm/TheRock#5868

## Issue Tracking

<!-- Include a GitHub Issue and/or JIRA ID. Do not post any full JIRA
links here. -->
<!-- GitHub issue: #1234 -->
<!-- JIRA ID: EXAMPLECOMPONENT-1234 -->
ROCM-28966

## Test Plan

<!-- Explain any relevant testing done to verify this PR. -->

## Test Result

<!-- Briefly summarize test outcomes. -->

## Submission Checklist

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

---------

Co-authored-by: Young Hui <young.hui@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants