Skip to content
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

Feature: A unified configure (and build) process for llama.cpp #5890

Closed
cebtenzzre opened this issue Mar 5, 2024 · 10 comments
Closed

Feature: A unified configure (and build) process for llama.cpp #5890

cebtenzzre opened this issue Mar 5, 2024 · 10 comments
Labels
enhancement New feature or request

Comments

@cebtenzzre
Copy link
Collaborator

An Example, For Context

The best solution for the issue fixed in #5889 is to conditionally define a _mm256_set_m128i fallback. This would prevent the mistake of using _mm256_set_m128i instead of MM256_SET_M128I, breaking the gcc 7 build.

Of course, _mm256_set_m128i is not a macro, so we can't directly test for it in the preprocessor. We can use a compiler version check, but this is a maintenance burden, because every supported compiler (at least gcc, clang, icc, and msvc) should be handled in each of these cases.

The elegant solution is a configure-time check that will set e.g. #define HAVE_MM256_SET_M128I 1 in our config.h, so that the fallback will be defined when HAVE_MM256_SET_M128I is not defined. Wait... what configure time? What config.h?

The Solution?

Adding these kinds of steps to the build is very high friction right now because they have to be written in three separate languages - CMake, the Makefile, and build.zig.

My question is simple - does it have to be this way? Is there a future in which llama.cpp can standardize on CMake or Meson as its sole build system, with a straightforward way to add conftests in a single location? I would be happy to contribute the necessary changes if we decide this is feasible. (Zig is not a limitation is far as I know - you can use zig cc as a toolchain with CMake or Meson. We obviously cannot standardize on the Makefile without losing support for native MSVC builds on Windows.)

Otherwise, it seems like we will always have to hack around the lack of a robust configure step and ignore the problem.

tl;dr I want to make a PR that removes the Makefile. Change my mind.

cc @ggerganov @slaren @LostRuins

@cebtenzzre cebtenzzre added the enhancement New feature or request label Mar 5, 2024
@slaren
Copy link
Collaborator

slaren commented Mar 5, 2024

I agree that the Makefile has outlived its usefulness, it has grown too complex to continue maintaining it alongside the other build systems. Standardizing on cmake seems like a good idea to me.

Additionally, I think it would be good to separate the build of ggml and llama.cpp, so that we can use the same build script for ggml in all the repositories that use it. Currently, we have to duplicate the build scripts on every project, and this is not so simple any more with all the new backends.

@cebtenzzre
Copy link
Collaborator Author

Additionally, I think it would be good to separate the build of ggml and llama.cpp,

I agree. A submodule would probably make it too difficult to contribute a change to llama.cpp that touches ggml as well, but maybe it could live in a subfolder? Then we would be able to leverage the add_subdirectory command of CMake.

@LostRuins
Copy link
Collaborator

LostRuins commented Mar 6, 2024

I do agree that cmake makes aggregating dependencies far easier, and reduces the maintenance burden necessary to ensure the makefile functions appropriately across all platforms. However, my main concern would be the added friction of having to get cmake itself setup and working for your build environment.

Could be wrong, but off the top of my head, the build essentials for WSL and Termux don't have cmake available, nor is it provided with the w64devkit that I use on windows. Whereas support for the makefile is almost universally available. Yes, most of the time you'd be able to get it ready-to-use from your package manager and just install it, but it's still an additional requirement to setup.

But ultimately, I am fine with either approach. I personally use the makefile and will continue to do so for my own personal uses.

@ggerganov
Copy link
Owner

I'm actually happy with the Makefile and think it still has value. I don't see a pressing issue to change the build system - it works pretty well IMO. It could be useful to reuse ggml build scripts, but at the same time I don't think it is so complex that it needs to be modified. If we start hearing overwhelming opinion from 3rd party projects that change is necessary, then we can consider again

@slaren
Copy link
Collaborator

slaren commented Mar 6, 2024

I don't think it works well at all, keeping the Makefile build working properly has been a struggle every time there are changes to the build. Some things still don't work properly on some systems, the flags passed to the compilers are not the same, there is no support for building with Kompute or SYCL. ggml does not have support for building with Vulkan, SYCL or Kompute at all, we just ship the source files in the repository and kind of tell users to figure it themselves, and this is because we don't have a unified build system.

@zsogitbe
Copy link

zsogitbe commented Mar 6, 2024

