skeeto / w64devkit

Portable C and C++ Development Kit for x64 (and x86) Windows
The Unlicense
2.66k stars 185 forks source link

[FindPkgConfig] CMake build fails for llama.cpp in windows 11 Intel CPU #114

Closed ratnampa closed 3 months ago

ratnampa commented 4 months ago

I am trying to build llama.cpp with Intel oneMKL on windows 11 Intel CPU, but when building with cmake, I get a parsing error when the code tries to find mkl-sdl. I saw that inside oneAPI base toolkit, mkl-sdl.pc file provides the link to the mkl_rt.lib but the path gets messed up and cmake cannot interpret it. I have added the bin for w64devkit inside PATH variable too.

I have also setup VS 2022 and oneAPI base toolkit.

oneAPI: call "C:\Program Files (x86)\Intel\oneAPI\setvars.bat" intel64 CMD: cmake .. -DLLAMA_BLAS=ON -DLLAMA_BLAS_VENDOR=Intel10_64lp -DCMAKE_C_COMPILER=icx -DCMAKE_CXX_COMPILER=icpx -DLLAMA_NATIVE=ON

error

@skeeto

Peter0x44 commented 4 months ago

Something about that path cmake is complaining about doesn't look right. Why are spaces replaced with ; ?

skeeto commented 4 months ago

Background: w64devkit includes a customized, native-Windows pkg-config implementation called u-config. It's likely oneAPI was never run against u-config before. I installed oneAPI mainly to take a look at mkl-sdl.pc, and with a small workaround I could build a test program. With your case there are a number of issues at play:

  1. mkl-sdl.pc uses unconventional quoting. Its quotes make more sense, but it's at odds with convention, so the result comes out wrong. Typical .pc files do not quote paths like ${prefix} or ${pcfiledir}. Left alone, they wouldn't work when paths contain spaces. On unix, development files are never installed under such paths, so nobody notices the problem. Clearly the oneAPI folks did, so they use quotes. Because paths with spaces are common on Windows (i.e. "Program Files"), and quoting in .pc files is so uncommon, u-config implicitly quotes ${prefix} when it contains spaces so that most .pc files work without changes. However, that breaks the rare .pc files that do quote, including here. I haven't yet found a good heuristic to cover both cases.

  2. The path contains parentheses ("(x86)"): shell meta-characters. The original pkg-config does not escape parentheses, but allows them through as a meta-characters for further interpretation (e.g. to run uname and insert the results, etc.). In other words, literal parentheses in paths is unsupported. On unix this doesn't come up because, again, nobody puts development files under such a path, but Windows is different. In that light, perhaps u-config should behave differently, at least on Windows. I'll have to think about it. (As a shell quirk, I suspect FindPkgConfig deals with this on its own.)

  3. There's a bug in FindPkgConfig, in something that calls it, or in something that consumes its output. Something is inserting semicolons when it shouldn't be. It may be that solving (1) makes this one irrelevant. I'm unsure, but whatever it is, it happens outside w64devkit.

For (1) and (2), you could install oneAPI under a path with neither spaces nor parentheses, or use a subst. Or simpler, use the 8.3 DOS name:

$ export PKG_CONFIG_PATH="c:/progra~2/Intel/oneAPI/mkl/2024.0/lib/pkgconfig"

The setvars.bat script adds the non-8.3 name of this directory, and one other directory. When I queried pkg-config it looked the way I expected:

$ pkg-config --cflags mkl-sdl -Ic:/progra~2/Intel/oneAPI/mkl/2024.0/include

Because the .pc is under lib/pkgconfig, u-config overrides its ${prefix} with the .pc location. This allows libraries to be installed anywhere and moved without changing the .pc file. There's a --dont-define-prefix option to disable this behavior. That's the default elsewhere, and so mkl-sdl.pc emulates it using ${pcfiledir}. Just as with quotes, its authors clearly had problems in other environments and added another unusual workaround to deal with it.

By chance, u-config does not quote ${pcfiledir} as it does when overriding ${prefix}, which is essentially a bug in u-config because that's not what I had intended. However, here it allows mkl-sdl.pc quoting to nearly work out:

$ export PKG_CONFIG_PATH="c:/Program Files (x86)/Intel/oneAPI/mkl/2024.0/lib/pkgconfig" $ pkg-config --cflags --dont-define-prefix mkl-sdl -Ic:/Program\ Files\ (x86)/Intel/oneAPI/mkl/2024.0/include

That almost works, except for those darn parentheses. (Though, again, I suspect it does not affect FindPkgConfig.) Also, you'd need to be able to inject that option into FindPkgConfig's pkg-config invocation, as there's no environment variable to control it. (Perhaps there should be!) So this route probably isn't work pursuing.

Going back to the 8.3 name, I wrote this test program:

include

include "mkl.h"

int main(void) { MKLVersion vers; MKL_Get_Version(&vers); printf("MKL %d.%d\n", vers.MajorVersion, vers.MinorVersion); }

Compiled it (note the eval, which nearly all tutorials / documentation forgets, and is mandatory when paths contain spaces):

$ eval cc test.c $(pkg-config --cflags --libs mkl-sdl)

Then set PATH (as setvars.bat does) and ran it:

$ PATH="$PATH;c:/Program Files (x86)/Intel/oneAPI/mkl/2024.0/bin" $ ./a MKL 2024.0

For (1) I need to find that heuristic that doesn't break other .pc files, though I remain stumped. For (2) I might just drop that old pkg-config "feature" as being too troublesome. I had not yet considered nor come across the "(x86)" problem.

ratnampa commented 4 months ago

Thank you so much for the detailed response, I found a workaround with using conda, and installing pkg-config as conda install pkg-config and that works fine for the MKL build.

skeeto commented 4 months ago

Some great news: After sleeping on the problem, I came up a far better, general solution to solve both (1) and (2) simultaneously. That means oneAPI should Just Work out of the box despite its wacky installation path, and despite its unusual, custom-made workarounds. (That is, w64devkit pkg-config is doing its part, and it's up to CMake and FindPkgConfig to correctly handle it from there.)

Quick summary: Paths are now internally encoded with an unambiguous UTF-8 extension that allows them to pass through string/variable processing without interpretation, and to be given the necessary escaping in the output, while still allowing .pc files to deliberately output shell/make meta characters.

It's probably too late for you to care about it, but in the future w64devkit should handle these situations better.

ratnampa commented 4 months ago

Thank you so much @skeeto for the fix. I just wanted to know by when can I expect a new release of w64devkit?

skeeto commented 4 months ago

Most likely the next w64devkit release will shortly follow the next GCC release (13.3?). There's nothing particularly pressing to do it sooner.

However, you can easily update pkg-config in place if you'd like to try it out. Clone the u-config repository, then "make install" in w64devkit. The install target is tailored for w64devkit and installs over your existing pkg-config. You can also copy this pkg-config.exe back into w64devkit.zip to update it for deployment elsewhere.

ratnampa commented 3 months ago

Thank you for the help!