Skip to content

Extend the hack to propagate HIP usage requirements a bit further.#147

Closed
systems-assistant[bot] wants to merge 2 commits intodevelopfrom
import/develop/taylding-amd_rocprofiler-sdk/the_rock_patch_3
Closed

Extend the hack to propagate HIP usage requirements a bit further.#147
systems-assistant[bot] wants to merge 2 commits intodevelopfrom
import/develop/taylding-amd_rocprofiler-sdk/the_rock_patch_3

Conversation

@systems-assistant
Copy link
Contributor

PR Details

Associated Jira Ticket Number/Link

Patch coming from TheRock: ROCm/TheRock#302

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Continuous Integration

Technical details

  • Also fetches usage requirements from hip::amdhip64 -> rocprofiler-sdk-hip-nolink
  • Adds rocprofiler-sdk-hip-nolink as a dependency of two libraries that indirectly depend on hip headers via hip.h.
  • The above may not be completely as precise as it can be (it seems like there should be an intermediate library for this of some kind).
  • Also conditions the link of hsa-amd-aqlprofile64_library on whether the library was found, which however might not be correct.

Added/updated tests?

  • Yes
  • No, Does not apply to this PR.

Updated CHANGELOG?

  • Yes
  • No, Does not apply to this PR.

Added/Updated documentation?

  • Yes
  • No, Does not apply to this PR.

🔁 Imported from ROCm/rocprofiler-sdk#65
🧑‍💻 Originally authored by @taylding-amd

ammallya pushed a commit that referenced this pull request Aug 7, 2025
Co-authored-by: Jonathan R. Madsen <jonathanrmadsen@gmail.com>

[ROCm/rocprofiler-sdk commit: 34145fa]
ywang103-amd pushed a commit to ywang103-amd/rocm-systems that referenced this pull request Aug 7, 2025
* Reapply "Upgrade ROCm-SMI to AMD SMI (ROCm#86)"

This reverts commit 9fcea73.

---------

Signed-off-by: Carrie Fallows <Carrie.Fallows@amd.com>
Co-authored-by: David Galiffi <David.Galiffi@amd.com>

[ROCm/rocprofiler-systems commit: 85bbea4]

target_sources(rocprofiler-sdk-common-library PRIVATE ${containers_sources}
${containers_headers})
target_link_libraries(rocprofiler-sdk-common-library PRIVATE rocprofiler-sdk-hip-nolink)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see why this is necessary. There are no headers included in the common library which rely on HIP that I can see.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems it is not needed. Addressed and removed this in #1086.

rocprofiler-sdk::rocprofiler-sdk-cereal
rocprofiler-sdk::rocprofiler-sdk-perfetto
rocprofiler-sdk::rocprofiler-sdk-otf2
rocprofiler-sdk-hip-nolink
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be formatted and you need to use the namespaced target.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #1086.

Comment on lines +134 to +139
# TODO: As of 2024/2/12, the hip::host target does not advertise its
# include directory but amdhip64 does. This ordinarily wouldn't be an issue
# because most folks just get it transitively, but here this is doing direct
# property copying to get usage requirements.
# The proper fix is for hip to export a hip::headers target with only usage
# requirements and depend on that.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is hip::host necessary anymore? Are you aware that rocprofiler_config_nolink_target is recursive? It recursively invokes itself for each CMake target linked to the initial target.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hip::host is not needed ad nolink target. Addressed in #1086.

@jrmadsen jrmadsen closed this Sep 30, 2025
ammallya pushed a commit that referenced this pull request Nov 17, 2025
* add API doc comments to goamdsmi.go
* update README and usage
* add sphinx directive to parse go doc
* fix walrus operator typos
* make docs more consistent
* add Go docs to index.md

---------

Signed-off-by: Arif, Maisam <Maisam.Arif@amd.com>
ammallya pushed a commit that referenced this pull request Nov 18, 2025
* add API doc comments to goamdsmi.go
* update README and usage
* add sphinx directive to parse go doc
* fix walrus operator typos
* make docs more consistent
* add Go docs to index.md

---------

Signed-off-by: Arif, Maisam <Maisam.Arif@amd.com>

[ROCm/amdsmi commit: 15c32f6]
ammallya pushed a commit that referenced this pull request Nov 21, 2025
* add API doc comments to goamdsmi.go
* update README and usage
* add sphinx directive to parse go doc
* fix walrus operator typos
* make docs more consistent
* add Go docs to index.md

---------

Signed-off-by: Arif, Maisam <Maisam.Arif@amd.com>

[ROCm/amdsmi commit: 15c32f6]
jamessiddeley-amd pushed a commit that referenced this pull request Dec 11, 2025
ammallya pushed a commit that referenced this pull request Jan 21, 2026
ammallya pushed a commit that referenced this pull request Jan 21, 2026
[ROCm/rocshmem commit: e0ef34a]
ammallya pushed a commit that referenced this pull request Jan 21, 2026
[SYNC] NCCL-Tests v2.16.7


[ROCm/rccl-tests commit: 690f97c]
ammallya pushed a commit that referenced this pull request Jan 30, 2026
…inx (#147)

Bumps [rocm-docs-core[api_reference]](https://github.com/ROCm/rocm-docs-core) from 1.18.2 to 1.18.4.
- [Release notes](https://github.com/ROCm/rocm-docs-core/releases)
- [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md)
- [Commits](ROCm/rocm-docs-core@v1.18.2...v1.18.4)

---
updated-dependencies:
- dependency-name: rocm-docs-core[api_reference]
  dependency-version: 1.18.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
ammallya pushed a commit that referenced this pull request Jan 30, 2026
…inx (#147)

Bumps [rocm-docs-core[api_reference]](https://github.com/ROCm/rocm-docs-core) from 1.18.2 to 1.18.4.
- [Release notes](https://github.com/ROCm/rocm-docs-core/releases)
- [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md)
- [Commits](ROCm/rocm-docs-core@v1.18.2...v1.18.4)

---
updated-dependencies:
- dependency-name: rocm-docs-core[api_reference]
  dependency-version: 1.18.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

[ROCm/rocjpeg commit: dfe2f0f]
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.

5 participants