Skip to content

Commit 4293cbb

Browse files
cebtenzzreyusiwen
cebtenzzre
authored and
yusiwen
committed
build : enable more non-default compiler warnings (ggml-org#3200)
1 parent 86f2016 commit 4293cbb

File tree

16 files changed

+287
-269
lines changed

16 files changed

+287
-269
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ models-mnt
4545
/main
4646
/metal
4747
/perplexity
48+
/q8dot
4849
/quantize
4950
/quantize-stats
5051
/result

‎CMakeLists.txt‎

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -414,37 +414,38 @@ endif()
414414

415415
if (LLAMA_ALL_WARNINGS)
416416
if (NOT MSVC)
417-
set(c_flags
418-
-Wall
419-
-Wextra
420-
-Wpedantic
421-
-Wcast-qual
422-
-Wdouble-promotion
423-
-Wshadow
424-
-Wstrict-prototypes
425-
-Wpointer-arith
426-
-Wmissing-prototypes
427-
-Werror=implicit-int
428-
-Wno-unused-function
429-
)
430-
set(cxx_flags
431-
-Wall
432-
-Wextra
433-
-Wpedantic
434-
-Wcast-qual
435-
-Wmissing-declarations
436-
-Wno-unused-function
437-
-Wno-multichar
438-
)
439-
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
440-
# g++ only
441-
set(cxx_flags ${cxx_flags} -Wno-format-truncation -Wno-array-bounds)
417+
set(warning_flags -Wall -Wextra -Wpedantic -Wcast-qual -Wno-unused-function)
418+
set(c_flags -Wshadow -Wstrict-prototypes -Wpointer-arith -Wmissing-prototypes -Werror=implicit-int
419+
-Werror=implicit-function-declaration)
420+
set(cxx_flags -Wmissing-declarations -Wmissing-noreturn)
421+
422+
if (CMAKE_C_COMPILER_ID MATCHES "Clang")
423+
set(warning_flags ${warning_flags} -Wunreachable-code-break -Wunreachable-code-return)
424+
set(cxx_flags ${cxx_flags} -Wmissing-prototypes -Wextra-semi)
425+
426+
if (
427+
(CMAKE_C_COMPILER_ID STREQUAL "Clang" AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 3.8.0) OR
428+
(CMAKE_C_COMPILER_ID STREQUAL "AppleClang" AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 7.3.0)
429+
)
430+
set(c_flags ${c_flags} -Wdouble-promotion)
431+
endif()
432+
elseif (CMAKE_C_COMPILER_ID STREQUAL "GNU")
433+
set(c_flags ${c_flags} -Wdouble-promotion)
434+
set(cxx_flags ${cxx_flags} -Wno-array-bounds)
435+
436+
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 7.1.0)
437+
set(cxx_flags ${cxx_flags} -Wno-format-truncation)
438+
endif()
439+
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 8.1.0)
440+
set(cxx_flags ${cxx_flags} -Wextra-semi)
441+
endif()
442442
endif()
443443
else()
444444
# todo : msvc
445445
endif()
446446

447447
add_compile_options(
448+
${warning_flags}
448449
"$<$<COMPILE_LANGUAGE:C>:${c_flags}>"
449450
"$<$<COMPILE_LANGUAGE:CXX>:${cxx_flags}>"
450451
)

‎Makefile‎

Lines changed: 52 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Define the default target now so that it is always the first target
2-
BUILD_TARGETS = main quantize quantize-stats perplexity embedding vdot train-text-from-scratch convert-llama2c-to-ggml simple batched save-load-state server embd-input-test gguf llama-bench baby-llama beam-search speculative parallel finetune export-lora tests/test-c.o
2+
BUILD_TARGETS = main quantize quantize-stats perplexity embedding vdot q8dot train-text-from-scratch convert-llama2c-to-ggml simple batched save-load-state server embd-input-test gguf llama-bench baby-llama beam-search speculative benchmark-matmult parallel finetune export-lora tests/test-c.o
33

44
# Binaries only useful for tests
55
TEST_TARGETS = tests/test-llama-grammar tests/test-grammar-parser tests/test-double-float tests/test-grad0 tests/test-opt tests/test-quantize-fns tests/test-quantize-perf tests/test-sampling tests/test-tokenizer-0-llama tests/test-tokenizer-0-falcon tests/test-tokenizer-1-llama
@@ -19,6 +19,20 @@ ifndef UNAME_M
1919
UNAME_M := $(shell uname -m)
2020
endif
2121

