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

[LLVM_full] Build version 16 #6777

Merged
merged 11 commits into from Oct 13, 2023
Merged

Conversation

giordano
Copy link
Member

@giordano giordano commented May 21, 2023

Just to start the ride, it'll likely have problems. CC: @gbaraldi

@giordano giordano added LLVM 🐉 julia 💜 ❤️ 💚 Builders and issues related to Julia and its dependencies labels May 21, 2023
@giordano giordano requested a review from vchuravy May 21, 2023 17:00
@vchuravy
Copy link
Member

Ambitious, but I like it xD.

I need to look at our patch list and rebase them appropriately.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this patch for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I have no clue why it fails to apply

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Especially because it works locally, but then it fails to apply the templates patch.

@giordano
Copy link
Member Author

giordano commented May 22, 2023

I still don't understand why this is failing to apply the libcxx patch, it works locally for me after having refreshed the patch, I really don't know what's going on here. Edit: it was assert build that was failing, because the bundled directory was pointing to the wrong version of LLVM 🤦

@giordano
Copy link
Member Author

Ok, let's start with the fun errors:

[22:53:41] /opt/x86_64-linux-musl/x86_64-linux-musl/include/c++/7.1.0/bits/stl_construct.h:75:7: error: use of deleted function ‘std::unique_ptr<_Tp, _Dp>::unique_ptr(const std::unique_ptr<_Tp, _Dp>&) [with _Tp = mlir::pdll::ast::Diagnostic; _Dp = std::default_delete<mlir::pdll::ast::Diagnostic>]’
[22:53:41]      { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }
[22:53:41]        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[22:53:41] In file included from /opt/x86_64-linux-musl/x86_64-linux-musl/include/c++/7.1.0/memory:80:0,
[22:53:41]                  from /workspace/srcdir/llvm-project/llvm/include/llvm/Support/Casting.h:20,
[22:53:41]                  from /workspace/srcdir/llvm-project/mlir/include/mlir/Support/LLVM.h:24,
[22:53:41]                  from /workspace/srcdir/llvm-project/mlir/include/mlir/Tools/PDLL/AST/Diagnostic.h:15,
[22:53:41]                  from /workspace/srcdir/llvm-project/mlir/lib/Tools/PDLL/AST/Diagnostic.cpp:9:
[22:53:41] /opt/x86_64-linux-musl/x86_64-linux-musl/include/c++/7.1.0/bits/unique_ptr.h:388:7: note: declared here
[22:53:41]        unique_ptr(const unique_ptr&) = delete;
[22:53:41]        ^~~~~~~~~~

@gbaraldi
Copy link
Contributor

And here we go

@vchuravy
Copy link
Member

I think we now need C++17?

@giordano
Copy link
Member Author

https://llvm.org/docs/GettingStarted.html#software says gcc 7 is the minimum requirement and we're using that already

preferred_gcc_version=v"7", preferred_llvm_version=v"12", julia_compat="1.10")
Happy to try with a newer version but one would hope the requirements are accurate.

@gbaraldi
Copy link
Contributor

Should we rebase it to 16.0.4?

@gbaraldi
Copy link
Contributor

It seems GCC8 did make a difference.

@giordano
Copy link
Member Author

giordano commented May 23, 2023

But now do we want to talk about

[21:50:27] /workspace/srcdir/llvm-project/mlir/include/mlir/IR/SubElementInterfaces.h:407:10: fatal error: mlir/IR/SubElementAttrInterfaces.h.inc: No such file or directory
[21:50:27]  #include "mlir/IR/SubElementAttrInterfaces.h.inc"
[21:50:27]           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[21:50:27] compilation terminated.

https://buildkite.com/julialang/yggdrasil/builds/3136#01884a92-6e09-4021-bbfa-ca61d3742a74 which fails on armv7l-linux-gnueabihf-cxx11 but not armv7l-linux-gnueabihf-cxx03? This smells a lot of race condition.

Edit: restarting the job got past that error, which would confirm that's a race condition.

@giordano
Copy link
Member Author

And

