From e908ace7175d51672a3b7f405191a68b5b019aba Mon Sep 17 00:00:00 2001 From: Georgi Gerganov Date: Sun, 24 Nov 2024 15:00:51 +0200 Subject: [PATCH 1/6] cmake : enable warnings in llama ggml-ci --- CMakeLists.txt | 1 + cmake/common.cmake | 24 ++++++++++++++++++++++++ common/CMakeLists.txt | 2 ++ examples/CMakeLists.txt | 4 ++++ src/CMakeLists.txt | 2 ++ 5 files changed, 33 insertions(+) create mode 100644 cmake/common.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 994e61e45fedd..668ca086ba436 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -82,6 +82,7 @@ option(LLAMA_CURL "llama: use libcurl to download model from an URL" OFF) # Required for relocatable CMake package include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/build-info.cmake) +include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/common.cmake) # override ggml options set(GGML_SANITIZE_THREAD ${LLAMA_SANITIZE_THREAD}) diff --git a/cmake/common.cmake b/cmake/common.cmake new file mode 100644 index 0000000000000..c761559d7dfd9 --- /dev/null +++ b/cmake/common.cmake @@ -0,0 +1,24 @@ +function(llama_add_compile_flags) + if (LLAMA_ALL_WARNINGS) + if (NOT MSVC) + list(APPEND C_FLAGS -Wshadow -Wstrict-prototypes -Wpointer-arith -Wmissing-prototypes + -Werror=implicit-int -Werror=implicit-function-declaration) + + list(APPEND CXX_FLAGS -Wmissing-declarations -Wmissing-noreturn) + + list(APPEND WARNING_FLAGS -Wall -Wextra -Wpedantic -Wcast-qual -Wno-unused-function) + + list(APPEND C_FLAGS ${WARNING_FLAGS}) + list(APPEND CXX_FLAGS ${WARNING_FLAGS}) + + get_flags(${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}) + + add_compile_options("$<$:${C_FLAGS};${GF_C_FLAGS}>" + "$<$:${CXX_FLAGS};${GF_CXX_FLAGS}>") + else() + # todo : msvc + set(C_FLAGS "" PARENT_SCOPE) + set(CXX_FLAGS "" PARENT_SCOPE) + endif() + endif() +endfunction() diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index 62a8a7db5652f..2231748848bf5 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -2,6 +2,8 @@ find_package(Threads REQUIRED) +llama_add_compile_flags() + # Build info header # diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index 632409d5591b9..d33a932ed085c 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -6,6 +6,10 @@ find_package(Threads REQUIRED) # ... +# flags + +llama_add_compile_flags() + # examples include_directories(${CMAKE_CURRENT_SOURCE_DIR}) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a86624750305e..2f581b9218728 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -5,6 +5,8 @@ if (WIN32) endif() endif() +llama_add_compile_flags() + # # libraries # From f1c0a93b581e4e0c13eb6fdd92a49cdf731efb83 Mon Sep 17 00:00:00 2001 From: Georgi Gerganov Date: Mon, 25 Nov 2024 23:43:26 +0200 Subject: [PATCH 2/6] cmake : add llama_get_flags and respect LLAMA_FATAL_WARNINGS --- cmake/common.cmake | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/cmake/common.cmake b/cmake/common.cmake index c761559d7dfd9..4a2552a7600ce 100644 --- a/cmake/common.cmake +++ b/cmake/common.cmake @@ -1,4 +1,39 @@ +function(llama_get_flags CCID CCVER) + set(C_FLAGS "") + set(CXX_FLAGS "") + + if (CCID MATCHES "Clang") + set(C_FLAGS -Wunreachable-code-break -Wunreachable-code-return) + set(CXX_FLAGS -Wunreachable-code-break -Wunreachable-code-return -Wmissing-prototypes -Wextra-semi) + + if ( + (CCID STREQUAL "Clang" AND CCVER VERSION_GREATER_EQUAL 3.8.0) OR + (CCID STREQUAL "AppleClang" AND CCVER VERSION_GREATER_EQUAL 7.3.0) + ) + list(APPEND C_FLAGS -Wdouble-promotion) + endif() + elseif (CCID STREQUAL "GNU") + set(C_FLAGS -Wdouble-promotion) + set(CXX_FLAGS -Wno-array-bounds) + if (CCVER VERSION_GREATER_EQUAL 8.1.0) + list(APPEND CXX_FLAGS -Wextra-semi) + endif() + endif() + + set(GF_C_FLAGS ${C_FLAGS} PARENT_SCOPE) + set(GF_CXX_FLAGS ${CXX_FLAGS} PARENT_SCOPE) +endfunction() + function(llama_add_compile_flags) + if (LLAMA_FATAL_WARNINGS) + if (CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + list(APPEND C_FLAGS -Werror) + list(APPEND CXX_FLAGS -Werror) + elseif (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + add_compile_options(/WX) + endif() + endif() + if (LLAMA_ALL_WARNINGS) if (NOT MSVC) list(APPEND C_FLAGS -Wshadow -Wstrict-prototypes -Wpointer-arith -Wmissing-prototypes @@ -11,7 +46,7 @@ function(llama_add_compile_flags) list(APPEND C_FLAGS ${WARNING_FLAGS}) list(APPEND CXX_FLAGS ${WARNING_FLAGS}) - get_flags(${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}) + llama_get_flags(${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}) add_compile_options("$<$:${C_FLAGS};${GF_C_FLAGS}>" "$<$:${CXX_FLAGS};${GF_CXX_FLAGS}>") From 5ea6fc59e935a5991c4ea813cbd2d875590a2f1e Mon Sep 17 00:00:00 2001 From: Georgi Gerganov Date: Mon, 25 Nov 2024 23:43:45 +0200 Subject: [PATCH 3/6] cmake : get_flags -> ggml_get_flags --- ggml/src/CMakeLists.txt | 5 +++-- ggml/src/ggml-cuda/CMakeLists.txt | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ggml/src/CMakeLists.txt b/ggml/src/CMakeLists.txt index 071508ddae021..9022aa3ae197d 100644 --- a/ggml/src/CMakeLists.txt +++ b/ggml/src/CMakeLists.txt @@ -24,7 +24,7 @@ if (NOT MSVC) endif() endif() -function(get_flags CCID CCVER) +function(ggml_get_flags CCID CCVER) set(C_FLAGS "") set(CXX_FLAGS "") @@ -41,6 +41,7 @@ function(get_flags CCID CCVER) elseif (CCID STREQUAL "GNU") set(C_FLAGS -Wdouble-promotion) set(CXX_FLAGS -Wno-array-bounds) + if (CCVER VERSION_GREATER_EQUAL 8.1.0) list(APPEND CXX_FLAGS -Wextra-semi) endif() @@ -69,7 +70,7 @@ if (GGML_ALL_WARNINGS) list(APPEND C_FLAGS ${WARNING_FLAGS}) list(APPEND CXX_FLAGS ${WARNING_FLAGS}) - get_flags(${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}) + ggml_get_flags(${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}) add_compile_options("$<$:${C_FLAGS};${GF_C_FLAGS}>" "$<$:${CXX_FLAGS};${GF_CXX_FLAGS}>") diff --git a/ggml/src/ggml-cuda/CMakeLists.txt b/ggml/src/ggml-cuda/CMakeLists.txt index b0cb93e070fd3..14761650f8a8e 100644 --- a/ggml/src/ggml-cuda/CMakeLists.txt +++ b/ggml/src/ggml-cuda/CMakeLists.txt @@ -132,7 +132,7 @@ if (CUDAToolkit_FOUND) message("-- CUDA host compiler is ${CUDA_CCID} ${CUDA_CCVER}") - get_flags(${CUDA_CCID} ${CUDA_CCVER}) + ggml_get_flags(${CUDA_CCID} ${CUDA_CCVER}) list(APPEND CUDA_CXX_FLAGS ${CXX_FLAGS} ${GF_CXX_FLAGS}) # This is passed to -Xcompiler later endif() From 7177eb8901994c793bbaa63a5a2101a418ba1713 Mon Sep 17 00:00:00 2001 From: Georgi Gerganov Date: Mon, 25 Nov 2024 23:43:55 +0200 Subject: [PATCH 4/6] speculative-simple : fix warnings --- examples/speculative-simple/speculative-simple.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/speculative-simple/speculative-simple.cpp b/examples/speculative-simple/speculative-simple.cpp index 7bf9056bf6db1..b317792e629d0 100644 --- a/examples/speculative-simple/speculative-simple.cpp +++ b/examples/speculative-simple/speculative-simple.cpp @@ -70,13 +70,13 @@ int main(int argc, char ** argv) { std::vector inp; inp = common_tokenize(ctx_tgt, params.prompt, true, true); - if (llama_n_ctx(ctx_tgt) < (int) inp.size()) { + if (llama_n_ctx(ctx_tgt) < (uint32_t) inp.size()) { LOG_ERR("%s: the prompt exceeds the context size (%d tokens, ctx %d)\n", __func__, (int) inp.size(), llama_n_ctx(ctx_tgt)); return 1; } - if (llama_n_batch(ctx_tgt) < (int) inp.size()) { + if (llama_n_batch(ctx_tgt) < (uint32_t) inp.size()) { LOG_ERR("%s: the prompt exceeds the batch size (%d tokens, batch %d)\n", __func__, (int) inp.size(), llama_n_batch(ctx_tgt)); return 1; @@ -154,7 +154,7 @@ int main(int argc, char ** argv) { // evaluate the target model on [id_last, draft0, draft1, ..., draftN-1] { // do not waste time on small drafts - if (draft.size() < n_draft_min) { + if (draft.size() < (size_t) n_draft_min) { draft.clear(); } From 33d49f7c5a9d37d9010d121b44e80c94c7f7a906 Mon Sep 17 00:00:00 2001 From: Georgi Gerganov Date: Mon, 25 Nov 2024 23:54:43 +0200 Subject: [PATCH 5/6] cmake : reuse ggml_get_flags ggml-ci --- cmake/common.cmake | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/cmake/common.cmake b/cmake/common.cmake index 4a2552a7600ce..0f54871e4143d 100644 --- a/cmake/common.cmake +++ b/cmake/common.cmake @@ -1,29 +1,3 @@ -function(llama_get_flags CCID CCVER) - set(C_FLAGS "") - set(CXX_FLAGS "") - - if (CCID MATCHES "Clang") - set(C_FLAGS -Wunreachable-code-break -Wunreachable-code-return) - set(CXX_FLAGS -Wunreachable-code-break -Wunreachable-code-return -Wmissing-prototypes -Wextra-semi) - - if ( - (CCID STREQUAL "Clang" AND CCVER VERSION_GREATER_EQUAL 3.8.0) OR - (CCID STREQUAL "AppleClang" AND CCVER VERSION_GREATER_EQUAL 7.3.0) - ) - list(APPEND C_FLAGS -Wdouble-promotion) - endif() - elseif (CCID STREQUAL "GNU") - set(C_FLAGS -Wdouble-promotion) - set(CXX_FLAGS -Wno-array-bounds) - if (CCVER VERSION_GREATER_EQUAL 8.1.0) - list(APPEND CXX_FLAGS -Wextra-semi) - endif() - endif() - - set(GF_C_FLAGS ${C_FLAGS} PARENT_SCOPE) - set(GF_CXX_FLAGS ${CXX_FLAGS} PARENT_SCOPE) -endfunction() - function(llama_add_compile_flags) if (LLAMA_FATAL_WARNINGS) if (CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") @@ -46,7 +20,7 @@ function(llama_add_compile_flags) list(APPEND C_FLAGS ${WARNING_FLAGS}) list(APPEND CXX_FLAGS ${WARNING_FLAGS}) - llama_get_flags(${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}) + ggml_get_flags(${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}) add_compile_options("$<$:${C_FLAGS};${GF_C_FLAGS}>" "$<$:${CXX_FLAGS};${GF_CXX_FLAGS}>") From 6fab3ffe02c036db3d2005efeb6655b7050f318e Mon Sep 17 00:00:00 2001 From: Georgi Gerganov Date: Tue, 26 Nov 2024 00:06:45 +0200 Subject: [PATCH 6/6] speculative-simple : fix compile warning ggml-ci --- examples/speculative-simple/speculative-simple.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/speculative-simple/speculative-simple.cpp b/examples/speculative-simple/speculative-simple.cpp index b317792e629d0..90cb5074995e4 100644 --- a/examples/speculative-simple/speculative-simple.cpp +++ b/examples/speculative-simple/speculative-simple.cpp @@ -190,7 +190,7 @@ int main(int argc, char ** argv) { // in this case, we do it for a group of accepted tokens at once // { - llama_token id; + llama_token id = 0; std::string token_str; for (size_t i = 0; i < ids.size(); ++i) {