Upgrade to LLVM 6 and use BinaryBuilder for LLVM on CI #26925
Conversation
| configure-llvm: | ||
| compile-llvm: | ||
| fastcheck-llvm: | ||
| check-llvm: |
There was a problem hiding this comment.
need to define whichever of these are applicable (probably just get?)
There was a problem hiding this comment.
Yeah, I thought about it and was unsure which ones to define.
One could argue that get-llvm really means getsrc-llvm.
One related question where should the tarball be stored? Right now I dump it in scratch and if we use get-llvm it probably should go into srccache.
ararslan
added
building
|
Intermittent status report: The build-system changes seem to work, but we hit issues with the prepared tarballs. |
|
Good news! All test pass. CI is red because apparently httpbin.org got hit by a network fluke and that caused one test to fail across Circle, Travis, and Appveyor. At this point I would encourage testing and reviews by others and I would like to see this merged soon (tm). |
vchuravy
changed the title
| @@ -220,6 +220,11 @@ includedir_rel := $(shell $(JULIAHOME)/contrib/relative_path.sh $(bindir) $(incl | |||
| INSTALL_F := $(JULIAHOME)/contrib/install.sh 644 | |||
| INSTALL_M := $(JULIAHOME)/contrib/install.sh 755 | |||
|
|
|||
| # BinaryBuilder options | |||
| # TODO: Autodiscover triplet | |||
There was a problem hiding this comment.
I think $(shell $(CC) -dumpmachine) should work in the majority of cases, with two exceptions: cross-compiling (in which case we already know the target triplet, right?) and platforms such as macOS and FreeBSD, where the minimum version number we support is part of the triplet. But if BinaryBuilder does triplet mapping using the Platform type, the latter case shouldn't be an issue, I would think.
There was a problem hiding this comment.
Yeah it is very close, but not quite and I don't want to spend time on figuring out the precise differences. So for now this has to be a conscious choice.
Make.inc
Outdated
| # BinaryBuilder options | ||
| # TODO: Autodiscover triplet | ||
| USE_BINARYBUILDER_LLVM := 0 | ||
| BB_TRIPLET := |
There was a problem hiding this comment.
If this is intended to be at all user-facing, could we use a more descriptive name, perhaps just spelling out BinaryBuilder as in the USE_BINARYBUILDER_LLVM variable above?
deps/llvm.mk
Outdated
|
|
||
| #Override provision of stage tarball | ||
| $(build_staging)/llvm-$(LLVM_VER)-$(LLVM_BB_REL).tgz: $(BUILDDIR)/llvm-$(LLVM_VER)-$(LLVM_BB_REL)/build-compiled | ||
| $(info Using staged install form staticfloat/LLVMBuilder) |
| @@ -5493,9 +5493,7 @@ static std::unique_ptr<Module> emit_function( | |||
| AttrBuilder *attr = new AttrBuilder(); | |||
| attr->addStackAlignmentAttr(16); | |||
| #if JL_LLVM_VERSION >= 50000 | |||
| f->addAttributes(AttributeList::FunctionIndex, | |||
| AttributeList::get(f->getContext(), | |||
| AttributeList::FunctionIndex, *attr)); | |||
There was a problem hiding this comment.
Is this not still applicable to LLVM 5.0?
There was a problem hiding this comment.
I think it was just never tested.
vchuravy
force-pushed
the
vc/llvmbb
branch
2 times, most recently
from
098428f to
7e06ce0
Compare
|
Wohaaa straight flush. |
|
@nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
|
I will merge this on Friday if there aren't any objections or comments that require big reworks. |
|
We don't have CI set up for Alpine Linux so I'm building and running tests for this branch on Alpine in a Docker container now. I'll report back with my findings. |
|
Note that all systems appear to be showing this during the build: Not sure what the deal with that is. The macro doesn't appear to be referenced directly in any of the Julia files. Perhaps we (or LLVM?) are missing an |
|
The situation on Alpine doesn't change with this PR; all tests pass except for those noted in #26761 ( |
|
Seems like all benchmarks with |
|
@nanosoldier |
|
Even if the BigInt benchmarks happen to be a bit slower here, I don't think that should block this PR. |
|
Restarted Nanosoldier. Looks like your invocation had a syntax error though: specific SHAs need to be prefixed with @nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
|
@Keno You asked how the new optimizer is faring on this branch. I run the tests locally and the only error I encountered was: but maybe I also haven't rebased this for the last three days. |
|
Yeah, you'll want to rebase. |
|
master at fa0704b only triggered with new optimizer. |
- changes the support LLVM version to 6.0.0
- adds support for using a pre-built LLVM binary from
`staticfloat/LLVMBuilder` taking advantage of the
BinaryBuilder infrastructure.
- adds two Makefile flags:
- `BINARYBUILDER_TRIPLET` the BinaryBuilder triplet you are building for.
An example would be `x86_64-linux-gnu`.
- `USE_BINARYBUILDER_LLVM = 0` set this to 1 if you want to use
the pre-built binaries for LLVM.
- uses the pre-built binaries on OSX Travis instead of the bottle.
- uses the pre-built binaries on Appveyor instead of a custom cache.
|
I'll take a look at that failure. |
This fixes gc lowering for phis of the form:
```
%phi = phi {%jl_value_t addrspace(10)*, i8} [%aunion, %a], [%bunion, %b]
```
This kind of struct (used by the tagged union representation) has special support
in GC lowering. After optimization, these structs often survive quite far as a
struct (i.e. the destructuring of them is often elided). We simply need to extend
this support to the cases that handle phi and select nodes.
Fixes crashes with the new optimizer enabled noted in #26925
This fixes gc lowering for phis of the form:
```
%phi = phi {%jl_value_t addrspace(10)*, i8} [%aunion, %a], [%bunion, %b]
```
This kind of struct (used by the tagged union representation) has special support
in GC lowering. After optimization, these structs often survive quite far as a
struct (i.e. the destructuring of them is often elided). We simply need to extend
this support to the cases that handle phi and select nodes.
Fixes crashes with the new optimizer enabled noted in #26925
|
Awesome work here, Valentin! |
This is a work-in-progress PR for #26398.
This uses the BinaryBuilder infrastructure to create tarballs
with our patchset, that we can use to seed the Mac OSX and Windows
CIs. See https://github.com/staticfloat/LLVMBuilder for the buildscripts.
Many thanks to @staticfloat