I wish everybody would work with Visual Studio and that we would just double click on the solution file, then select the configuration (GPU, CPU,...) and then hit compile and ready! All of these complicated build systems are just the waste of time (for me).

@cebtenzzre
Copy link
Collaborator Author

LostRuins: Could be wrong, but off the top of my head, the build essentials for WSL and Termux don't have cmake available, nor is it provided with the w64devkit that I use on windows. Whereas support for the makefile is almost universally available. Yes, most of the time you'd be able to get it ready-to-use from your package manager and just install it, but it's still an additional requirement to setup.

I don't think the hurdle of running a single command (or installer) before the first time you build llama.cpp is worth maintaining two completely separate sets of build scripts (the Makefile cannot be used for native Windows builds):

  • Ubuntu, Debian, Termux: apt install cmake
  • Arch Linux, MSYS2: pacman -S cmake
  • Homebrew: brew install cmake
  • Any system with python installed: pip install cmake
  • cmake package on MacPorts, Cygwin, Chocolatey, and Winget
  • Any other platform (e.g. native Windows/macOS): https://cmake.org/download/

w64devkit can run the native Windows version of CMake, although it will use the Visual Studio generator by default, which defeats the purpose. With some extra arguments you can tell it to generate Makefiles, but we should probably just stop recommending w64devkit - MSYS2 is much more flexible.

ggerganov: I'm actually happy with the Makefile and think it still has value. I don't see a pressing issue to change the build system - it works pretty well IMO. It could be useful to reuse ggml build scripts, but at the same time I don't think it is so complex that it needs to be modified. If we start hearing overwhelming opinion from 3rd party projects that change is necessary, then we can consider again

I'd be okay with keeping the Makefile on the condition that I can improve and modify the CMake build with complete disregard for whether I am breaking the Makefile build. I would like to actually start using features of CMake like check_function_exists without having to also implement them from scratch with pure shell commands.

zsogitbe: I wish everybody would work with Visual Studio and that we would just double click on the solution file, then select the configuration (GPU, CPU,...) and then hit compile and ready! All of these complicated build systems are just the waste of time (for me).

Visual Studio has built-in CMake integration, but I think the most straightforward graphical way to build llama.cpp on Windows is to use the CMake GUI to select a configuration and generate a Visual Studio solution.

@MarcusDunn
Copy link
Contributor

For what it is worth, I am in favor of keeping the Makefile. I use it as documentation to implement the build script for the rust bindings. This allows me to have no build time dependencies beyond a c and c++ compiler; in addition, having never authored anything at all complex in CMake, I find makefiles to be much more readable.

@ggerganov
Copy link
Owner

@cebtenzzre If you want to improve the CMake build you can do so. I don't see how it would break the Makefile. If you mean that CMake will support more environments compared to the Makefile, then I don't consider this "breakage" - this has never been a requirement. Anyway, in case I'm not seeing it, the changes should not break the Makefile

But these little annoyances with _mm256_set_m128i and sys_getcpu do not warrant significant efforts - we can workaround those edge cases easily. IMO, the focus if any should be to reuse ggml build scripts

In any case, the Makefile will stay. It does not have to support all environments and backends and I don't think we win anything from removing it. It's the simplest build one can get on MacOS and Linux and that's already good enough reason to have it. If it's hard to keep CUDA up-to-date, we can drop CUDA support from the Makefile and/or we can no longer recommend it as a build option to avoid receiving related issue

@cebtenzzre
Copy link
Collaborator Author

cebtenzzre commented Mar 6, 2024

MarcusDunn: This allows me to have no build time dependencies beyond a c and c++ compiler

There is a cmake crate available via cargo, surely it's better to use that than to maintain your own build script? https://crates.io/crates/cmake

ggerganov: If you want to improve the CMake build you can do so. I don't see how it would break the Makefile.

The idea was to be able to introduce a config.h (configured from a config.h.in template) to improve portability, which is produced in part from the result of various configure-time tests (often involving running the compiler). In theory this would extend to much more than just these recent examples. This would necessarily require changes to both the code and the build scripts, and the build scripts would have to maintain feature parity for this to work. But I am not interested in authoring the changes that would be required to the Makefile or to build.zig, and frankly I am tired of helping to maintain two main build systems in parallel, I would rather just focus on one.

ggerganov: In any case, the Makefile will stay.

Okay, I will close this issue then. I'm not satisfied with the current situation but I will have to accept it.

@cebtenzzre cebtenzzre closed this as not planned Won't fix, can't repro, duplicate, stale Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants