ldc-developers / ldc2.snap

Snap package definition for LDC, the LLVM-based D compiler
11 stars 4 forks source link

Update to LLVM 9.0.1 #88

Closed WebDrake closed 4 years ago

WebDrake commented 4 years ago

This update involves switching repo to LDC's direct fork of the LLVM upstream: https://github.com/ldc-developers/llvm-project/tree/ldc-v9.0.1

Thanks to the absence of a base-directory-level CMakeLists.txt we have to add a manual override-build to run cmake on the llvm directory.

CMake build flags have also been updated to bring them in line with the choices in the LDC project CI. Note that this includes updates to the supported target architectures.

WebDrake commented 4 years ago

Unfortunately the version as written falls over when trying to build LLVM:

CMake Error at cmake/modules/AddLLVM.cmake:1408 (add_dependencies):
  The dependency target "clang" of target "check-all" does not exist.
Call Stack (most recent call first):
  CMakeLists.txt:977 (add_lit_target)

CMake Error at cmake/modules/AddLLVM.cmake:1408 (add_dependencies):
  The dependency target "clang" of target "check-llvm-spirv" does not exist.
Call Stack (most recent call first):
  cmake/modules/AddLLVM.cmake:1429 (add_lit_target)
  projects/SPIRV-LLVM-Translator/test/CMakeLists.txt:69 (add_lit_testsuite)

-- Generating done
CMake Warning:
  Manually-specified variables were not used by the project:

    COMPILER_RT_INCLUDE_TESTS
    COMPILER_RT_USE_LIBCXX

I'd assumed the -DCMAKE_CXX_FLAGS=-static-libstdc++ and -DCOMPILER_RT_USE_LIBCXX=OFF would avoid that problem, but seems not -- any suggestions for what could be wrong? (I can't see obvious differences with the LDC CI setup.)

WebDrake commented 4 years ago

Ah, that fixes things, thanks!

I've also added the missing ninja and ninja install commands.

diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index d8d0631..a040690 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -109,13 +109,15 @@ parts:
         -DCOMPILER_RT_INCLUDE_TESTS=OFF \
         -DCOMPILER_RT_USE_LIBCXX=OFF \
         -DLLVM_BINUTILS_INCDIR=/usr/include \
-        -DLLVM_TARGETS_TO_BUILD=AArch64;ARM;Mips;MSP430;NVPTX;PowerPC;RISCV;WebAssembly;X86 \
-        -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=AVR \
+        -DLLVM_TARGETS_TO_BUILD='AArch64;ARM;Mips;MSP430;NVPTX;PowerPC;RISCV;WebAssembly;X86' \
+        -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD='AVR' \
         -DLLVM_ENABLE_PROJECTS='compiler-rt;lld' \
         -DLLVM_ENABLE_TERMINFO=OFF \
         -DLLVM_ENABLE_LIBEDIT=OFF \
         -DLLVM_INCLUDE_TESTS=OFF \
         -GNinja
+      ninja
+      DESTDIR=../install ninja install
     stage:
     - -*
     prime:
kinke commented 4 years ago

According to the yaml reference, you should be able to specify the src.tar.xz archive as source instead of the repo. That structure hasn't changed (only contains the required projects, no llvm subdir), so there'd be no need to override the CMake build. Edit: You also won't need to set LLVM_ENABLE_PROJECTS and COMPILER_RT_USE_LIBCXX that way.

I'm also not sure you really need -static-libstdc++ for these snap builds; I only use it for the official builds to increase portability across distros.

WebDrake commented 4 years ago

Thanks for the advice -- I'll give that a go as a follow-up PR to simplify things. This seems to be working for now so I'll take it in as a way of moving forward.