llama.cpp icon indicating copy to clipboard operation
llama.cpp copied to clipboard

Adding missing features of CMakeLists.txt & Refactoring

Open nusu-github opened this issue 1 year ago • 2 comments

We have come up with several changes to CMakeLists.txt that are expected to improve performance, compatibility, and maintainability, and we have drafted them.

Change list :

  1. remove NO from option names that have NO in the option name. Change LLAMA_NO_ACCELERATE to LLAMA_ACCELERATE to avoid "NOT NO" in if statements.
  2. make it possible to enable/disable SIMD extension instructions for non-Apple. I see no reason to make it Apple-only. It should be possible to enable/disable on all platforms.
  3. give priority to CMake functions. Use "add_compile_options" instead of writing directly to CMAKE_C_FLAGS to avoid double flags and compile errors due to typos.
  4. Add new options. LLAMA_STATIC_LINK, LLAMA_NATIVE, and LLAMA_LTO were added. LLAMA_STATIC_LINK is an option to enable static linking. This will be useful when compiling for Windows with MinGW and for future distribution. Also, LLAMA_NATIVE to add -march=native and LLAMA_LTO for link-time optimization will both be useful options for this project for speed.
  5. other changes. cmake_minimum_required was changed to 3.9 for LTO support. CMAKE_CXX_STANDARD was changed to c++11, the same as in the Makefile. I made utils build using add_library and link to it. Added Threads link since it was missing. Added option LLAMA_OPENBLAS. (Not tested).

We have confirmed that it works on MSVC, MSVC Clang GUN like (not clang-cl), MinGW, and Cygwin on Windows, but have not confirmed that it works on other platforms. Also, prioritizing CMake's features may sometimes sacrifice simplicity.

P.S. Sorry if there is any strange English. This is using automatic translation.

nusu-github avatar Mar 14 '23 12:03 nusu-github

Someone figure out with the Windows build is failing and merge

ggerganov avatar Mar 19 '23 18:03 ggerganov

LLAMA_LTO was changed so that it can be used outside of Release so that it can be built on Windows.

nusu-github avatar Mar 20 '23 07:03 nusu-github

What is the reasoning for changing C++11 standard to C++17 since the project does not use any of C++17 extensions? To my knowledge all compiler versions which support C++17 also support C++11, but not vice-versa.

anzz1 avatar Mar 21 '23 02:03 anzz1

@anzz1 we have been using c++17 features (string_view) since https://github.com/ggerganov/llama.cpp/commit/074bea2eb1f1349a0118239c4152914aecaa1be4

Green-Sky avatar Mar 21 '23 03:03 Green-Sky