22+
ifeq '' '$(findstring clang,$(shell $(CC) --version))'
23+
CC_IS_GCC=1
24+
CC_VER := $(shell $(CC) -dumpfullversion -dumpversion | awk -F. '{ printf("%02d%02d%02d", $$1, $$2, $$3) }')
25+
else
26+
CC_IS_CLANG=1
27+
ifeq '' '$(findstring Apple LLVM,$(shell $(CC) --version))'
28+
CC_IS_LLVM_CLANG=1
29+
else
30+
CC_IS_APPLE_CLANG=1
31+
endif
32+
CC_VER := $(shell $(CC) --version | sed -n 's/^.* version \([0-9.]*\).*$$/\1/p' \
33+
| awk -F. '{ printf("%02d%02d%02d", $$1, $$2, $$3) }')
34+
endif
35+
2236
# Mac OS + Arm can report x86_64
2337
# ref: https://github.com/ggerganov/whisper.cpp/issues/66#issuecomment-1282546789
2438
ifeq ($(UNAME_S),Darwin)
@@ -87,9 +101,6 @@ CC := riscv64-unknown-linux-gnu-gcc
87101
CXX := riscv64-unknown-linux-gnu-g++
88102
endif
89103

90-
CCV := $(shell $(CC) --version | head -n 1)
91-
CXXV := $(shell $(CXX) --version | head -n 1)
92-
93104
#
94105
# Compile flags
95106
#
@@ -173,20 +184,33 @@ ifdef LLAMA_DISABLE_LOGS
173184
endif # LLAMA_DISABLE_LOGS
174185

175186
# warnings
176-
MK_CFLAGS += -Wall -Wextra -Wpedantic -Wcast-qual -Wdouble-promotion -Wshadow -Wstrict-prototypes -Wpointer-arith \
177-
-Wmissing-prototypes -Werror=implicit-int -Wno-unused-function
178-
MK_CXXFLAGS += -Wall -Wextra -Wpedantic -Wcast-qual -Wmissing-declarations -Wno-unused-function -Wno-multichar
179-
180-
# TODO(cebtenzzre): remove this once PR #2632 gets merged
181-
TTFS_CXXFLAGS = $(CXXFLAGS) -Wno-missing-declarations
182-
183-
ifneq '' '$(findstring clang,$(shell $(CXX) --version))'
184-
# clang++ only
185-
MK_CXXFLAGS += -Wmissing-prototypes
186-
TTFS_CXXFLAGS += -Wno-missing-prototypes
187+
WARN_FLAGS = -Wall -Wextra -Wpedantic -Wcast-qual -Wno-unused-function
188+
MK_CFLAGS += $(WARN_FLAGS) -Wshadow -Wstrict-prototypes -Wpointer-arith -Wmissing-prototypes -Werror=implicit-int \
189+
-Werror=implicit-function-declaration
190+
MK_CXXFLAGS += $(WARN_FLAGS) -Wmissing-declarations -Wmissing-noreturn
191+
192+
ifeq ($(CC_IS_CLANG), 1)
193+
# clang options
194+
MK_CFLAGS += -Wunreachable-code-break -Wunreachable-code-return
195+
MK_HOST_CXXFLAGS += -Wunreachable-code-break -Wunreachable-code-return -Wmissing-prototypes -Wextra-semi
196+
197+
ifneq '' '$(and $(CC_IS_LLVM_CLANG),$(filter 1,$(shell expr $(CC_VER) \>= 030800)))'
198+
MK_CFLAGS += -Wdouble-promotion
199+
endif
200+
ifneq '' '$(and $(CC_IS_APPLE_CLANG),$(filter 1,$(shell expr $(CC_VER) \>= 070300)))'
201+
MK_CFLAGS += -Wdouble-promotion
202+
endif
187203
else
188-
# g++ only
189-
MK_CXXFLAGS += -Wno-format-truncation -Wno-array-bounds
204+
# gcc options
205+
MK_CFLAGS += -Wdouble-promotion
206+
MK_HOST_CXXFLAGS += -Wno-array-bounds
207+
208+
ifeq ($(shell expr $(CC_VER) \>= 070100), 1)
209+
MK_HOST_CXXFLAGS += -Wno-format-truncation
210+
endif
211+
ifeq ($(shell expr $(CC_VER) \>= 080100), 1)
212+
MK_HOST_CXXFLAGS += -Wextra-semi
213+
endif
190214
endif
191215

