WasmEdge / WasmEdge

WasmEdge is a lightweight, high-performance, and extensible WebAssembly runtime for cloud native, edge, and decentralized applications. It powers serverless apps, embedded functions, microservices, smart contracts, and IoT devices.
https://WasmEdge.org
Apache License 2.0
8.62k stars 770 forks source link

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

Closed paravoid closed 6 months ago

paravoid commented 1 year ago

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 https://github.com/ggerganov/llama.cpp/commit/ef156499721c67748cde01a5436cb6f0648bb4b4 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.

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 commented 1 year ago

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: https://github.com/WasmEdge/WasmEdge/commit/5308cc1c70e9515e8e6f12ae8c121e6ba8e1b958
  2. Agree. I will set up a new workflow for the new compilers.
hydai commented 1 year ago

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

hydai commented 1 year ago

/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 5 months ago

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.