[Docker] Add script to build llvm from source #13823
Conversation
|
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
|
@tvm-bot rerun |
|
@neildhickey what is the benefit of building from source if we are using the released version anyway? |
|
@mehrdadh Newer versions of gcc and clang introduce an option to outline atomic calls which means it will look for these atomic functions calls in glibc. The atomic calls are of the form __aarch64_ldadd4_acq_rel and others. With the version of gcc and glibc we use to build tvm, these functions aren't available, and the build fails when trying to link against prebuilt libraries which use these options. The released llvm packages for llvm-15 are built with these options enabled, so a workaround is to build llvm from source using the same version of the compiler we use to build tvm, thus alleviating the issue of the undefined function references. |
neildhickey
force-pushed
the
llvm_15_upgrade
branch
2 times, most recently
from
90c83e5 to
5033f5d
Compare
neildhickey
force-pushed
the
llvm_15_upgrade
branch
from
5033f5d to
cac77d7
Compare
| LLVM_VERSION_MAJOR=$1 | ||
|
|
||
| detect_llvm_version() { | ||
| curl -sL "https://api.github.com/repos/llvm/llvm-project/releases?per_page=100" | \ |
There was a problem hiding this comment.
Do we need to follow redirects here?
|
|
||
| set -e | ||
|
|
||
| LLVM_VERSION_MAJOR=$1 |
There was a problem hiding this comment.
Doesn't this introduce ambiguity as to which version of LLVM we've built with and make it harder to debug issues (we have to figure out which version of LLVM was live at the time of the build of the container)? Wouldn't it be better to fix it to a full version?
There was a problem hiding this comment.
I agree, and if we remove the $1 parameter, then should we also cleanup the detect_llvm_version function? What do you think?
There was a problem hiding this comment.
We could still use the parameter if we wanted to, similar to:
tvm/docker/Dockerfile.ci_adreno
Line 25 in 6c04ac5
Otherwise, yes, detect_llvm_version should be removed.
| unxz llvm-project-${LLVM_VERSION}.src.tar.xz | ||
| tar xf llvm-project-${LLVM_VERSION}.src.tar |
There was a problem hiding this comment.
Does this not work? Surprised tar doesn't sniff the format.
| unxz llvm-project-${LLVM_VERSION}.src.tar.xz | |
| tar xf llvm-project-${LLVM_VERSION}.src.tar | |
| tar xf llvm-project-${LLVM_VERSION}.src.tar.xz |
| ninja install | ||
| popd | ||
| popd | ||
| rm -rf llvm-${LLVM_VERSION}.src.tar.xz llvm-${LLVM_VERSION}.src.tar llvm-${LLVM_VERSION}.src |
There was a problem hiding this comment.
Can you use this cleanup pattern that should auto clean up these things on script failure?
https://github.com/apache/tvm/blob/main/docker/install/ubuntu_download_arm_compute_lib_binaries.sh#L46-L53
neildhickey
force-pushed
the
llvm_15_upgrade
branch
from
cac77d7 to
563dddc
Compare
docker/Dockerfile.ci_arm
Outdated
| COPY install/ubuntu_install_llvm.sh /install/ubuntu_install_llvm.sh | ||
| RUN bash /install/ubuntu_install_llvm.sh | ||
| COPY install/ubuntu_install_llvm_from_source.sh /install/ubuntu_install_llvm_from_source.sh | ||
| RUN bash /install/ubuntu_install_llvm_from_source.sh 15.0.7 |
There was a problem hiding this comment.
Can we add a hash parameter in here to verify the source tarball? Similar to:
https://github.com/apache/tvm/blob/main/docker/install/ubuntu_install_zephyr_sdk.sh#L45-L48
This allows us to use the latest llvm and make sure it builds with the same configuration as the rest of tvm Change-Id: I004ec89700498167c632d25f4fdf667922ba4943
Note: This will be undone once the docker images have been updated
neildhickey
force-pushed
the
llvm_15_upgrade
branch
from
563dddc to
1e6754c
Compare
This allows us to use the latest llvm and make sure it builds with the same configuration as the rest of tvm
Change-Id: I004ec89700498167c632d25f4fdf667922ba4943