From 65c2c1c5ab7c5089dbc6d10bc49b9c58f0164317 Mon Sep 17 00:00:00 2001 From: Cebtenzzre Date: Wed, 20 Sep 2023 12:06:08 -0400 Subject: [PATCH 1/2] benchmark-matmult : do not use integer abs() on a float (#3277) --- examples/benchmark/benchmark-matmult.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/examples/benchmark/benchmark-matmult.cpp b/examples/benchmark/benchmark-matmult.cpp index c8f7d486976d7b..f1c382aa9b9557 100644 --- a/examples/benchmark/benchmark-matmult.cpp +++ b/examples/benchmark/benchmark-matmult.cpp @@ -21,7 +21,7 @@ #pragma warning(disable: 4244 4267) // possible loss of data #endif -void ggml_graph_compute_helper(std::vector & buf, ggml_cgraph * graph, int n_threads) { +static void ggml_graph_compute_helper(std::vector & buf, ggml_cgraph * graph, int n_threads) { struct ggml_cplan plan = ggml_graph_plan(graph, n_threads); if (plan.work_size > 0) { @@ -32,7 +32,7 @@ void ggml_graph_compute_helper(std::vector & buf, ggml_cgraph * graph, ggml_graph_compute(graph, &plan); } -float tensor_sum_elements(const ggml_tensor * tensor) { +static float tensor_sum_elements(const ggml_tensor * tensor) { double sum = 0; if (tensor->type == GGML_TYPE_F32) { for (int j = 0; j < tensor->ne[1]; j++) { @@ -44,7 +44,7 @@ float tensor_sum_elements(const ggml_tensor * tensor) { return sum; } -void tensor_dump(const ggml_tensor * tensor, const char * name) { +static void tensor_dump(const ggml_tensor * tensor, const char * name) { printf("%15s: type = %i (%5s) ne = %5" PRIi64 " x %5" PRIi64 " x %5" PRIi64 ", nb = (%5zi, %5zi, %5zi) - ", name, tensor->type, ggml_type_name(tensor->type), tensor->ne[0], tensor->ne[1], tensor->ne[2], tensor->nb[0], tensor->nb[1], tensor->nb[2]); @@ -59,7 +59,7 @@ struct benchmark_params_struct { int32_t n_iterations = 10; }; -void print_usage(int /*argc*/, char ** argv, struct benchmark_params_struct params) { +static void print_usage(int /*argc*/, char ** argv, struct benchmark_params_struct params) { fprintf(stderr, "usage: %s [options]\n", argv[0]); fprintf(stderr, "\n"); fprintf(stderr, "options:\n"); @@ -253,7 +253,7 @@ int main(int argc, char ** argv) { // Check that the matrix multiplication result is in the right ballpark // We cannot use the exact value from the F32 multiplication because the quantizuation will be slightly different float sum_of_Q4_result = tensor_sum_elements(gf31.nodes[0]); - float delta = abs(sum_of_Q4_result - sum_of_F32_reference); + float delta = std::abs(sum_of_Q4_result - sum_of_F32_reference); float allowed_delta = (sum_of_F32_reference) / 1000 / 1000; // Let's accept an epsilon of 10^-6 if (delta > allowed_delta) { From a6b74764c7666d2b2cc9750a0668ae7644bf4fd5 Mon Sep 17 00:00:00 2001 From: Cebtenzzre Date: Tue, 19 Sep 2023 13:49:50 -0400 Subject: [PATCH 2/2] compiler version detection --- CMakeLists.txt | 54 +++++++++++++++++++++++--------------------------- Makefile | 52 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 64 insertions(+), 42 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 001574af7fd8c3..d1363c09a847f6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -412,42 +412,38 @@ endif() if (LLAMA_ALL_WARNINGS) if (NOT MSVC) - set(warning_flags - -Wall - -Wextra - -Wpedantic - -Wcast-qual - -Wno-unused-function - ) - if (CMAKE_C_COMPILER_ID MATCHES "Clang") # clang only + set(warning_flags -Wall -Wextra -Wpedantic -Wcast-qual -Wno-unused-function) + set(c_flags -Wshadow -Wstrict-prototypes -Wpointer-arith -Wmissing-prototypes -Werror=implicit-int + -Werror=implicit-function-declaration) + set(cxx_flags -Wmissing-declarations -Wmissing-noreturn) + + if (CMAKE_C_COMPILER_ID MATCHES "Clang") set(warning_flags ${warning_flags} -Wunreachable-code-break -Wunreachable-code-return) - endif() - set(c_flags - ${warning_flags} - -Wdouble-promotion - -Wshadow - -Wstrict-prototypes - -Wpointer-arith - -Wmissing-prototypes - -Werror=implicit-int - -Werror=implicit-function-declaration - ) - set(cxx_flags - ${warning_flags} - -Wmissing-declarations - -Wmissing-noreturn - -Wextra-semi - ) - if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") # clang++ only - set(cxx_flags ${cxx_flags} -Wmissing-prototypes) - elseif (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") # g++ only - set(cxx_flags ${cxx_flags} -Wno-format-truncation -Wno-array-bounds) + set(cxx_flags ${cxx_flags} -Wmissing-prototypes -Wextra-semi) + + if ( + (CMAKE_C_COMPILER_ID STREQUAL "Clang" AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 3.8.0) OR + (CMAKE_C_COMPILER_ID STREQUAL "AppleClang" AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 7.3.0) + ) + set(c_flags ${c_flags} -Wdouble-promotion) + endif() + elseif (CMAKE_C_COMPILER_ID STREQUAL "GNU") + set(c_flags ${c_flags} -Wdouble-promotion) + set(cxx_flags ${cxx_flags} -Wno-array-bounds) + + if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 7.1.0) + set(cxx_flags ${cxx_flags} -Wno-format-truncation) + endif() + if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 8.1.0) + set(cxx_flags ${cxx_flags} -Wextra-semi) + endif() endif() else() # todo : msvc endif() add_compile_options( + ${warning_flags} "$<$:${c_flags}>" "$<$:${cxx_flags}>" ) diff --git a/Makefile b/Makefile index 8118ea489afb3f..f7ba6add563f7d 100644 --- a/Makefile +++ b/Makefile @@ -19,6 +19,20 @@ ifndef UNAME_M UNAME_M := $(shell uname -m) endif +ifeq '' '$(findstring clang,$(shell $(CC) --version))' + CC_IS_GCC=1 + CC_VER := $(shell $(CC) -dumpfullversion -dumpversion | awk -F. '{ printf("%02d%02d%02d", $$1, $$2, $$3) }') +else + CC_IS_CLANG=1 + ifeq '' '$(findstring Apple LLVM,$(shell $(CC) --version))' + CC_IS_LLVM_CLANG=1 + else + CC_IS_APPLE_CLANG=1 + endif + CC_VER := $(shell $(CC) --version | sed -n 's/^.* version \([0-9.]*\) .*$$/\1/p' \ + | awk -F. '{ printf("%02d%02d%02d", $$1, $$2, $$3) }') +endif + # Mac OS + Arm can report x86_64 # ref: https://github.com/ggerganov/whisper.cpp/issues/66#issuecomment-1282546789 ifeq ($(UNAME_S),Darwin) @@ -87,9 +101,6 @@ CC := riscv64-unknown-linux-gnu-gcc CXX := riscv64-unknown-linux-gnu-g++ endif -CCV := $(shell $(CC) --version | head -n 1) -CXXV := $(shell $(CXX) --version | head -n 1) - # # Compile flags # @@ -174,21 +185,36 @@ endif # LLAMA_DISABLE_LOGS # warnings WARN_FLAGS = -Wall -Wextra -Wpedantic -Wcast-qual -Wno-unused-function -MK_CFLAGS += $(WARN_FLAGS) -Wdouble-promotion -Wshadow -Wstrict-prototypes -Wpointer-arith -Wmissing-prototypes \ - -Werror=implicit-int -Werror=implicit-function-declaration -MK_CXXFLAGS += $(WARN_FLAGS) -Wmissing-declarations -Wmissing-noreturn -Wextra-semi +MK_CFLAGS += $(WARN_FLAGS) -Wshadow -Wstrict-prototypes -Wpointer-arith -Wmissing-prototypes -Werror=implicit-int \ + -Werror=implicit-function-declaration +MK_CXXFLAGS += $(WARN_FLAGS) -Wmissing-declarations -Wmissing-noreturn # TODO(cebtenzzre): remove this once PR #2632 gets merged TTFS_CXXFLAGS = $(CXXFLAGS) -Wno-missing-declarations -ifneq '' '$(findstring clang,$(shell $(CC) --version))' - # clang only +ifeq ($(CC_IS_CLANG), 1) + # clang options MK_CFLAGS += -Wunreachable-code-break -Wunreachable-code-return - MK_HOST_CXXFLAGS += -Wunreachable-code-break -Wunreachable-code-return -Wmissing-prototypes + MK_HOST_CXXFLAGS += -Wunreachable-code-break -Wunreachable-code-return -Wmissing-prototypes -Wextra-semi TTFS_CXXFLAGS += -Wno-missing-prototypes + + ifneq '' '$(and $(CC_IS_LLVM_CLANG),$(filter 1,$(shell expr $(CC_VER) \>= 030800)))' + MK_CFLAGS += -Wdouble-promotion + endif + ifneq '' '$(and $(CC_IS_APPLE_CLANG),$(filter 1,$(shell expr $(CC_VER) \>= 070300)))' + MK_CFLAGS += -Wdouble-promotion + endif else - # gcc only - MK_HOST_CXXFLAGS += -Wno-format-truncation -Wno-array-bounds + # gcc options + MK_CFLAGS += -Wdouble-promotion + MK_HOST_CXXFLAGS += -Wno-array-bounds + + ifeq ($(shell expr $(CC_VER) \>= 070100), 1) + MK_HOST_CXXFLAGS += -Wno-format-truncation + endif + ifeq ($(shell expr $(CC_VER) \>= 080100), 1) + MK_HOST_CXXFLAGS += -Wextra-semi + endif endif # OS specific @@ -472,8 +498,8 @@ $(info I CFLAGS: $(CFLAGS)) $(info I CXXFLAGS: $(CXXFLAGS)) $(info I NVCCFLAGS: $(NVCCFLAGS)) $(info I LDFLAGS: $(LDFLAGS)) -$(info I CC: $(CCV)) -$(info I CXX: $(CXXV)) +$(info I CC: $(shell $(CC) --version | head -n 1)) +$(info I CXX: $(shell $(CXX) --version | head -n 1)) $(info ) #