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

llama.cpp fails to build with g++ 13 #2802

Closed
paravoid opened this issue Sep 12, 2023 · 8 comments
Closed

llama.cpp fails to build with g++ 13 #2802

paravoid opened this issue Sep 12, 2023 · 8 comments
Labels
bug Something isn't working c-Plugin An issue related to WasmEdge Plugin dependencies Pull requests that update a dependency file

Comments

@paravoid
Copy link
Contributor

Building WasmEdge 0.13.4 on Debian unstable with gcc/g++ 13, results into this build failure:

Build log ``` cd <>/wasmedge/obj-x86_64-linux-gnu/lib/validator && /usr/lib/ccache/c++ -DFMT_SHARED -DSPDLOG_COMPILED_LIB -DSPDLOG_FMT_EXTERNAL -DSPDLOG_SHARED_LIB -I<>/wasmedge/obj-x86_64-linux-gnu/include -I<>/wasmedge/include -g -O2 -ffile-prefix-map=<>/wasmedge=. -fstack-protector-strong -fstack-clash-protection -Wformat -Werror=format-security -fcf-protection -Wdate-time -D_FORTIFY_SOURCE=2 -std=c++17 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Wall -Wextra -Werror -Wno-error=pedantic -Wno-psabi -MD -MT lib/validator/CMakeFiles/wasmedgeValidator.dir/validator.cpp.o -MF CMakeFiles/wasmedgeValidator.dir/validator.cpp.o.d -o CMakeFiles/wasmedgeValidator.dir/validator.cpp.o -c <>/wasmedge/lib/validator/validator.cpp In file included from /usr/include/c++/13/string:51, from <>/wasmedge/thirdparty/ggml/llama-util.h:15, from <>/wasmedge/thirdparty/ggml/llama.cpp:9: In static member function ‘static void std::__copy_move::__assign_one(_Tp*, _Up*) [with _Tp = const llama_grammar_element*; _Up = const llama_grammar_element* const]’, inlined from ‘static _Up* std::__copy_move<_IsMove, true, std::random_access_iterator_tag>::__copy_m(_Tp*, _Tp*, _Up*) [with _Tp = const llama_grammar_element* const; _Up = const llama_grammar_element*; bool _IsMove = false]’ at /usr/include/c++/13/bits/stl_algobase.h:440:20, inlined from ‘_OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove = false; _II = const llama_grammar_element* const*; _OI = const llama_grammar_element**]’ at /usr/include/c++/13/bits/stl_algobase.h:506:30, inlined from ‘_OI std::__copy_move_a1(_II, _II, _OI) [with bool _IsMove = false; _II = const llama_grammar_element* const*; _OI = const llama_grammar_element**]’ at /usr/include/c++/13/bits/stl_algobase.h:533:42, inlined from ‘_OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove = false; _II = __gnu_cxx::__normal_iterator >; _OI = const llama_grammar_element**]’ at /usr/include/c++/13/bits/stl_algobase.h:540:31, inlined from ‘_OI std::copy(_II, _II, _OI) [with _II = __gnu_cxx::__normal_iterator >; _OI = const llama_grammar_element**]’ at /usr/include/c++/13/bits/stl_algobase.h:633:7, inlined from ‘static _ForwardIterator std::__uninitialized_copy::__uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = __gnu_cxx::__normal_iterator >; _ForwardIterator = const llama_grammar_element**]’ at /usr/include/c++/13/bits/stl_uninitialized.h:147:27, inlined from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = __gnu_cxx::__normal_iterator >; _ForwardIterator = const llama_grammar_element**]’ at /usr/include/c++/13/bits/stl_uninitialized.h:185:15, inlined from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, allocator<_Tp>&) [with _InputIterator = __gnu_cxx::__normal_iterator >; _ForwardIterator = const llama_grammar_element**; _Tp = const llama_grammar_element*]’ at /usr/include/c++/13/bits/stl_uninitialized.h:373:37, inlined from ‘std::vector<_Tp, _Alloc>::vector(const std::vector<_Tp, _Alloc>&) [with _Tp = const llama_grammar_element*; _Alloc = std::allocator]’ at /usr/include/c++/13/bits/stl_vector.h:603:31, inlined from ‘void std::__new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = std::vector; _Args = {const std::vector >&}; _Tp = std::vector]’ at /usr/include/c++/13/bits/new_allocator.h:187:4, inlined from ‘static void std::allocator_traits >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = std::vector; _Args = {const std::vector >&}; _Tp = std::vector]’ at /usr/include/c++/13/bits/alloc_traits.h:537:17, inlined from ‘void std::vector<_Tp, _Alloc>::push_back(const value_type&) [with _Tp = std::vector; _Alloc = std::allocator >]’ at /usr/include/c++/13/bits/stl_vector.h:1283:30, inlined from ‘void llama_grammar_advance_stack(const std::vector >&, const std::vector&, std::vector >&)’ at <>/wasmedge/thirdparty/ggml/llama.cpp:2175:29: /usr/include/c++/13/bits/stl_algobase.h:398:17: error: array subscript 0 is outside array bounds of ‘const llama_grammar_element* [0]’ [-Werror=array-bounds=] 398 | { *__to = *__from; } | ~~~~~~^~~~~~~~~ In file included from /usr/include/x86_64-linux-gnu/c++/13/bits/c++allocator.h:33, from /usr/include/c++/13/bits/allocator.h:46, from /usr/include/c++/13/string:43: In member function ‘_Tp* std::__new_allocator<_Tp>::allocate(size_type, const void*) [with _Tp = const llama_grammar_element*]’, inlined from ‘static _Tp* std::allocator_traits >::allocate(allocator_type&, size_type) [with _Tp = const llama_grammar_element*]’ at /usr/include/c++/13/bits/alloc_traits.h:482:28, inlined from ‘std::_Vector_base<_Tp, _Alloc>::pointer std::_Vector_base<_Tp, _Alloc>::_M_allocate(std::size_t) [with _Tp = const llama_grammar_element*; _Alloc = std::allocator]’ at /usr/include/c++/13/bits/stl_vector.h:378:33, inlined from ‘std::_Vector_base<_Tp, _Alloc>::pointer std::_Vector_base<_Tp, _Alloc>::_M_allocate(std::size_t) [with _Tp = const llama_grammar_element*; _Alloc = std::allocator]’ at /usr/include/c++/13/bits/stl_vector.h:375:7, inlined from ‘void std::_Vector_base<_Tp, _Alloc>::_M_create_storage(std::size_t) [with _Tp = const llama_grammar_element*; _Alloc = std::allocator]’ at /usr/include/c++/13/bits/stl_vector.h:395:44, inlined from ‘std::_Vector_base<_Tp, _Alloc>::_Vector_base(std::size_t, const allocator_type&) [with _Tp = const llama_grammar_element*; _Alloc = std::allocator]’ at /usr/include/c++/13/bits/stl_vector.h:332:26, inlined from ‘std::vector<_Tp, _Alloc>::vector(const std::vector<_Tp, _Alloc>&) [with _Tp = const llama_grammar_element*; _Alloc = std::allocator]’ at /usr/include/c++/13/bits/stl_vector.h:600:61, inlined from ‘void std::__new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = std::vector; _Args = {const std::vector >&}; _Tp = std::vector]’ at /usr/include/c++/13/bits/new_allocator.h:187:4, inlined from ‘static void std::allocator_traits >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = std::vector; _Args = {const std::vector >&}; _Tp = std::vector]’ at /usr/include/c++/13/bits/alloc_traits.h:537:17, inlined from ‘void std::vector<_Tp, _Alloc>::push_back(const value_type&) [with _Tp = std::vector; _Alloc = std::allocator >]’ at /usr/include/c++/13/bits/stl_vector.h:1283:30, inlined from ‘void llama_grammar_advance_stack(const std::vector >&, const std::vector&, std::vector >&)’ at <>/wasmedge/thirdparty/ggml/llama.cpp:2175:29: /usr/include/c++/13/bits/new_allocator.h:147:55: note: object of size 0 allocated by ‘operator new’ 147 | return static_cast<_Tp*>(_GLIBCXX_OPERATOR_NEW(__n * sizeof(_Tp))); | ^ cc1plus: all warnings being treated as errors ```

