Skip to content

Openmp cmake config#1038

Merged
estewart08 merged 2 commits into
ROCm:developfrom
estewart08:openmp-cmake-config
Jul 7, 2025
Merged

Openmp cmake config#1038
estewart08 merged 2 commits into
ROCm:developfrom
estewart08:openmp-cmake-config

Conversation

@estewart08
Copy link
Copy Markdown
Contributor

@estewart08 estewart08 commented Jun 20, 2025

Summary of proposed changes:

  • First search for ROCm's libomp.so via openmp-config.cmake. This is what we would prefer instead of searching for a system libomp.so/libgomp.so and then manually adding in a ROCm lib path.
  • This methodology should still be RHEL-10 RPATH compliant.
  • This patch is applied ontop of explicit install rpath with ORIGIN #1015

TorreZuk
TorreZuk approved these changes Jun 20, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@TorreZuk TorreZuk left a comment

Choose a reason for hiding this comment

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

It looks good but need to test manually a bit to confirm. Can teams message for docker or other build to test against.

@estewart08
Copy link
Copy Markdown
Contributor Author

estewart08 commented Jun 20, 2025

This also will take care of a future situation where LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON is used when building llvm-project. When that option is turned on, libomp.so is installed into /opt/rocm/lib/llvm/lib/x86_64-unknown-linux-gnu. The openmp-config.cmake will be automatically updated with the new path.

@TorreZuk
Copy link
Copy Markdown
Contributor

TorreZuk commented Jun 24, 2025

@estewart08 this seems to work in my latest docker so are we still good to merge and cherry pick this?

@estewart08
Copy link
Copy Markdown
Contributor Author

estewart08 commented Jun 24, 2025

@TorreZuk, that is fine with me. Not sure if the failing checks need to run again.

@TorreZuk
Copy link
Copy Markdown
Contributor

TorreZuk commented Jun 24, 2025

@estewart08 not sure on the Azure failures, can you look as maybe the old cmake config is bad so causes the errors shown in the build log? Not sure how our other builds are all fine. Maybe rebase PR on develop and run again.

@estewart08 estewart08 force-pushed the openmp-cmake-config branch 3 times, most recently from c1924cc to e59e543 Compare July 3, 2025 03:32
@estewart08
Add support for finding ROCm's openmp config
b97e104 
The first attempt to locate libomp.so will be done via
find_package(OpenMP) in CONFIG mode. This will look for openmp-config.cmake
in ROCm, which will have the full path of libomp.so stored in 'openmp_LIB_INSTALL_DIR'.
This variable is calculated based on the location of the config file and
a suffix of 'openmp_LIB_DIR', which is gathered during build time. 'openmp_LIB_DIR'
usually resolves to 'lib' or 'lib/<clang-target>'.

Currently, find_package(OpenMP) only locates the system (non-ROCm)
libomp.so/libgomp.so and when found a path to ROCm is manually provided.
This is not really what we want to achieve, but will be left in as a fallback
if the config is not found.
@estewart08 estewart08 force-pushed the openmp-cmake-config branch from e59e543 to b97e104 Compare July 4, 2025 16:13
@estewart08 estewart08 merged commit fe3af3c into ROCm:develop Jul 7, 2025
8 of 11 checks passed
TorreZuk pushed a commit to TorreZuk/hipBLAS that referenced this pull request Jul 7, 2025
@estewart08 @TorreZuk
Merge pull request ROCm#1038 from estewart08/openmp-cmake-config
782047d 
OpenMP cmake config support along with RPATH updates.

(cherry picked from commit fe3af3c)
@TorreZuk TorreZuk mentioned this pull request Jul 7, 2025
Closed
This was referenced Jul 7, 2025
Closed
Closed
TorreZuk added a commit that referenced this pull request Jul 8, 2025
@TorreZuk @estewart08
Merge pull request #1038 from estewart08/openmp-cmake-config (#1043)
ad2e8cb 
OpenMP cmake config support along with RPATH updates.

(cherry picked from commit fe3af3c)

Co-authored-by: estewart08 <ethan.stewart@amd.com>
@TorreZuk TorreZuk mentioned this pull request Jul 9, 2025
Merged
TorreZuk added a commit to TorreZuk/hipBLAS that referenced this pull request Jul 14, 2025
@TorreZuk @estewart08
Merge pull request ROCm#1038 from estewart08/openmp-cmake-config (ROC…
2e4857a 
…m#1043)

OpenMP cmake config support along with RPATH updates.

(cherry picked from commit fe3af3c)

Co-authored-by: estewart08 <ethan.stewart@amd.com>
(cherry picked from commit ad2e8cb)
shahamed pushed a commit that referenced this pull request Jul 15, 2025
@TorreZuk @estewart08 @shahamed
Merge pull request #1038 from estewart08/openmp-cmake-config (#1043)
6d22aa4 
OpenMP cmake config support along with RPATH updates.

(cherry picked from commit fe3af3c)

Co-authored-by: estewart08 <ethan.stewart@amd.com>
(cherry picked from commit ad2e8cb)
vamovsik added a commit that referenced this pull request Jul 23, 2025
@vamovsik
Merge pull request #1047 from TorreZuk/cherry-pick-omp
8d7ecf9 
CP merge pull request #1038 from estewart08/openmp-cmake-config (#1043)
amcamd added a commit that referenced this pull request Jul 24, 2025
@amcamd
Revert "CP merge pull request #1038 from estewart08/openmp-cmake-conf…
23ea2fa 
…ig (#1043)"
WBobby pushed a commit that referenced this pull request Jul 24, 2025
@amcamd
Revert "CP merge pull request #1038 from estewart08/openmp-cmake-conf…
e4ba998 
…ig (#1043)" (#1051)
rahulc-gh added a commit that referenced this pull request Jul 28, 2025
@rahulc-gh
Revert "Revert "CP merge pull request #1038 from estewart08/openmp-cm…
e4f06c3 
…ake-conf…"

This reverts commit e4ba998.
rahulc-gh added a commit that referenced this pull request Jul 28, 2025
@rahulc-gh
Revert "Revert "CP merge pull request #1038 from estewart08/openmp-cm…
bcd0217 
…ake-conf…" (#1052)

This reverts commit e4ba998.
ammallya pushed a commit that referenced this pull request Oct 28, 2025
@estewart08
Merge pull request #1038 from estewart08/openmp-cmake-config
4e71449 
OpenMP cmake config support along with RPATH updates.

[ROCm/hipBLAS commit: fe3af3c]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TorreZuk TorreZuk TorreZuk approved these changes

@amcamd amcamd Awaiting requested review from amcamd amcamd is a code owner

@mahmoodw mahmoodw Awaiting requested review from mahmoodw mahmoodw is a code owner

@daineAMD daineAMD Awaiting requested review from daineAMD

@bragadeesh bragadeesh Awaiting requested review from bragadeesh bragadeesh is a code owner

@rkamd rkamd Awaiting requested review from rkamd

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@estewart08 @TorreZuk