192216
# OS specific
@@ -383,7 +407,7 @@ ifdef LLAMA_CUDA_CCBIN
383407
NVCCFLAGS += -ccbin $(LLAMA_CUDA_CCBIN)
384408
endif
385409
ggml-cuda.o: ggml-cuda.cu ggml-cuda.h
386-
$(NVCC) $(NVCCFLAGS) -Wno-pedantic -c $< -o $@
410+
$(NVCC) $(NVCCFLAGS) -c $< -o $@
387411
endif # LLAMA_CUBLAS
388412

389413
ifdef LLAMA_CLBLAST
@@ -473,8 +497,8 @@ $(info I CFLAGS: $(CFLAGS))
473497
$(info I CXXFLAGS: $(CXXFLAGS))
474498
$(info I NVCCFLAGS: $(NVCCFLAGS))
475499
$(info I LDFLAGS: $(LDFLAGS))
476-
$(info I CC: $(CCV))
477-
$(info I CXX: $(CXXV))
500+
$(info I CC: $(shell $(CC) --version | head -n 1))
501+
$(info I CXX: $(shell $(CXX) --version | head -n 1))
478502
$(info )
479503

480504
#
@@ -555,7 +579,7 @@ gguf: examples/gguf/gguf.cpp ggml.o llama.o $(OBJS)
555579
$(CXX) $(CXXFLAGS) $(filter-out %.h,$^) -o $@ $(LDFLAGS)
556580

557581
train-text-from-scratch: examples/train-text-from-scratch/train-text-from-scratch.cpp ggml.o llama.o common.o train.o $(OBJS)
558-
$(CXX) $(TTFS_CXXFLAGS) $(filter-out %.h,$^) -o $@ $(LDFLAGS)
582+
$(CXX) $(CXXFLAGS) $(filter-out %.h,$^) -o $@ $(LDFLAGS)
559583

560584
convert-llama2c-to-ggml: examples/convert-llama2c-to-ggml/convert-llama2c-to-ggml.cpp ggml.o llama.o $(OBJS)
561585
$(CXX) $(CXXFLAGS) $(filter-out %.h,$^) -o $@ $(LDFLAGS)
@@ -602,11 +626,18 @@ tests: $(TEST_TARGETS)
602626

603627
benchmark-matmult: examples/benchmark/benchmark-matmult.cpp build-info.h ggml.o $(OBJS)
604628
$(CXX) $(CXXFLAGS) $(filter-out %.h,$^) -o $@ $(LDFLAGS)
629+
630+
run-benchmark-matmult: benchmark-matmult
605631
./$@
606632

633+
.PHONY: run-benchmark-matmult
634+
607635
vdot: pocs/vdot/vdot.cpp ggml.o $(OBJS)
608636
$(CXX) $(CXXFLAGS) $^ -o $@ $(LDFLAGS)
609637

638+
q8dot: pocs/vdot/q8dot.cpp ggml.o $(OBJS)
639+
$(CXX) $(CXXFLAGS) $^ -o $@ $(LDFLAGS)
640+
610641
tests/test-llama-grammar: tests/test-llama-grammar.cpp build-info.h ggml.o common.o grammar-parser.o $(OBJS)
611642
$(CXX) $(CXXFLAGS) $(filter-out %.h,$^) -o $@ $(LDFLAGS)
612643

‎common/common.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -755,10 +755,9 @@ std::string gpt_random_prompt(std::mt19937 & rng) {
755755
case 7: return "He";
756756
case 8: return "She";
757757
case 9: return "They";
758-
default: return "To";
759758
}
760759

761-
return "The";
760+
GGML_UNREACHABLE();
762761
}
763762

764763
//