This seems to have been fixed in upstream llama-cpp, with ggerganov/llama.cpp@ef15649 and indeed backporting the push_back -> emplace_back llama.cpp changes is enough to make the build work.

I'm not sure if you have a mechanism to update llama.cpp to newer versions, or to backport upstream fixes. I assume sending PRs against llama.cpp would just make the source diverge and cause more issues down the line.

  • Hardware Architecture: x86_64
  • Operating system: Debian unstable
  • C++ Compiler version: gcc version 13.2.0 (Debian 13.2.0-3)
  • CMake version: cmake version 3.27.4
  • CMake flags: -DCMAKE_BUILD_TYPE=Release -DWASMEDGE_BUILD_PLUGINS=OFF

Separately,

  1. It's unclear to me why ggml/llama needs to be built if plugins are turned off? thirdparty/CMakeLists.txt seems to unconditionally add_subdirectory(ggml), so perhaps that needs to be turned into a conditional?
  2. Given both 0.13.3 & 0.13.4 were failing to build with g++-13 with separate (legitimate) errors, perhaps CI needs to be adjusted to also try builds with a newer compiler version? Ubuntu mantic 23.10 will ship with 13 as the default.
@hydai
Copy link
Member

hydai commented Sep 12, 2023

Hi @paravoid

  1. The ggml turned into a conditional now. Not in the 0.13.4 release. For a workaround, here is a patch to fix it: 5308cc1
  2. Agree. I will set up a new workflow for the new compilers.

