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

Revert "cmake: use HIP CONFIG on Windows" #67

Closed
wants to merge 1 commit into from

Conversation

heroxbd
Copy link

@heroxbd heroxbd commented Jul 29, 2023

This reverts commit 41491c9.

By the cmake documentation,

Do not write find modules for packages that themselves build with
CMake. Instead provide a CMake package configuration file with the
package itself.

HIP uses cmake to build. It should not have shipped a FindHIP.cmake (a cmake MODULE). A hip-config.cmake (a cmake CONFIG) is enough.

We should use CONFIG on both Windows and GNU/Linux. Thus the commit needs to be reverted.

Reference: https://cmake.org/cmake/help/latest/command/find_package.html

resolves #___

Summary of proposed changes:

This reverts commit 41491c9.

By the cmake documentation,

  Do not write find modules for packages that themselves build with
  CMake. Instead provide a CMake package configuration file with the
  package itself.

HIP uses cmake to build.  It should not have shipped a FindHIP.cmake (a
cmake MODULE).  A hip-config.cmake (a cmake CONFIG) is enough.

We should use CONFIG on both Windows and GNU/Linux. Thus the commit
needs to be reverted.

Reference: https://cmake.org/cmake/help/latest/command/find_package.html
Signed-off-by: Benda Xu <heroxbd@gentoo.org>
@heroxbd
Copy link
Author

heroxbd commented Jul 29, 2023

@evetsso would you please help review this commit? Thanks for your attention.

Maybe I have missed an important point that a GNU/Linux hipFFT has to search for HIP by FindHIP.cmake. Please enlighten me.

@heroxbd
Copy link
Author

heroxbd commented Jul 29, 2023

I see why it is the way it is at #66 (review)

This merge request should be closed as a duplication of !66

@heroxbd heroxbd closed this Jul 29, 2023
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.

1 participant