‎common/log.h‎

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -225,31 +225,31 @@ enum LogTriState
225225
// USE LOG() INSTEAD
226226
//
227227
#ifndef _MSC_VER
228-
#define LOG_IMPL(str, ...) \
229-
{ \
228+
#define LOG_IMPL(str, ...) \
229+
do { \
230230
if (LOG_TARGET != nullptr) \
231231
{ \
232232
fprintf(LOG_TARGET, LOG_TIMESTAMP_FMT LOG_FLF_FMT str "%s" LOG_TIMESTAMP_VAL LOG_FLF_VAL, __VA_ARGS__); \
233233
fflush(LOG_TARGET); \
234234
} \
235-
}
235+
} while (0)
236236
#else
237-
#define LOG_IMPL(str, ...) \
238-
{ \
237+
#define LOG_IMPL(str, ...) \
238+
do { \
239239
if (LOG_TARGET != nullptr) \
240240
{ \
241241
fprintf(LOG_TARGET, LOG_TIMESTAMP_FMT LOG_FLF_FMT str "%s" LOG_TIMESTAMP_VAL LOG_FLF_VAL "", ##__VA_ARGS__); \
242242
fflush(LOG_TARGET); \
243243
} \
244-
}
244+
} while (0)
245245
#endif
246246

247247
// INTERNAL, DO NOT USE
248248
// USE LOG_TEE() INSTEAD
249249
//
250250
#ifndef _MSC_VER
251-
#define LOG_TEE_IMPL(str, ...) \
252-
{ \
251+
#define LOG_TEE_IMPL(str, ...) \
252+
do { \
253253
if (LOG_TARGET != nullptr) \
254254
{ \
255255
fprintf(LOG_TARGET, LOG_TIMESTAMP_FMT LOG_FLF_FMT str "%s" LOG_TIMESTAMP_VAL LOG_FLF_VAL, __VA_ARGS__); \
@@ -260,10 +260,10 @@ enum LogTriState
260260
fprintf(LOG_TEE_TARGET, LOG_TEE_TIMESTAMP_FMT LOG_TEE_FLF_FMT str "%s" LOG_TEE_TIMESTAMP_VAL LOG_TEE_FLF_VAL, __VA_ARGS__); \
261261
fflush(LOG_TEE_TARGET); \
262262
} \
263-
}
263+
} while (0)
264264
#else
265-
#define LOG_TEE_IMPL(str, ...) \
266-
{ \
265+
#define LOG_TEE_IMPL(str, ...) \
266+
do { \
267267
if (LOG_TARGET != nullptr) \
268268
{ \
269269
fprintf(LOG_TARGET, LOG_TIMESTAMP_FMT LOG_FLF_FMT str "%s" LOG_TIMESTAMP_VAL LOG_FLF_VAL "", ##__VA_ARGS__); \
@@ -274,7 +274,7 @@ enum LogTriState
274274
fprintf(LOG_TEE_TARGET, LOG_TEE_TIMESTAMP_FMT LOG_TEE_FLF_FMT str "%s" LOG_TEE_TIMESTAMP_VAL LOG_TEE_FLF_VAL "", ##__VA_ARGS__); \
275275
fflush(LOG_TEE_TARGET); \
276276
} \
277-
}
277+
} while (0)
278278
#endif
279279

280280
// The '\0' as a last argument, is a trick to bypass the silly
@@ -435,41 +435,41 @@ inline FILE *log_handler() { return log_handler1_impl(); }
435435
inline void log_test()
436436
{
437437
log_disable();
438-
LOG("01 Hello World to nobody, because logs are disabled!\n")
438+
LOG("01 Hello World to nobody, because logs are disabled!\n");
439439
log_enable();
440-
LOG("02 Hello World to default output, which is \"%s\" ( Yaaay, arguments! )!\n", LOG_STRINGIZE(LOG_TARGET))
441-
LOG_TEE("03 Hello World to **both** default output and " LOG_TEE_TARGET_STRING "!\n")
440+
LOG("02 Hello World to default output, which is \"%s\" ( Yaaay, arguments! )!\n", LOG_STRINGIZE(LOG_TARGET));
441+
LOG_TEE("03 Hello World to **both** default output and " LOG_TEE_TARGET_STRING "!\n");
442442
log_set_target(stderr);
443-
LOG("04 Hello World to stderr!\n")
444-
LOG_TEE("05 Hello World TEE with double printing to stderr prevented!\n")
443+
LOG("04 Hello World to stderr!\n");
444+
LOG_TEE("05 Hello World TEE with double printing to stderr prevented!\n");
445445
log_set_target(LOG_DEFAULT_FILE_NAME);
446-
LOG("06 Hello World to default log file!\n")
446+
LOG("06 Hello World to default log file!\n");
447447
log_set_target(stdout);
448-
LOG("07 Hello World to stdout!\n")
448+
LOG("07 Hello World to stdout!\n");
449449
log_set_target(LOG_DEFAULT_FILE_NAME);
450-
LOG("08 Hello World to default log file again!\n")
450+
LOG("08 Hello World to default log file again!\n");
451451
log_disable();
452-
LOG("09 Hello World _1_ into the void!\n")
452+
LOG("09 Hello World _1_ into the void!\n");
453453
log_enable();
454-
LOG("10 Hello World back from the void ( you should not see _1_ in the log or the output )!\n")
454+
LOG("10 Hello World back from the void ( you should not see _1_ in the log or the output )!\n");
455455
log_disable();
456456
log_set_target("llama.anotherlog.log");
457-
LOG("11 Hello World _2_ to nobody, new target was selected but logs are still disabled!\n")
457+
LOG("11 Hello World _2_ to nobody, new target was selected but logs are still disabled!\n");
458458
log_enable();
459-
LOG("12 Hello World this time in a new file ( you should not see _2_ in the log or the output )?\n")
459+
LOG("12 Hello World this time in a new file ( you should not see _2_ in the log or the output )?\n");
460460
log_set_target("llama.yetanotherlog.log");
461-
LOG("13 Hello World this time in yet new file?\n")
461+
LOG("13 Hello World this time in yet new file?\n");
462462
log_set_target(log_filename_generator("llama_autonamed", "log"));
463-
LOG("14 Hello World in log with generated filename!\n")
463+
LOG("14 Hello World in log with generated filename!\n");
464464
#ifdef _MSC_VER
465-
LOG_TEE("15 Hello msvc TEE without arguments\n")
466-
LOG_TEE("16 Hello msvc TEE with (%d)(%s) arguments\n", 1, "test")
467-
LOG_TEELN("17 Hello msvc TEELN without arguments\n")
468-
LOG_TEELN("18 Hello msvc TEELN with (%d)(%s) arguments\n", 1, "test")
469-
LOG("19 Hello msvc LOG without arguments\n")
470-
LOG("20 Hello msvc LOG with (%d)(%s) arguments\n", 1, "test")
471-
LOGLN("21 Hello msvc LOGLN without arguments\n")
472-
LOGLN("22 Hello msvc LOGLN with (%d)(%s) arguments\n", 1, "test")
465+
LOG_TEE("15 Hello msvc TEE without arguments\n");
466+
LOG_TEE("16 Hello msvc TEE with (%d)(%s) arguments\n", 1, "test");
467+
LOG_TEELN("17 Hello msvc TEELN without arguments\n");
468+
LOG_TEELN("18 Hello msvc TEELN with (%d)(%s) arguments\n", 1, "test");
469+
LOG("19 Hello msvc LOG without arguments\n");
470+
LOG("20 Hello msvc LOG with (%d)(%s) arguments\n", 1, "test");
471+
LOGLN("21 Hello msvc LOGLN without arguments\n");
472+
LOGLN("22 Hello msvc LOGLN with (%d)(%s) arguments\n", 1, "test");
473473
#endif
474474
}
475475

@@ -542,7 +542,7 @@ inline void log_dump_cmdline_impl(int argc, char **argv)
542542
buf << " " << argv[i];
543543
}
544544
}
545-
LOGLN("Cmd:%s", buf.str().c_str())
545+
LOGLN("Cmd:%s", buf.str().c_str());
546546
}
547547

548548
#define log_tostr(var) log_var_to_string_impl(var).c_str()
@@ -620,10 +620,10 @@ inline std::string log_var_to_string_impl(const std::vector<int> & var)
620620
#define LOGLN(...) // dummy stub
621621

622622
#undef LOG_TEE
623-
#define LOG_TEE(...) fprintf(stderr, __VA_ARGS__); // convert to normal fprintf
623+
#define LOG_TEE(...) fprintf(stderr, __VA_ARGS__) // convert to normal fprintf
624624

625625
#undef LOG_TEELN
626-
#define LOG_TEELN(...) fprintf(stderr, __VA_ARGS__); // convert to normal fprintf
626+
#define LOG_TEELN(...) fprintf(stderr, __VA_ARGS__) // convert to normal fprintf
627627

628628
#undef LOG_DISABLE
629629
#define LOG_DISABLE() // dummy stub

0 commit comments

Comments
 (0)