[22:41:35] ninja: job failed: /opt/bin/x86_64-linux-gnu-libgfortran5-cxx11/x86_64-linux-gnu-g++ --sysroot=/opt/x86_64-linux-gnu/x86_64-linux-gnu/sys-root/ -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=1 -DMLIR_ROCM_CONVERSIONS_ENABLED=1 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Dmlir_c_runner_utils_EXPORTS -I/workspace/build/tools/mlir/lib/ExecutionEngine -I/workspace/srcdir/llvm-project/mlir/lib/ExecutionEngine -I/workspace/build/include -I/workspace/srcdir/llvm-project/llvm/include -I/workspace/srcdir/llvm-project/mlir/include -I/workspace/build/tools/mlir/include -fno-gnu-unique -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -Wno-misleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -fPIC  -fno-exceptions -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/mlir/lib/ExecutionEngine/CMakeFiles/mlir_c_runner_utils.dir/CRunnerUtils.cpp.o -MF tools/mlir/lib/ExecutionEngine/CMakeFiles/mlir_c_runner_utils.dir/CRunnerUtils.cpp.o.d -o tools/mlir/lib/ExecutionEngine/CMakeFiles/mlir_c_runner_utils.dir/CRunnerUtils.cpp.o -c /workspace/srcdir/llvm-project/mlir/lib/ExecutionEngine/CRunnerUtils.cpp
[22:41:35] /workspace/srcdir/llvm-project/mlir/lib/ExecutionEngine/CRunnerUtils.cpp: In function ‘void* mlirAlignedAlloc(uint64_t, uint64_t)’:
[22:41:35] /workspace/srcdir/llvm-project/mlir/lib/ExecutionEngine/CRunnerUtils.cpp:147:10: error: ‘aligned_alloc’ was not declared in this scope
[22:41:35]    return aligned_alloc(alignment, size);
[22:41:35]           ^~~~~~~~~~~~~
[22:41:35] /workspace/srcdir/llvm-project/mlir/lib/ExecutionEngine/CRunnerUtils.cpp:147:10: note: suggested alternative: ‘mlirAlignedAlloc’
[22:41:35]    return aligned_alloc(alignment, size);
[22:41:35]           ^~~~~~~~~~~~~
[22:41:35]           mlirAlignedAll

According to https://en.cppreference.com/w/cpp/memory/c/aligned_alloc aligned_alloc is defined in cstdlib, which as far as I can tell is included: https://github.com/llvm/llvm-project/blob/b2e6b7354452c10ed6f38958253fd76aca0877de/mlir/lib/ExecutionEngine/CRunnerUtils.cpp#L32

@gbaraldi
Copy link
Contributor

@giordano
Copy link
Member Author

Right

% julia --compile=min -e 'using BinaryBuilderBase; BinaryBuilderBase.runshell(Platform("x86_64", "linux"); preferred_gcc_version=v"8")'    
sandbox:${WORKSPACE} # echo '#include <cstdlib>' | c++ -x c++ -E -dM - | grep GLIBCXX_HAVE_AL
sandbox:${WORKSPACE} # 
logout
% julia --compile=min -e 'using BinaryBuilderBase; BinaryBuilderBase.runshell(Platform("aarch64", "linux"); preferred_gcc_version=v"8")'    
sandbox:${WORKSPACE} # echo '#include <cstdlib>' | c++ -x c++ -E -dM - | grep GLIBCXX_HAVE_AL
#define _GLIBCXX_HAVE_ALIGNED_ALLOC 1
sandbox:${WORKSPACE} # 
logout
[mose@dream]/tmp/tmp.PUce7Bs0Uk% julia --compile=min -e 'using BinaryBuilderBase; BinaryBuilderBase.runshell(Platform("powerpc64le", "linux"); preferred_gcc_version=v"8")'
sandbox:${WORKSPACE} # echo '#include <cstdlib>' | c++ -x c++ -E -dM - | grep GLIBCXX_HAVE_AL
#define _GLIBCXX_HAVE_ALIGNED_ALLOC 1
sandbox:${WORKSPACE} # 

Sigh.

@giordano
Copy link
Member Author

https://linux.die.net/man/3/aligned_alloc

Versions

[...]

The function aligned_alloc() was added to glibc in version 2.16.

🪦

@gbaraldi
Copy link
Contributor

Maybe we can add a patch, though I'm worried it might use other C11 features

@giordano giordano force-pushed the mg/llvm-16 branch 3 times, most recently from 792c286 to 48a3af8 Compare May 27, 2023 22:26
@giordano
Copy link
Member Author

giordano commented May 27, 2023

Failure to build for arm32 32-bit platforms is due to llvm/llvm-project#61581

@gbaraldi
Copy link
Contributor

gbaraldi commented Aug 7, 2023

simdjson/simdjson#578 the tie issue.

@gbaraldi
Copy link
Contributor

gbaraldi commented Aug 7, 2023

This makes it work for me. But it seems to bump it to 11.3 because of the libc++ bug. And we use the system one IIRC.

