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

Add RISC-V + all default targets to LLVM build #788

Merged
merged 3 commits into from
Nov 23, 2021

Conversation

seibert
Copy link
Contributor

@seibert seibert commented Nov 16, 2021

There is growing interest in using llvmlite as a cross-compiler to target RISC-V. (See #785 and #775) This adds the RISC-V target as well as all the default targets to both the llvmdev package build and the llvmdev_manylinux2014 package build for all platforms. The increase in package size with this target should be ~30%.

(Note this is not adding RISC-V as a supported host platform for llvmlite, as that will require greater availability of RISC-V hardware running Linux, as well as a reasonable ecosystem of Python packages on that platform.)

Edit: Originally this PR only added RISC-V (at a size cost of 1.5%) but further discussion in #785 made it clear that we could add all the standard targets too. Note that starting with LLVM 13, the standard targets include RISC-V and WebAssembly, so the explicit target selection in our config can be dropped whenever we upgrade to LLVM 13.

@seibert seibert changed the title Add RISCV target to LLVM build Add RISC-V + all default targets to LLVM build Nov 16, 2021
@esc
Copy link
Member

esc commented Nov 17, 2021

I am running this through the anaconda internal build farm as llvmdev_18.

Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for the patch. In principle this change looks fine, I think the build farm run will give us more confidence. I also wonder if llvm-config --targets-built should be checked or printed as part of the conda build test phase?

@@ -5,7 +5,8 @@
set -x

# allow setting the targets to build as an environment variable
LLVM_TARGETS_TO_BUILD=${LLVM_TARGETS_TO_BUILD:-"host;AMDGPU;NVPTX"}
# default is LLVM 11 default architectures + RISCV. Can remove this entire option in LLVM 13
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, checked.

Copy link
Member

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

This works locally for me and enabled #775 to pass its tests.

@esc
Copy link
Member

esc commented Nov 18, 2021

I ran this through the anaconda internal build farm and received:

Screen Shot 2021-11-18 at 22 27 10

@esc
Copy link
Member

esc commented Nov 18, 2021

INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/vcp-decl.ll (10762 of 10779)
INFO - Traceback (most recent call last):
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/vcp-no-this.ll (10763 of 10779)
INFO - File "/opt/conda/bin/conda-build", line 11, in <module>
INFO - sys.exit(main())
INFO - File "/opt/conda/lib/python3.7/site-packages/conda_build/cli/main_build.py", line 456, in main
INFO - execute(sys.argv[1:])
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/vcp-too-wide-ints.ll (10764 of 10779)
INFO - File "/opt/conda/lib/python3.7/site-packages/conda_build/cli/main_build.py", line 447, in execute
INFO - PASS: LLVM :: Transforms/VectorCombine/X86/extract-binop.ll (10765 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/vcp-non-constant-arg.ll (10766 of 10779)
INFO - verify=args.verify, variants=args.variants)
INFO - File "/opt/conda/lib/python3.7/site-packages/conda_build/api.py", line 208, in build
INFO - notest=notest, need_source_download=need_source_download, variants=variants)
INFO - File "/opt/conda/lib/python3.7/site-packages/conda_build/build.py", line 2311, in build_tree
INFO - notest=notest,
INFO - File "/opt/conda/lib/python3.7/site-packages/conda_build/build.py", line 1477, in build
INFO - cwd=src_dir, stats=build_stats)
INFO - File "/opt/conda/lib/python3.7/site-packages/conda_build/utils.py", line 374, in check_call_env
INFO - return _func_defaulting_env_to_os_environ('call', *popenargs, **kwargs)
INFO - File "/opt/conda/lib/python3.7/site-packages/conda_build/utils.py", line 354, in _func_defaulting_env_to_os_environ
INFO - raise subprocess.CalledProcessError(proc.returncode, _args)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/vcp-uses-this.ll (10767 of 10779)
INFO - subprocess.CalledProcessError: Command '['/bin/bash', '-e', '/opt/conda/conda-bld/llvmdev_1637166008062/work/conda_build.sh']' returned non-zero exit status 1.
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/vcp-type-mismatch.ll (10768 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/virtual-const-prop-begin.ll (10769 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/vtable-decl.ll (10770 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/branch-funnel-threshold.ll (10771 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/vcp-accesses-memory.ll (10772 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/virtual-const-prop-end.ll (10773 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/export-vcp.ll (10774 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/branch-funnel.ll (10775 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/devirt-single-impl2.ll (10776 of 10779)
INFO - PASS: LLVM :: Transforms/Util/assume-builder.ll (10777 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/virtual-const-prop-check.ll (10778 of 10779)
INFO - PASS: LLVM :: Transforms/WholeProgramDevirt/import.ll (10779 of 10779)
INFO - ********************
INFO - Failed Tests (2):
INFO - LLVM :: ExecutionEngine/RuntimeDyld/PowerPC/ppc64_elf.s
INFO - LLVM :: ExecutionEngine/RuntimeDyld/PowerPC/ppc64_reloc.s
INFO - 
INFO - 
INFO - Testing Time: 162.46s
INFO - Unsupported      :   190
INFO - Passed           : 10536
INFO - Expectedly Failed:    51
INFO - Failed           :     2

Here is a log-snippet ⬆️

@seibert
Copy link
Contributor Author

seibert commented Nov 19, 2021

OK, I'll see if there is a way to skip those tests. We really don't care about the ability of an x86 32-bit host to cross compile to PPC 64-bit.

@esc
Copy link
Member

esc commented Nov 19, 2021

@seibert yes, if you can skip the tests, we would have a new llvmdev ready for 0.38.0 release.

FWIW: @stuartarchibald suggested OOB to skip running them from within the conda-build recpie.

@esc
Copy link
Member

esc commented Nov 22, 2021

Here is a suggested patch:

diff --git i/conda-recipes/llvmdev/build.sh w/conda-recipes/llvmdev/build.sh
index 0426dc85ca..ed9f756cdc 100644
--- i/conda-recipes/llvmdev/build.sh
+++ w/conda-recipes/llvmdev/build.sh
@@ -87,5 +87,11 @@ make install || exit $?
 if [[ $ARCH == 'x86_64' ]]; then
    bin/opt -S -vector-library=SVML -mcpu=haswell -O3 $RECIPE_DIR/numba-3016.ll | bin/FileCheck $RECIPE_DIR/numba-3016.ll || exit $?
 fi
+
+# run
 cd ../test
-../build/bin/llvm-lit -vv Transforms ExecutionEngine Analysis CodeGen/X86
+if [[ $ARCH == 'i686' ]]; then
+    ../build/bin/llvm-lit -vv Transforms ExecutionEngine Analysis CodeGen/X86
+else
+    ../build/bin/llvm-lit -vv ExecutionEngine Analysis CodeGen/X86
+fi

@esc
Copy link
Member

esc commented Nov 22, 2021

In the interest of time, I'll push the above patch now and re-run it through the farm.

@seibert
Copy link
Contributor Author

seibert commented Nov 22, 2021

Thanks! I was having some VPN trouble trying to test the patch.

@esc
Copy link
Member

esc commented Nov 22, 2021

build farm result:

Screen Shot 2021-11-22 at 21 22 39

and packages on:

Screen Shot 2021-11-22 at 21 29 45

esc added a commit to esc/llvmlite that referenced this pull request Nov 22, 2021
Testing the `llvmdev` build 4 produced via:

numba#788
@gmarkall gmarkall added 5 - Ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed and removed 4 - Waiting on author labels Nov 23, 2021
@esc
Copy link
Member

esc commented Nov 23, 2021

@seibert thank you for the patch! @gmarkall thank you for the review!

@esc esc merged commit d2d33be into numba:master Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants