Skip to content

Commit

Permalink
[build] Some improvements to the LLVM build system (JuliaLang#55354)
Browse files Browse the repository at this point in the history
### Elaboration of the issue

After JuliaLang#55180 we implicitly require an LLVM built with Zlib support, but
compiling Julia with `make USE_BINARYBUILDER_LLVM=0` builds an LLVM
without Zlib support, despite the fact we attempt to request it at

https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97
This was first identified in JuliaLang#55337.

### Explanation of how configuration of LLVM failed

`ZLIB_LIBRARY` must be the path to the zlib library, but we currently
set it to the libdir where the library is installed (introduced in
JuliaLang#42622):

https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97
which is wrong. However, CMake is actually able to find Zlib correctly,
but then the check at
https://github.com/llvm/llvm-project/blob/46425b8d0fac3c529aa4a716d19abd7032e452f3/llvm/cmake/config-ix.cmake#L139-L141
uses the value of `ZLIB_LIBRARY` to list the Zlib to link for the test,
but being `ZLIB_LIBRARY` a directory, CMake doesn't see any valid Zlib
and thus tries to run the test without linking any Zlib, and the test
silently fails (they're silent only when `LLVM_ENABLE_ZLIB=ON`),
resulting in no usable Zlib available, even if found.

### Proposed solution

`ZLIB_ROOT` is the only [hint recommended by the CMake module
`FindZLIB`](https://cmake.org/cmake/help/latest/module/FindZLIB.html#hints).
This PR replaces a broken `ZLIB_LIBRARY` with an appropriate
`ZLIB_ROOT`. Also, we set `LLVM_ENABLE_ZLIB=FORCE_ON` which is the only
way to make CMake fail loudly if no usable Zlib is available, and avoid
going on with a non-usable build.

### Other comments

I confirm this fixes JuliaLang#55337 for me, it should likely address
JuliaCI/julia-buildkite#373 as well.

Also, options `COMPILER_RT_ENABLE_IOS`, `COMPILER_RT_ENABLE_WATCHOS`,
`COMPILER_RT_ENABLE_TVOS`, and `HAVE_HISTEDIT_H` don't exist anymore,
and they are removed.
  • Loading branch information
giordano authored and lazarusA committed Aug 17, 2024
1 parent da2122c commit 7ed7b40
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 4 deletions.
2 changes: 1 addition & 1 deletion deps/BOLT.mk
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ LLVM_LDFLAGS += $(LDFLAGS)
LLVM_LDFLAGS += $(LLVM_LDFLAGS)
LLVM_CMAKE += -DLLVM_TARGETS_TO_BUILD:STRING=host -DCMAKE_BUILD_TYPE=Release
LLVM_CMAKE += -DLLVM_ENABLE_LIBXML2=OFF -DLLVM_HOST_TRIPLE="$(or $(XC_HOST),$(BUILD_MACHINE))"
LLVM_CMAKE += -DLLVM_ENABLE_ZLIB=ON -DZLIB_LIBRARY="$(build_prefix)/lib"
LLVM_CMAKE += -DLLVM_ENABLE_ZLIB=FORCE_ON -DZLIB_ROOT="$(build_prefix)"

LLVM_CMAKE += -DLLVM_BINDINGS_LIST="" -DLLVM_ENABLE_BINDINGS=OFF -DLLVM_INCLUDE_DOCS=Off -DLLVM_ENABLE_TERMINFO=Off -DHAVE_LIBEDIT=Off

Expand Down
5 changes: 2 additions & 3 deletions deps/llvm.mk
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,15 @@ LLVM_LDFLAGS += $(BOLT_LDFLAGS)
LLVM_CMAKE += -DLLVM_TARGETS_TO_BUILD:STRING="$(LLVM_TARGETS)" -DCMAKE_BUILD_TYPE="$(LLVM_CMAKE_BUILDTYPE)"
LLVM_CMAKE += -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD:STRING="$(LLVM_EXPERIMENTAL_TARGETS)"
LLVM_CMAKE += -DLLVM_ENABLE_LIBXML2=OFF -DLLVM_HOST_TRIPLE="$(or $(XC_HOST),$(BUILD_MACHINE))"
LLVM_CMAKE += -DLLVM_ENABLE_ZLIB=ON -DZLIB_LIBRARY="$(build_prefix)/lib"
LLVM_CMAKE += -DLLVM_ENABLE_ZLIB=FORCE_ON -DZLIB_ROOT="$(build_prefix)"
LLVM_CMAKE += -DLLVM_ENABLE_ZSTD=OFF
LLVM_CMAKE += -DCOMPILER_RT_ENABLE_IOS=OFF -DCOMPILER_RT_ENABLE_WATCHOS=OFF -DCOMPILER_RT_ENABLE_TVOS=OFF
ifeq ($(USE_POLLY_ACC),1)
LLVM_CMAKE += -DPOLLY_ENABLE_GPGPU_CODEGEN=ON
endif
LLVM_CMAKE += -DLLVM_TOOLS_INSTALL_DIR=$(call rel_path,$(build_prefix),$(build_depsbindir))
LLVM_CMAKE += -DLLVM_UTILS_INSTALL_DIR=$(call rel_path,$(build_prefix),$(build_depsbindir))
LLVM_CMAKE += -DLLVM_INCLUDE_UTILS=ON -DLLVM_INSTALL_UTILS=ON
LLVM_CMAKE += -DLLVM_BINDINGS_LIST="" -DLLVM_ENABLE_BINDINGS=OFF -DLLVM_INCLUDE_DOCS=Off -DLLVM_ENABLE_TERMINFO=Off -DHAVE_HISTEDIT_H=Off -DHAVE_LIBEDIT=Off
LLVM_CMAKE += -DLLVM_BINDINGS_LIST="" -DLLVM_ENABLE_BINDINGS=OFF -DLLVM_INCLUDE_DOCS=Off -DLLVM_ENABLE_TERMINFO=Off -DHAVE_LIBEDIT=Off
ifeq ($(LLVM_ASSERTIONS), 1)
LLVM_CMAKE += -DLLVM_ENABLE_ASSERTIONS:BOOL=ON
endif # LLVM_ASSERTIONS
Expand Down

0 comments on commit 7ed7b40

Please sign in to comment.