KomputeProject / kompute

General purpose GPU compute framework built on Vulkan to support 1000s of cross vendor graphics cards (AMD, Qualcomm, NVIDIA & friends). Blazing fast, mobile-enabled, asynchronous and optimized for advanced GPU data processing usecases. Backed by the Linux Foundation.
http://kompute.cc/
Apache License 2.0
1.88k stars 145 forks source link

Fix find_package, enable CPack and support clang-cl #338

Closed ToKiNoBug closed 9 months ago

ToKiNoBug commented 9 months ago

This PR aims to make kompute easier to be used as a C++ SDK.

  1. Add support to clang-cl Before this clang-cl is treated like g++ or clang++, and many -W flags are passed to it, but clang-cl is a MSVC-like frontend.

  2. Fix an error when kompute is being imported as a cmake package komputeTargets.cmake is required in a find_package call, but this file is not generated previously.

  3. Fix a bug to support fmtlib v10 fmtlib v10 doesn't think std::array<char,N> is formattable, so we use .data().

  4. Fix KOMPUTE_OPT_INSTALL. Previously this option doesn't work because install calls will always take effect regardless of the value of KOMPUTE_OPT_INSTALL. Now no kompute files will be installed if KOMPUTE_OPT_INSTALL is set to OFF.

  5. Enable CPack to generate many kinds of software packages like deb, rpm, zip, 7z, mac bundle, NSIS, etc. Configurations for many other kinds of packages can be added in future. To use cpack, run cpack -G <generator> in the build directory. For example:

    cd build
    cpack -G TXZ
    cpack -G DEB

    This will generate a deb package and a tar.xz package.

ToKiNoBug commented 9 months ago

~Oh, I see the python tests failed. I'm fixing it.~

Python build fixed.

ToKiNoBug commented 9 months ago

Besides, I found that gcc13 can't build kompute because built in fmtlib(v8.1.1) has a possibly dangling reference to a temporary warning, and this is turned to an error by extra -W compiler flags. Either removing extra -W options, or updating fmtlib to v10.0.0 will fix this problem. I don't know which plan is better.

axsaucedo commented 9 months ago

Either removing extra -W options, or updating fmtlib to v10.0.0 will fix this problem. I don't know which plan is better. Do you have more info on this? I also don't have an huge issue with upgrading fmtlib but we woudl also have to test that it works with the rest of the dependencies (ie spdlog), as well as if there are any other issues / changes required to the codebase.

I would be keen to continue with strict -W options as it ensures a clean / maintainable codebase even if often can be a pain intially when introducing

ToKiNoBug commented 9 months ago

I would be keen to continue with strict -W options as it ensures a clean / maintainable codebase even if often can be a pain intially when introducing

We can update the version of builtin fmtlib to v10.1.1.

Althoug we can't control the version of external fmtlib, we can report a warning/error if users are building kompute with gcc13 or later with fmtlib v9 or earlier.

axsaucedo commented 9 months ago

We can update the version of builtin fmtlib to v10.1.1. Yes this seems reasonable.

Althoug we can't control the version of external fmtlib, we can report a warning/error if users are building kompute with gcc13 or later with fmtlib v9 or earlier. Absolutely, if we can do a warning without too much complexity on our side that could be helpful, but agree that we can't control what others bring in - we can make it clear that to minimise warnings requires this version - similarly that we don't run tests with other versions

ToKiNoBug commented 9 months ago

Absolutely, if we can do a warning without too much complexity on our side that could be helpful, but agree that we can't control what others bring in - we can make it clear that to minimise warnings requires this version - similarly that we don't run tests with other versions

I have updated the version of builtin fmt to 10.1.1. But I found it hard to provide a warning about old fmt without complexity. Older fmt doesn't tell its version, may be there's some tricks to get its version, but the code will be too complex.

ToKiNoBug commented 9 months ago

I hope to know if there is anything I should do about this PR. Any problem or requirement?