@hydai
Copy link
Member

hydai commented Sep 12, 2023

@dm4
Please update the llama.cpp to a new version.

@hydai hydai added bug Something isn't working c-Plugin An issue related to WasmEdge Plugin dependencies Pull requests that update a dependency file labels Sep 12, 2023
@hydai
Copy link
Member

hydai commented Sep 12, 2023

/cc @q82419
Would you be willing to have a new release? Release 0.13.5, including the fix for making llama.cpp an optional dependency and several compatible fixes for MSVC.

/cc @ibmibmibm
I also found that the 0.13.4 will fail on fedora:rawhide due to the API breaking changes of the LLVM-C interfaces. (LLVM17-rc)

@hydai
Copy link
Member

hydai commented Sep 12, 2023

Hi @paravoid
What's your recommendation for the base image? Debian unstable or Ubuntu 23.10?

We are going to have fedora:rawhide soon, which will help us to reproduce the future environment of testing before releasing the packages.

@paravoid
Copy link
Contributor Author

Hi @paravoid What's your recommendation for the base image? Debian unstable or Ubuntu 23.10?

Debian unstable is a moving/rolling target, so it will always test with the latest and greatest, but could be broken from time to time. Debian testing is also rolling, but (even) more stable. It basically lags behind 3-5 days.

Ubuntu 23.10 will have a "frozen" set of packages. It will include gcc 13, but won't get updated to 14 or e.g. LLVM 17, when the time comes.

Personally, I'd go for Debian testing, and reevaluate later if it ends up being too tedious.

@hydai
Copy link
Member

hydai commented Sep 13, 2023

Personally, I'd go for Debian testing, and reevaluate later if it ends up being too tedious.

Thanks for the information. I will go with debian:testing first.

@hydai
Copy link
Member

hydai commented Sep 14, 2023

Hi @paravoid
FYI.

Here is the patch for avoiding the ggml dependency based on the 0.13.4 release I used on Fedora:rawhide:

From 2856e5ea22b396cdd7cdef450e968e913d7e1752 Mon Sep 17 00:00:00 2001
From: hydai <hydai@secondstate.io>
Date: Thu, 14 Sep 2023 10:16:55 +0800
Subject: [PATCH] [Misc] Only build thirdparty/ggml when the WASI-NN ggml
 plugin is enabled

Signed-off-by: hydai <hydai@secondstate.io>
---
 thirdparty/CMakeLists.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt
index 57d481f2..1f2a932b 100644
--- a/thirdparty/CMakeLists.txt
+++ b/thirdparty/CMakeLists.txt
@@ -5,4 +5,9 @@ if(WASMEDGE_BUILD_AOT_RUNTIME)
   add_subdirectory(blake3)
 endif()

-add_subdirectory(ggml)
+if(WASMEDGE_PLUGIN_WASI_NN_BACKEND)
+  string(TOLOWER ${WASMEDGE_PLUGIN_WASI_NN_BACKEND} BACKEND)
+  if(BACKEND STREQUAL "ggml")
+    add_subdirectory(ggml)
+  endif()
+endif()
--
2.34.1

@hydai
Copy link
Member

hydai commented Jun 7, 2024

Hi @paravoid
After merging #3457, we will use the debian:testing to verify the building process and unit tests. I hope that we will no longer hit this kind of issue anymore. Thanks for your suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working c-Plugin An issue related to WasmEdge Plugin dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

2 participants