Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CMake] Remove FindRoc* files #490

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

hjabird
Copy link
Contributor

@hjabird hjabird commented May 14, 2024

Description

This PR removes the files Findroc* to remove complexity. Additionally, it resolves an issue where HIP is not installed at the default location given by HIP_PATH. If the (as far as I now) undocumented HIP_PATH is not set, the build system (make, ninja) produces an error when the .so file is not found in the expected location.

The version in this PR seems to find the ROCm maths libraries by default and reduce the amount of CMake that we need to maintain.

The need for the files being deleted was not clear to me. Deleting them seemed like the most simple fix for the above issue, but we could also check the shared library exists and generate an error in the CMake instead.

Checklist

All Submissions

  • Do all unit tests pass locally? Attach a log.
  • [N/A] Have you formatted the code using clang-format?

testrng.txt
lapackres.txt
rocblas_test.log

@hjabird hjabird changed the title Remove FindRoc* files [CMake] Remove FindRoc* files May 15, 2024
@hjabird hjabird marked this pull request as ready for review May 29, 2024 09:44
@Rbiessy
Copy link
Contributor

Rbiessy commented May 29, 2024

Did you test to build this with CMake 1.13, the minimum version required by the root CMakeLists.txt? It occurred to me just now that older version of CMake may not be able to find these packages.
We know it works for rocfft, hopefully the other domains will work too.

@hjabird
Copy link
Contributor Author

hjabird commented May 29, 2024

Did you test to build this with CMake 1.13, the minimum version required by the root CMakeLists.txt? It occurred to me just now that older version of CMake may not be able to find these packages. We know it works for rocfft, hopefully the other domains will work too.

It didn't occur to me to do this. I've now built the library (tests disabled), and it builds with this patch with 3.13.

@hjabird hjabird merged commit be002ea into oneapi-src:develop Jun 3, 2024
6 checks passed
Rbiessy pushed a commit to Rbiessy/oneMKL that referenced this pull request Jun 7, 2024
* Some files are deleted.
* These files were not needed, but added potential for issues.
  * Eg. if HIP_PATH was not set, the shared library associated with the target would not be found.
normallytangent pushed a commit to normallytangent/oneMKL that referenced this pull request Aug 6, 2024
* Some files are deleted.
* These files were not needed, but added potential for issues.
  * Eg. if HIP_PATH was not set, the shared library associated with the target would not be found.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants