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

Add OpenCL SDK and clMath ports #2837

Merged
merged 9 commits into from
Mar 1, 2018
Merged

Add OpenCL SDK and clMath ports #2837

merged 9 commits into from
Mar 1, 2018

Conversation

MathiasMagnus
Copy link
Contributor

@MathiasMagnus MathiasMagnus commented Feb 19, 2018

I would like to invite people to help me with constructive criticism and guidance in creating these new, highly inter-related ports. While the OpenCL SDK in itself could stand, I do want to get all of the mentioned in the matching issue as well.

I started the portfile, but there are some errors I don't know how to go about fixing, partly because I donnodawey.

General

I can't quite grasp how it would be optimal to package all of the above SW. The minimal OpenCL SDK consists of the C API headers and the ICD. The prior is included in source files, the latter is linked against. On Linux, these are usually separate packages. On Windows, every SDK ships both as well (AMD APP SDK, Intel OpenCL SDK, Nvidia CUDA SDK), but all the GPU drivers also ship the ICD (which will install under C:\Windows\system32\OpenCL.dll), this is what all applications will pick up if there's no alternative installed beside the exe. The DLLs in the SDKs are only to make the linker happy.

So if you have something to hook a display on to, Windows Update will pull alongside your drivers an OpenCL ICD and install it in a default location. This ICD at best is the very same that is found in the Khronos repo, but is most likely outdated compared to the one we're trying to package here.

OpenCL ICD/SDK

