From b39ae695558f9a383dd1913b8806eb19d859b5e1 Mon Sep 17 00:00:00 2001 From: ananta <12180395+ananta@users.noreply.github.com> Date: Sun, 5 Nov 2023 18:24:55 -0500 Subject: [PATCH] feat(ci): add an option to fail on compile warning --- .github/workflows/build.yml | 18 ++++++++++++------ CMakeLists.txt | 11 +++++++++++ Makefile | 5 +++++ ggml-metal.m | 2 +- ggml.c | 4 ++-- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bc295d52d..58d30aacd 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -37,6 +37,8 @@ jobs: - name: Build id: make_build + env: + LLAMA_FATAL_WARNINGS: 1 run: | CC=gcc-8 make -j $(nproc) @@ -65,7 +67,7 @@ jobs: run: | mkdir build cd build - cmake .. + cmake .. -DLLAMA_FATAL_WARNINGS=ON cmake --build . --config Release -j $(nproc) - name: Test @@ -100,7 +102,7 @@ jobs: run: | mkdir build cd build - cmake .. -DLLAMA_SANITIZE_${{ matrix.sanitizer }}=ON -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + cmake .. -DLLAMA_FATAL_WARNINGS=ON -DLLAMA_SANITIZE_${{ matrix.sanitizer }}=ON -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} cmake --build . --config ${{ matrix.build_type }} -j $(nproc) - name: Test @@ -134,7 +136,7 @@ jobs: run: | mkdir build cd build - cmake -DLLAMA_MPI=ON .. + cmake -DLLAMA_FATAL_WARNINGS=ON -DLLAMA_MPI=ON .. cmake --build . --config Release -j $(nproc) - name: Test @@ -159,6 +161,8 @@ jobs: - name: Build id: make_build + env: + LLAMA_FATAL_WARNINGS: 1 run: | make -j $(sysctl -n hw.logicalcpu) @@ -188,7 +192,7 @@ jobs: sysctl -a mkdir build cd build - cmake .. + cmake .. -DLLAMA_FATAL_WARNINGS=ON cmake --build . --config Release -j $(sysctl -n hw.logicalcpu) - name: Test @@ -218,6 +222,7 @@ jobs: mkdir build cd build cmake -G Xcode .. \ + -DLLAMA_FATAL_WARNINGS=ON \ -DLLAMA_BUILD_EXAMPLES=OFF \ -DLLAMA_BUILD_TESTS=OFF \ -DLLAMA_BUILD_SERVER=OFF \ @@ -246,6 +251,7 @@ jobs: mkdir build cd build cmake -G Xcode .. \ + -DLLAMA_FATAL_WARNINGS=ON \ -DLLAMA_BUILD_EXAMPLES=OFF \ -DLLAMA_BUILD_TESTS=OFF \ -DLLAMA_BUILD_SERVER=OFF \ @@ -352,7 +358,7 @@ jobs: run: | mkdir build cd build - cmake .. ${{ matrix.defines }} + cmake .. -DLLAMA_FATAL_WARNINGS=ON ${{ matrix.defines }} cmake --build . --config Release -j ${env:NUMBER_OF_PROCESSORS} - name: Add clblast.dll @@ -455,7 +461,7 @@ jobs: run: | mkdir build cd build - cmake .. -DLLAMA_NATIVE=OFF -DLLAMA_BUILD_SERVER=ON -DLLAMA_CUBLAS=ON -DBUILD_SHARED_LIBS=ON + cmake .. -DLLAMA_FATAL_WARNINGS=ON -DLLAMA_NATIVE=OFF -DLLAMA_BUILD_SERVER=ON -DLLAMA_CUBLAS=ON -DBUILD_SHARED_LIBS=ON cmake --build . --config Release -j ${env:NUMBER_OF_PROCESSORS} - name: Determine tag name diff --git a/CMakeLists.txt b/CMakeLists.txt index f32df5fe5..aa370c881 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,6 +52,9 @@ option(LLAMA_ALL_WARNINGS "llama: enable all compiler warnings" option(LLAMA_ALL_WARNINGS_3RD_PARTY "llama: enable all compiler warnings in 3rd party libs" OFF) option(LLAMA_GPROF "llama: enable gprof" OFF) +# build +option(LLAMA_FATAL_WARNINGS "llama: enable -Werror flag" OFF) + # sanitizers option(LLAMA_SANITIZE_THREAD "llama: enable thread sanitizer" OFF) option(LLAMA_SANITIZE_ADDRESS "llama: enable address sanitizer" OFF) @@ -112,6 +115,14 @@ set(THREADS_PREFER_PTHREAD_FLAG ON) find_package(Threads REQUIRED) include(CheckCXXCompilerFlag) +if (LLAMA_FATAL_WARNINGS) + if (CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_compile_options(-Werror) + elseif (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + add_compile_options(/WX) + endif() +endif() + if (NOT MSVC) if (LLAMA_SANITIZE_THREAD) add_compile_options(-fsanitize=thread) diff --git a/Makefile b/Makefile index 95d85236f..104a95bbb 100644 --- a/Makefile +++ b/Makefile @@ -215,6 +215,11 @@ MK_CFLAGS += $(WARN_FLAGS) -Wshadow -Wstrict-prototypes -Wpointer-arith -Wmis -Werror=implicit-function-declaration MK_CXXFLAGS += $(WARN_FLAGS) -Wmissing-declarations -Wmissing-noreturn +ifeq ($(LLAMA_FATAL_WARNINGS),1) + MK_CFLAGS += -Werror + MK_CXXFLAGS += -Werror +endif + ifeq ($(CC_IS_CLANG), 1) # clang options MK_CFLAGS += -Wunreachable-code-break -Wunreachable-code-return diff --git a/ggml-metal.m b/ggml-metal.m index d52a1c3c4..040e5b88e 100644 --- a/ggml-metal.m +++ b/ggml-metal.m @@ -128,7 +128,7 @@ struct ggml_metal_context { // MSL code // TODO: move the contents here when ready // for now it is easier to work in a separate file -//static NSString * const msl_library_source = @"see metal.metal"; +// static NSString * const msl_library_source = @"see metal.metal"; // Here to assist with NSBundle Path Hack @interface GGMLMetalClass : NSObject diff --git a/ggml.c b/ggml.c index 0c7264a36..1c8c7a108 100644 --- a/ggml.c +++ b/ggml.c @@ -786,7 +786,7 @@ do { \ const __m128 t0 = _mm_add_ps(_mm256_castps256_ps128(x[0]), \ _mm256_extractf128_ps(x[0], 1)); \ const __m128 t1 = _mm_hadd_ps(t0, t0); \ - res = _mm_cvtss_f32(_mm_hadd_ps(t1, t1)); \ + res = (ggml_float) _mm_cvtss_f32(_mm_hadd_ps(t1, t1)); \ } while (0) // TODO: is this optimal ? @@ -1067,7 +1067,7 @@ inline static void __wasm_f16x4_store(ggml_fp16_t * p, v128_t x) { x[i] = _mm_add_ps(x[i], x[offset+i]); \ } \ const __m128 t0 = _mm_hadd_ps(x[0], x[0]); \ - res = _mm_cvtss_f32(_mm_hadd_ps(t0, t0)); \ + res = (ggml_float) _mm_cvtss_f32(_mm_hadd_ps(t0, t0)); \ } // TODO: is this optimal ?