diff --git a/L/LLVM/common.jl b/L/LLVM/common.jl
index 548d5a24..c5f9e343 100644
--- a/L/LLVM/common.jl
+++ b/L/LLVM/common.jl
@@ -23,7 +23,7 @@ const buildscript = raw"""
 # We want to exit the program if errors occur.
 set -o errexit

-if [[ ("${target}" == x86_64-apple-darwin*) && ("${LLVM_MAJ_VER}" -ge "15") ]]; then
+if [[ ("${target}" == x86_64-apple-darwin*) && ("${LLVM_MAJ_VER}" -eq "15") ]]; then
     # LLVM 15 requires macOS SDK 10.14, see
     # <https://github.com/JuliaPackaging/Yggdrasil/pull/5592#issuecomment-1309525112> and
     # references therein.
@@ -35,6 +35,16 @@ if [[ ("${target}" == x86_64-apple-darwin*) && ("${LLVM_MAJ_VER}" -ge "15") ]];
     popd
 fi

+if [[ ("${target}" == *-apple-darwin*) && ("${LLVM_MAJ_VER}" -ge "16") ]]; then
+    # LLVM 16 exposes a libc++ that needs 11.3
+    pushd $WORKSPACE/srcdir/MacOSX11.*.sdk
+    rm -rf /opt/${target}/${target}/sys-root/System
+    cp -ra usr/* "/opt/${target}/${target}/sys-root/usr/."
+    cp -ra System "/opt/${target}/${target}/sys-root/."
+    export MACOSX_DEPLOYMENT_TARGET=11.3
+    popd
+fi
+
 if [[ ${bb_full_target} == *-sanitize+memory* ]]; then
     # Install msan runtime (for clang)
     cp -rL ${prefix}/lib/linux/* /opt/x86_64-linux-musl/lib/clang/*/lib/linux/
@@ -592,6 +602,10 @@ function configure_build(ARGS, version; experimental_platforms=false, assert=fal
                   "0f03869f72df8705b832910517b47dd5b79eb4e160512602f593ed243b28715f"))
     end
     if version >= v"16"
+        push!(sources,
+              ArchiveSource(
+                  "https://github.com/alexey-lysiuk/macos-sdk/releases/download/11.3/MacOSX11.3.tar.bz2",
+                  "d6604578f4ee3090d1c3efce1e5c336ecfd7be345d046c729189d631ea3b8ec6"))
         push!(dependencies,
               # On Intel Linux platforms we use glibc 2.12, but building LLVM 16
               # requires glibc 2.16+.

@giordano
Copy link
Member Author

giordano commented Aug 7, 2023

For future reference, the errors we're facing with macOS are

[23:03:31] /workspace/srcdir/llvm-project/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:5492:13: error: no viable constructor or deduction guide for deduction of template arguments of 'tuple'
[23:03:31]           : std::tuple(HSAMD::AssemblerDirectiveBegin,
[23:03:31]             ^
[...]
[23:03:31] /workspace/srcdir/llvm-project/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:5488:60: error: no viable overloaded '='
[23:03:31]   std::tie(AssemblerDirectiveBegin, AssemblerDirectiveEnd) =
[23:03:31]   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^

@giordano
Copy link
Member Author

giordano commented Aug 7, 2023

As discussed on Slack, we may want to disable AMDGPU target on platforms where we trigger the libc++ bug, that is macOS (FreeBSD seems to be good?)

@gbaraldi
Copy link
Contributor

gbaraldi commented Aug 9, 2023

Why is this still trying to build AMDGPU 😠

L/LLVM/common.jl Outdated
CMAKE_CXX_FLAGS+=(-fno-gnu-unique)
fi

# LLVM 16 requires `align_alloc`, make it available for Intel Linux platforms
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hack should probably be removed now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this solved by the glibc bump?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should have been, yes

@vtjnash
Copy link
Member

vtjnash commented Oct 12, 2023

we may want to disable AMDGPU target on platforms where we trigger the libc++ bug, that is macOS

Also should disable NVIDIA there, if we don't already, since drivers for that haven't existed for many years

@gbaraldi
Copy link
Contributor

@giordano do we have a way to only rebuild macos, or do we just let ccache do it's thing?

@giordano
Copy link
Member Author

No, it's only all or nothing. But a hot ccache rebuild should take relatively little, in the range of 5 minutes for the build proper (audit is much longer though)

@vchuravy vchuravy marked this pull request as ready for review October 13, 2023 17:20
@vchuravy vchuravy enabled auto-merge (squash) October 13, 2023 17:21
@vchuravy vchuravy merged commit 2dc6d14 into JuliaPackaging:master Oct 13, 2023
29 checks passed
amontoison pushed a commit to amontoison/Yggdrasil that referenced this pull request Nov 27, 2023
Co-authored-by: Gabriel Baraldi <baraldigabriel@gmail.com>
Co-authored-by: Valentin Churavy <vchuravy@users.noreply.github.com>
@giordano giordano deleted the mg/llvm-16 branch March 9, 2024 19:41
@Zentrik Zentrik mentioned this pull request Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
julia 💜 ❤️ 💚 Builders and issues related to Julia and its dependencies LLVM 🐉
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants