Issue with building backend on MSVC: failure to link llamamodels, gptjmodels,

jacoobes commented 1 year ago

System Info

Windows 10 Processor AMD Ryzen 3 3100 4-Core Processor 3.60 GHz Installed RAM 16.0 GB System type 64-bit operating system, x64-based processor Pen and touch No pen or touch input is available for this display Visual Studio 17 2022 MSVC v143 x64/x86 Windows 10 SDK 10.0.20348.0 CMake 3.26.4

Information

[ ] The official example notebooks/scripts
[X] My own modified scripts

Related Components

[X] backend
[X] bindings
[ ] python-bindings
[ ] chat-ui
[ ] models
[ ] circleci
[ ] docker
[ ] api

Reproduction

Building the backend with this script , (ensure you're in the right directory)

Remove-Item -Force -Recurse .\runtimes\win-x64\msvc -ErrorAction SilentlyContinue
mkdir .\runtimes\win-x64\msvc\build | Out-Null
cmake -G "Visual Studio 17 2022" -A X64 -S ..\..\gpt4all-backend -B .\runtimes\win-x64\msvc\build
cmake --build .\runtimes\win-x64\msvc\build  --parallel --config RelWithDebInfo
# cp .\runtimes\win-x64\msvc\build\bin\Release\*.dll .\runtimes\win-x64

You may also use visual studio 2022 IDE:

I generated the cmake build and then opened the solution from there.

expect the error Error MSB3073 The command "setlocal cd (path) if %errorlevel% neq 0 goto :cmEnd C: if %errorlevel% neq 0 goto :cmEnd "C:\Program Files\CMake\bin\cmake.exe" -E create_def (path)/runtimes/win-x64/mpt-avxonly.dir/RelWithDebInfo/exports.def (path)/runtimes/win-x64/mpt-avxonly.dir/RelWithDebInfo//objects.txt if %errorlevel% neq 0 goto :cmEnd :cmEnd endlocal & call :cmErrorLevel %errorlevel% & goto :cmDone :cmErrorLevel exit /b %1 :cmDone if %errorlevel% neq 0 goto :VCEnd :VCEnd" exited with code -1073741819. mpt-avxonly C:\Program Files\Microsoft Visual Studio\2022\Community\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets

Error code is -1073741819, which is a filesystem error.

I investigated and found out I can successfully compile the rest of the application by editing the command line arguments generated by CMake:

GGML_BUILD_VARIANT
CMAKE_INDIR

When building with cmake, these defines specified on the command line cause the MSVC compiler to believe it is a file or directory, and no such file or directory exists (hence the filesystem error) However, I attempted to fix the generation of cmake defines, to no avail.

Expected behavior

The DLLs to compile successfully on windows. Running dumpbin /EXPORTS (dll file) should properly export the symbols is_g4a_implementation.... and friends

jacoobes commented 1 year ago

should be fixed. @drasticactions used this script which worked for me. It bruteforced and retried until it worked (wtf?) https://github.com/drasticactions/gpt4all.net.Runtime/commit/1ba1022dcf135a3b2d25105b71d6523a02723736

manyoso commented 1 year ago

Yeah, I have same experience. Lookup ninja subcommand failed on google and you can see many projects apparently experience this and it is inexplicable. Typical windows quality

mvenditto commented 1 year ago

Take what I report with a grain of salt because I'm not very familiar with CMake and stuff.

After following a chain of issues spread among many repos, what I've found is that the combination of the following does not work well (if it works at all):

MSVC
INTERPROCEDURAL_OPTIMIZATION (WholeProgramOptimization, /GL flag in MSVC)
BUILD_SHARED_LIBS
WINDOWS_EXPORT_ALL_SYMBOLS (cmake produces export.defs)

This comment gives an in-depth explanation of the possible root cause.

As far as I understand, if it is not feasible to manually define the exports (aka using WINDOWS_EXPORT_ALL_SYMBOLS is necessary), the only alternative to brute-forcing the build is to disable IPO while building with MSVC. I imagine this is not an option because it will produce a suboptimal build.

With IPO enabled the first run seems to produce empty exports.def for the failing libraries, not sure why the second time works but the logs may give additional information.

Quick and dirty hack to try that disabling IPO makes the build succeed

Adding the following snippet [at the end of the](https://github.com/nomic-ai/gpt4all/blob/1d4c8e7091b02d40318bfbda7ebd4868569c2cba/gpt4all-backend/CMakeLists.txt#L74) `prepare_target` function in CMakeLists.txt for example, makes the build successfully at the first attempt: ```cmake if(WIN32 AND MSVC AND CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS AND IPO_SUPPORTED) message(STATUS "Workaround MSVC: Interprocedural optimization disabled for '${TARGET_NAME}'") set_property(TARGET ${TARGET_NAME} PROPERTY INTERPROCEDURAL_OPTIMIZATION OFF) target_link_options(${TARGET_NAME} PRIVATE /LTCG) endif() ```

TL;DR The combination of WINDOWS_EXPORT_ALL_SYMBOLS and INTERPROCEDURAL_OPTIMIZATION seems to have issues with MSVC, see the issue below for more detail.

For reference:

https://github.com/luxonis/depthai-core/issues/334
"cmake.exe -E __create_def" is crashing when adding /GL flag to CMAKE_CXX_FLAGS, for MSVC 2015 (this comment cites the "running multiple times succeeds" thing)
https://gitlab.kitware.com/cmake/cmake/-/issues/20484
https://gitlab.kitware.com/cmake/cmake/-/issues/23035
https://learn.microsoft.com/en-us/cpp/build/reference/gl-whole-program-optimization?view=msvc-170

cosmic-snow commented 1 year ago

Error MSB3073 The command "setlocal
cd (path)
if %errorlevel% neq 0 goto :cmEnd
C:
if %errorlevel% neq 0 goto :cmEnd
"C:\Program Files\CMake\bin\cmake.exe" -E create_def (path)/runtimes/win-x64/mpt-avxonly.dir/RelWithDebInfo/exports.def (path)/runtimes/win-x64/mpt-avxonly.dir/RelWithDebInfo//objects.txt
if %errorlevel% neq 0 goto :cmEnd
:cmEnd
endlocal & call :cmErrorLevel %errorlevel% & goto :cmDone
:cmErrorLevel
exit /b %1
:cmDone
if %errorlevel% neq 0 goto :VCEnd
:VCEnd" exited with code -1073741819.

Yes, this looks familiar.

The first option is to try to build it twice. But I've "fixed" this for me by disabling 'whole program optimisation' (/GL). See edit2 & edit4 here: https://github.com/nomic-ai/gpt4all/pull/746#issuecomment-1573546380

I didn't find all the extra info there, @mvenditto. Interesting, thanks for sharing.

manyoso commented 1 year ago

I just pushed a fix by disabling IPO until someone figures out how to enable it.

bartanderson commented 1 year ago

I don't think that is going to happen since MSVC can't link if it can't see the symbols. There was a good writeup in previous from mvenditto. I read the links and looked at the MS page for /GL. Can this be reviewed and closed? I would try building if I knew how to pull it.

cosmic-snow commented 1 year ago

I don't think that is going to happen since MSVC can't link if it can't see the symbols. There was a good writeup in previous from mvenditto. I read the links and looked at the MS page for /GL. Can this be reviewed and closed? I would try building if I knew how to pull it.

What do you mean? /GL is disabled right now to work around this problem, so it's building fine.

bartanderson commented 1 year ago

Yes, I am suggesting this move forward

On Sun, Jun 18, 2023, 1:55 AM cosmic-snow @.***> wrote:

I don't think that is going to happen since MSVC can't link if it can't see the symbols. There was a good writeup in previous from mvenditto. I read the links and looked at the MS page for /GL. Can this be reviewed and closed? I would try building if I knew how to pull it.

What do you mean? /GL is disabled right now to work around this problem, so it's building fine.

— Reply to this email directly, view it on GitHub https://github.com/nomic-ai/gpt4all/issues/841#issuecomment-1595996275, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMXAKDZMV5LETH633HCUSDXL2Q4XANCNFSM6AAAAAAY2NYKCY . You are receiving this because you commented.Message ID: @.***>

cosmic-snow commented 1 year ago

There are still some things that can be done:

Check what the performance impact actually is between having it on and dealing with build errors, or turning it off completely.
There's a related flag, I forgot its name. Maybe that could be used instead?
Maybe it could be enabled partially, in one or a few of the subprojects.

Also, I guess it's not a priority to close this issue.

bartanderson commented 1 year ago

I thought it was holding up gpu

On Sun, Jun 18, 2023, 8:33 AM cosmic-snow @.***> wrote:

There are still some things that can be done:

Check what the performance impact actually is between having it on and dealing with build errors, or turning it off completely.

There's a related flag, I forgot its name. Maybe that could be used instead?

Maybe it could be enabled partially, in one or a few of the subprojects.

Also, I guess it's not a priority to close this issue.

— Reply to this email directly, view it on GitHub https://github.com/nomic-ai/gpt4all/issues/841#issuecomment-1596149652, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMXAKFAVS32MKVATGOK4LDXL37T5ANCNFSM6AAAAAAY2NYKCY . You are receiving this because you commented.Message ID: @.***>

cosmic-snow commented 1 year ago

No that's not it. It's an optimisation that when turned on is causing build problems. So the workaround was to turn it off until maybe someone can look into it a bit more.

bartanderson commented 1 year ago

That's the part I was saying wasn't likely to happen based on the links provided previously.

On Sun, Jun 18, 2023, 9:23 AM cosmic-snow @.***> wrote:

No that's not it. It's an optimisation that when turned on is causing build problems. So the workaround was to turn it off until maybe someone can look into it a bit more.

— Reply to this email directly, view it on GitHub https://github.com/nomic-ai/gpt4all/issues/841#issuecomment-1596166712, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMXAKGN76JSQUNFF3D7MR3XL4FMLANCNFSM6AAAAAAY2NYKCY . You are receiving this because you commented.Message ID: @.***>

nomic-ai / gpt4all