For those that don't know, this library is responsible for dynamically loading the vendor implementations of OpenCL. It is the GLVND of OpenCL, but it's been there since day one. This library canonically is a dynamic library (I've never encountered any indication of it having to be that way, but) nobody expects it to be linked statically. Therefor I would like to forbid it being built as a static lib. This part is written as it should be I guess.

Currently there are post-build validation scripts that argue about mismatching number of debug and release binaries. The reason I don't know how to fix this, is because canonically one never meets debug builds of the OpenCL ICD (aka. OpenCL.dll), as it's usually obtained with drivers. Although in some cases it might be nice to be able to step through the ICD. This would be the case of faulty driver installations when a call to clGetPlatformIDs() results in the undocumented -1000 error code, which corresponds to: something went wrong while loading some vendor implementation.

Few questions:

  1. If I were to package both Debug and Release dynamic libraries, how will consumers find the DLLs when canonically there's no OpenCL-d.dll? (The CMake built-in FindOpenCL.cmake will definitely not look for one. A hand-crafted OpenCL::OpenCL imported target could in theory mask away this discrepancy from the canonical workflow.)
  2. Shall I just simply forbid Debug builds of the ICD? How can I do that?
  3. The C, C++ header and the ICD are found in 3 different repos, but they don't make much sense without each other. Shall I package them together, or should they be 3 different packages and have an opencl meta-package?
  4. The C++ bindings are (unfortunately) not shipped by all SDKs (it is not strictly part of the OpenCL registry), hence FindOpenCL.cmake doesn't even look for it. What do you think should be the best way to fix this "problem"? Shall I patch CMakes FindOpenCL.cmake to also include an OpenCL::CLHPP imported target and try using that here as well?
  5. All 3 repos have their own LICENSE files, which are very similar, but are slightly tailored toward the repos contents. Which shall I install along with the package?

clMath

clMath is the collective name for clFFT, clBLAS, clRNG and clSPARSE. These are definitely big enough to be separate packages, but are meta-packages still a thing? There are traces of a time when Boost was a meta-package that referred to other packages.

Once I gain enough experience with the OpenCL SDK, I'll incorporate the clMath stuff as well into the PR.

@msftclas
Copy link

msftclas commented Feb 19, 2018

CLA assistant check
All CLA requirements met.

@MathiasMagnus
Copy link
Contributor Author

I just wanted to invite @BenjaminCoquelle into the conversation, contributor to most clMath libraries, plus I believe he packaged OCL_SDK_Light.zip for Windows. I think he would be interested in

vcpkg.exe install opencl clfft[fftw3] clblas clsparse

@BenjaminCoquelle
Copy link

The ICD (OpenCL.dll) should only come with the driver.
Don't ship the ICD, just the headers and the lib to link against

@MathiasMagnus
Copy link
Contributor Author

Heeding Benjamins advice, I modified the script to not deploy the DLLs, only the LIBs.

There is a post-build check that's failing, but I don't understand it.

-- Using cached C:/Users/mnagy/Source/Repos/vcpkg/downloads/KhronosGroup-OpenCL-ICD-Loader-26a38983cbe5824fd5be03eab8d037758fc44360.tar.gz
-- Testing integrity of cached file...
-- Testing integrity of cached file... OK
-- Extracting done
-- Building the ICD loader as a static library is not supported. Building as DLLs instead.
-- Configuring x64-windows-static-rel
-- Configuring x64-windows-static-rel done
-- Configuring x64-windows-static-dbg
-- Configuring x64-windows-static-dbg done
-- Build x64-windows-static-rel
-- Build x64-windows-static-rel done
-- Build x64-windows-static-dbg
-- Build x64-windows-static-dbg done
-- Installing: C:/Users/mnagy/Source/Repos/vcpkg/packages/opencl_x64-windows-static/lib/Release/OpenCL.lib
-- Installing: C:/Users/mnagy/Source/Repos/vcpkg/packages/opencl_x64-windows-static/lib/Release/OpenCL.exp
-- Installing: C:/Users/mnagy/Source/Repos/vcpkg/packages/opencl_x64-windows-static/lib/Debug/OpenCL.lib
-- Installing: C:/Users/mnagy/Source/Repos/vcpkg/packages/opencl_x64-windows-static/lib/Debug/OpenCL.exp
-- Installing: C:/Users/mnagy/Source/Repos/vcpkg/packages/opencl_x64-windows-static/share/opencl/copyright/LICENSE.txt
-- Performing post-build validation
Mismatching number of debug and release binaries. Found 0 for debug but 2 for release.
Debug binaries


Release binaries

    C:/Users/mnagy/Source/Repos/vcpkg/packages/opencl_x64-windows-static/lib/Release/OpenCL.lib
    C:/Users/mnagy/Source/Repos/vcpkg/packages/opencl_x64-windows-static/lib/Debug/OpenCL.lib

Debug binaries were not found

Found 1 error(s). Please correct the portfile:
    C:\Users\mnagy\Source\Repos\vcpkg\ports\opencl\portfile.cmake

Looking inside the build logs, the library was indeed built using separate flags, but the .lib files are identical to the last bit. ???

Release

[2/6] C:\Kellekek\MICROS1\VISUAL1\Preview\VC\Tools\MSVC\1413~1.261\bin\Hostx64\x64\cl.exe -DOpenCL_EXPORTS -I\Include -IC:\Users\mnagy\Source\Repos\vcpkg\packages\opencl_x64-windows-static\include /DWIN32 /D_WINDOWS /W3 /utf-8 /MP /MT /O2 /Oi /Gy /DNDEBUG /Z7 /showIncludes /FoCMakeFiles\OpenCL.dir\icd_windows.c.obj /FdCMakeFiles\OpenCL.dir\ /FS -c C:\Users\mnagy\Source\Repos\vcpkg\buildtrees\opencl\src\OpenCL-ICD-Loader-26a38983cbe5824fd5be03eab8d037758fc44360\icd_windows.c

Debug

[2/6] C:\Kellekek\MICROS1\VISUAL1\Preview\VC\Tools\MSVC\1413~1.261\bin\Hostx64\x64\cl.exe -DOpenCL_EXPORTS -I\Include -IC:\Users\mnagy\Source\Repos\vcpkg\packages\opencl_x64-windows-static\include /DWIN32 /D_WINDOWS /W3 /utf-8 /MP /D_DEBUG /MTd /Z7 /Ob0 /Od /RTC1 /showIncludes /FoCMakeFiles\OpenCL.dir\icd_windows.c.obj /FdCMakeFiles\OpenCL.dir\ /FS -c C:\Users\mnagy\Source\Repos\vcpkg\buildtrees\opencl\src\OpenCL-ICD-Loader-26a38983cbe5824fd5be03eab8d037758fc44360\icd_windows.c

Because the OpenCL.dll is a system DLL anyway and there is no reason to build a Debug version of it, is there a way to disable this check and omit debug builds all together and only deploy release builds ?

@MathiasMagnus
Copy link
Contributor Author

@ras0219-msft Could you let me know how to omit Debug builds if I want to conform to having only the drivers shipping a Release DLL? The docs did not help, neither searching the previous issues.

@ras0219-msft
Copy link
Contributor

Thanks for making this PR!

First, the error message is actually catching a misplacement of lib files:

C:/Users/mnagy/Source/Repos/vcpkg/packages/opencl_x64-windows-static/lib/Release/OpenCL.lib
C:/Users/mnagy/Source/Repos/vcpkg/packages/opencl_x64-windows-static/lib/Debug/OpenCL.lib

These should be in packages/<package>/lib/OpenCL.lib and packages/<package>/debug/lib/OpenCL.lib. Since they're currently both under the /lib prefix, neither will be available in debug mode!

We don't currently have an easy way to reliably opt-out from building Debug, but could you try the above changes first and see if that works?

@MathiasMagnus
Copy link
Contributor Author

Thanks for the input. It does work indeed. I already tested the SDK and it seems to work now. I proceeded to tweaking clFFT and it builds fine, even installs and passes post-build tests, but consumers cannot detect it properly.

First, the Package Config files are not found. It argues about clFFTConfig.cmake not being found and that I should set clFFT_DIR to the folder containing it. Even if I set this variable at configuration time to <VCPKG_ROOT>\installed\x64-windows-static\share\clfft\ where the file indeed does exist, it won't find it.

If I register the installed package into my user package registry, it is found alright, but the headers are not found when building. "clFFT.h" not found I started digging around and the installed clFFTConfig.cmake contains these lines:

include(${CMAKE_CURRENT_LIST_DIR}/clFFTTargets.cmake)
get_filename_component(CLFFT_INCLUDE_DIRS ${CMAKE_CURRENT_LIST_DIR}/../include ABSOLUTE)
set(CLFFT_LIBRARIES clFFT)

However, ${CMAKE_CURRENT_LIST_DIR}/../include does not exist. The package layout expected by Vcpkg seems to differ from that in which the package was built. One up from the package config file is still inside share.

How should I go about fixing a generated package config file that is generated in a manner not to be relocated, moreover have it's layout changed? Or am I doing something totally wrong?

Should you want to give it a try, clone my working tree and try building a simple sample found here.

@MathiasMagnus
Copy link
Contributor Author

@ras0219-msft Thanks for the help thus far. Getting clFFT to build was tougher than I had anticipated (infact, this entire process is a lot harder than I had anticipated). Once, when everything was building, It failed to link the sample clFFT_test application, because of MT/MD mismatch. (By default, CMake uses MD, and the built-in x64-windows-static also links the CRT statically. Because I did not want to set MT in ALL my downstream projects, I went ahead and started using x64-windows instead.)

Now my problem is (for some reason), is that OpenCL detection via the built-in FindOpenCL.cmake is half baked. This is the config output when building the clFFT runtime library:

-- The C compiler identification is MSVC 19.13.26126.2
-- The CXX compiler identification is MSVC 19.13.26126.2
-- Check for working C compiler: C:/Kellekek/Microsoft/Visual Studio/Preview/VC/Tools/MSVC/14.13.26126/bin/Hostx86/x64/cl.exe
-- Check for working C compiler: C:/Kellekek/Microsoft/Visual Studio/Preview/VC/Tools/MSVC/14.13.26126/bin/Hostx86/x64/cl.exe -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: C:/Kellekek/Microsoft/Visual Studio/Preview/VC/Tools/MSVC/14.13.26126/bin/Hostx86/x64/cl.exe
-- Check for working CXX compiler: C:/Kellekek/Microsoft/Visual Studio/Preview/VC/Tools/MSVC/14.13.26126/bin/Hostx86/x64/cl.exe -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- UNICODE build
-- 64bit build - FIND_LIBRARY_USE_LIB64_PATHS TRUE
-- Could NOT find Boost
-- Found OPENCL: C:/Users/mnagy/Source/Repos/vcpkg/installed/x64-windows/lib/OpenCL.lib  
-- Could NOT find FFTW (missing: FFTW_LIBRARIES FFTW_INCLUDE_DIRS) 
-- FindFFTW looked for single precision libraries named: fftw3f or libfftw3f-3
-- FindFFTW looked for double precision libraries named: fftw3 or libfftw3-3
-- Detected MSVS Ver: 1913
-- CMAKE_CXX_COMPILER flags:  /DWIN32 /D_WINDOWS /W3 /utf-8 /GR /EHsc /MP
-- CMAKE_CXX_COMPILER debug flags: /MDd /Zi /Ob0 /Od /RTC1
-- CMAKE_CXX_COMPILER release flags: /MD /O2 /Oi /Gy /DNDEBUG /Z7
-- CMAKE_CXX_COMPILER relwithdebinfo flags: /MD /Zi /O2 /Ob1 /DNDEBUG
-- CMAKE_EXE_LINKER link flags: /machine:x64
-- OpenCL_INCLUDE_DIRS: 
-- Configuring done
-- Generating done
-- Build files have been written to: C:/Users/mnagy/Source/Repos/vcpkg/buildtrees/clfft/x64-windows-rel

Notice that find_package(OpenCL) finds the OpenCL built with Vcpkg, but later on, the manually inserted

message(STATUS "OpenCL_INCLUDE_DIRS: ${OpenCL_INCLUDE_DIRS}")

returns -- OpenCL_INCLUDE_DIRS:
Honestly, I have no idea how it finds the library, when I still have AMD APP SDK installed (which I want to get rid of in the long run), so I have an environmental variable AMDAPPSDKROOT, which should pretty much sidetrack the detection logic in FindOpenCL.cmake

find_library(OpenCL_LIBRARY
      NAMES OpenCL
      PATHS
        ENV "PROGRAMFILES(X86)"
        ENV AMDAPPSDKROOT
        ENV INTELOCLSDKROOT
        ENV CUDA_PATH
        ENV NVSDKCOMPUTE_ROOT
        ENV ATISTREAMSDKROOT
      PATH_SUFFIXES
        "AMD APP/lib/x86_64"
        lib/x86_64
        lib/x64
        OpenCL/common/lib/x64)

Is there a way to manipulate the tool-chain file to force defining the key variables, OpenCL_LIBRARY and OpenCL_INCLUDE_DIR to guide the built-in detection script? This process is becoming a nightmare!!

@ras0219-msft
Copy link
Contributor

Sorry it's so tough!

Is there a way to manipulate the tool-chain file to force defining the key variables, OpenCL_LIBRARY and OpenCL_INCLUDE_DIR to guide the built-in detection script?

We definitely do[1] have that capability, and already use it for other libraries. Note, however, that environment variables will not be passed through to the actual build (to ensure it's reproducible on other machines). You can see what the build environment looks like by running vcpkg env --triplet <triplet>.

How should I go about fixing a generated package config file that is generated in a manner not to be relocated, moreover have it's layout changed?

Yep, this is a very common issue! We have a helper which does all the common fixups needed called vcpkg_fixup_cmake_targets()[2]. It doesn't (yet) have a nice docs page like the other commands, but you can use git grep vcpkg_fixup_cmake_targets in the vcpkg repository to find tons of usage examples!

[1] https://github.com/Microsoft/vcpkg/blob/4642191a099ddd578a46ecbb2c08899233e9e754/scripts/buildsystems/vcpkg.cmake#L185-L253
[2] https://github.com/Microsoft/vcpkg/blob/4642191a099ddd578a46ecbb2c08899233e9e754/scripts/cmake/vcpkg_fixup_cmake_targets.cmake

@MathiasMagnus
Copy link
Contributor Author

MathiasMagnus commented Feb 28, 2018

Apologies for being such a slow learner, but there must be a fundamental misunderstanding between me and Vcpkg. How come that if a package installs inside port.cmake (and as such, sees things that are in vcpkg.cmake and all dependent find_package invocations work), the same is not true when I try to consume these built libraries?

Here's my problem: find_package(OpenCL) works as long as CMAKE_INCLUDE_PATH is modified. I append something to it. I don't even need to hijack the find_package() invocation itself. However, when I'm using Vcpkg on a totally different project using -D CMAKE_TOOLCHAIN_FILE=.../vcpkg.cmake then these modifications are not visible, hence find_package(OpenCL) does not work and won't find the very same stuff that clFFT found, when I was building the package.

What am I doing wrong?

(After all these noobish questions, you'll might just have another package maintainer producing all sorts of HPC goodies.)

@ras0219-msft
Copy link
Contributor

ras0219-msft commented Mar 1, 2018

So it turns out that clfft was secretly to blame :) They included their own FindOpenCL.cmake[1] file which was shadowing the built-in copy. Their version sets OPENCL_XYZ instead of OpenCL_XYZ variables, which is why it was blank! In the master branch they've since removed it, so it's extra sneaky :)

Once I reverted the variable names back to the uppercase forms, everything just worked!

I also converted your file-based replacements into a patch file -- it's problematic for us to check in entire source files, plus they're much larger!

You'll need to delete your buildtrees\clfft directory to make sure the new patches apply cleanly.

Since everything appears working to me, I've gone ahead and merged it. If you encounter any more issues, please let us know and/or make a PR! Thanks so much for this effort, OpenCL is really popular so it's great to have it in Vcpkg!

[1] https://github.com/clMathLibraries/clFFT/blob/v2.12.2/src/FindOpenCL.cmake

@ras0219-msft ras0219-msft merged commit b21c895 into microsoft:master Mar 1, 2018
@MathiasMagnus
Copy link
Contributor Author

There were multiple things at fault.

Because I started running low on time with the project I started this development for, I started putting my environment separate from Vcpkg. I noticed just before leaving work (you beat me to reporting it), that I couldn't guide find_package(OpenCL) with the mixed case variables, because of the outdated (pre-CMake 3.0?) style scripts.

Second, this only became apparent when I did a build outside VS Code, which I primarily use for development. The compiler detection feature (Kits)[1] of CMake Tools and the toolchain file was disregarded. I found a workaround since, but it still requites a bit of tweaking.

Based on your work, I'll add the very similar clBLAS, clRNG and hopefully the clSPARSE libs too.

[1]
https://vector-of-bool.github.io/docs/vscode-cmake-tools/kits.html

@cenit
Copy link
Contributor

cenit commented Mar 1, 2018

If only vcpkg was available few years ago (together with the great work on the MSVC compiler)!! Windows would have not been almost abandoned in academic usage (at least here it is so)! Thanks @MathiasMagnus for this work, please bring it on with also the other cl libs!

@MathiasMagnus MathiasMagnus deleted the clMath branch March 2, 2018 08:55
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.

5 participants