-
Notifications
You must be signed in to change notification settings - Fork 9.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build : enable more non-default compiler warnings #3200
Conversation
a322a7a
to
578a2ee
Compare
We don't know for sure whether nvcc calls gcc or clang.
95ade21
to
2657f7b
Compare
did you check which version of the compilers support the flags? |
It looks like -Wextra-semi was only added in GCC 8.1. master compiles fine with GCC 5. So I'll have to add some version checks to the build scripts. |
29cfecc
to
426ef6e
Compare
I added some compiler version checks. I've tested with several compilers, including LLVM clang 3.7.0, Apple clang 8.0.0, and gcc 5.5.0. |
7e0e179
to
d38d59c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should merge after resolving conflicts
With CUDA builds, I think the
|
I also see that when building with cmake with CUDA enabled. The Line 484 in 40e07a6
|
I could try and change the structure of the flags to match the Makefile better. I wasn't really thinking about CUDA when I was porting my changes to CMake. |
…example * 'master' of github.com:ggerganov/llama.cpp: ggml-cuda : perform cublas mat mul of quantized types as f16 (ggerganov#3412) llama.cpp : add documentation about rope_freq_base and scale values (ggerganov#3401) train : fix KQ_pos allocation (ggerganov#3392) llama : quantize up to 31% faster on Linux and Windows with mmap (ggerganov#3206) readme : update hot topics + model links (ggerganov#3399) readme : add link to grammars app (ggerganov#3388) swift : fix build on xcode 15 (ggerganov#3387) build : enable more non-default compiler warnings (ggerganov#3200) ggml_tensor: update the structure comments. (ggerganov#3283) ggml : release the requested thread pool resource (ggerganov#3292) llama.cpp : split llama_context_params into model and context params (ggerganov#3301) ci : multithreaded builds (ggerganov#3311) train : finetune LORA (ggerganov#2632) gguf : basic type checking in gguf_get_* (ggerganov#3346) gguf : make token scores and types optional (ggerganov#3347) ci : disable freeBSD builds due to lack of VMs (ggerganov#3381) llama : custom attention mask + parallel decoding + no context swaps (ggerganov#3228) docs : mark code as Bash (ggerganov#3375) readme : add Mistral AI release 0.1 (ggerganov#3362) ggml-cuda : perform cublas fp16 matrix multiplication as fp16 (ggerganov#3370)
I compiled llama.cpp with clang's -Weverything option, and found some warning flags that I think we should use.
The new warnings are
-Wmissing-noreturn -Wextra-semi
for g++ and clang++,-Wunreachable-code-break -Wunreachable-code-return
for clang and clang++, and-Wrange-loop-bind-reference
for clang++.I made a new
GGML_UNREACHABLE()
macro to cover some of the cases where it should be explicit that code is unreachable. It's basicallyassert(false)
, but with the added benefit that it will compile to__builtin_unreachable()
when assertions are disabled, both to enable compiler optimizations and to make sure no -Wreturn-type warnings appear because of the lack of a return statement at the end of a function.Other included changes: