bump LLVM BB version and use assertion builds on CI #27182

vchuravy · 2018-05-20T19:23:25Z

Adds patches for:

JeffBezanson · 2018-05-20T19:53:31Z

.travis.yml

        brew install -v --only-dependencies --HEAD julia;
        brew install -v staticfloat/juliadeps/libgfortran;
-        BUILDOPTS="-j3 USECLANG=1 USECCACHE=1 BINARYBUILDER_TRIPLET=x86_64-apple-darwin14";
+        BUILDOPTS="-j3 USECLANG=1 USECCACHE=1 BINARYBUILDER_TRIPLET=x86_64-apple-darwin14 BINARYBUILDER_LLVM_DEBUG=1";


Does this use a full Debug build of LLVM, or just enable assertions? Debug builds would probably be too slow for CI.

It uses a RelWithDebInfo build + Assertions, my intention was to make assertions give us a bit more useful information, but you are right it might be to slow for CI.

Oh ok. I don't think I've ever used a RelWithDebInfo build. I would guess it's probably much faster than a full Debug build though.

Yeah I use that as my daily driver and it allows me to have reasonable speeds while being able to debug LLVM code. You can set LLVM_DEBUG=Release in your Make.user to get that.

We should call it BINARYBUILDER_LLVM_RELWITHDEBINFO=1 then; so that we're not mixing metaphors.

Hm that is a lot to write..., a full debug build is not really useful in any case.

vchuravy · 2018-05-20T20:23:43Z

@iblis17 Looks like FreeBSD doesn't clean properly and as such is sticky to patches? The current recommendation is to use ccache and git clean -fdx to make sure that each build is from scratch (and reasonably fast)

iblislin · 2018-05-21T04:10:34Z

I'm going to try ccache locally at first. Will gmake CC='ccache clang' work? (IIRC, our Make.inc will override CC)

iblislin · 2018-05-21T04:32:43Z

Oh, I got it. There is an option USECCACHE.

iblislin · 2018-05-21T05:32:20Z

@vchuravy But in case of this CI build (https://freebsdci.julialang.org/#/builders/1/builds/9574),
I can reproduce the same error at local box from a clean clone.
I think it's not cache issue.

--------------------------
|Index: llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp
|===================================================================
|--- a/lib/Target/X86/X86DomainReassignment.cpp
|+++ b/lib/Target/X86/X86DomainReassignment.cpp
--------------------------
Patching file lib/Target/X86/X86DomainReassignment.cpp using Plan A...
Hunk #1 succeeded at 26.
Hunk #2 succeeded at 311 (offset 19 lines).
patch: **** malformed patch at line 38: @@ -339,7 +365,7 @@
gmake[1]: *** [/usr/home/julia/julia-fbsd-buildbot/worker/11rel-amd64/build/deps/llvm.mk:492: /usr/home/julia/julia-fbsd-buildbot/worker/11rel-amd64/build/deps/srccache/llvm-6.0.0/llvm-rL332682.patch-applied] Error 2
gmake[1]: *** Waiting for unfinished jobs....

ccache is an attractive suggestion. I will add it later.

iblislin · 2018-05-21T06:01:55Z

I manually tuned the patch llvm-rL332682.patch

diff --git a/deps/patches/llvm-rL332682.patch b/deps/patches/llvm-rL332682.patch
index b6a15c29b0..7bfe1e420f 100644
--- a/deps/patches/llvm-rL332682.patch
+++ b/deps/patches/llvm-rL332682.patch
@@ -24,7 +24,7 @@ Index: llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp
      for (RegDomain D : LegalDstDomainList)
        LegalDstDomains.set(D);
    }
-@@ -328,6 +333,27 @@
+@@ -328,6 +333,10 @@
      return Instrs;
    }

@@ -69,7 +69,7 @@ Index: llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp
    for (unsigned Idx = 0; Idx < MRI->getNumVirtRegs(); ++Idx) {
      unsigned Reg = TargetRegisterInfo::index2VirtReg(Idx);

-@@ -717,20 +744,22 @@
+@@ -717,20 +744,21 @@
        continue;

      // Calculate closure starting with Reg.

Then, the error message changed. It's from llvm-rL332302.patch now. Any idea?

Hmm...  Looks like a unified diff to me...                                                                                                         
The text leading up to this was:                                                                                                                   
--------------------------                                                                                                                         
|Index: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp                                                                             
|===================================================================                                                                               
|--- a/lib/Transforms/InstCombine/InstructionCombining.cpp      (revision 332301)                                                                  
|+++ b/lib/Transforms/InstCombine/InstructionCombining.cpp      (revision 332302)                                                                  
--------------------------                                                                                                                         
Patching file lib/Transforms/InstCombine/InstructionCombining.cpp using Plan A...                                                                  
Hunk #1 failed at 1.
1 out of 1 hunks failed--saving rejects to lib/Transforms/InstCombine/InstructionCombining.cpp.rej
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Index: llvm/trunk/test/Transforms/InstCombine/gep-addrspace.ll
|===================================================================
|--- a/test/Transforms/InstCombine/gep-addrspace.ll     (revision 332301)
|+++ b/test/Transforms/InstCombine/gep-addrspace.ll     (revision 332302)
--------------------------
Patching file test/Transforms/InstCombine/gep-addrspace.ll using Plan A...
Hunk #1 succeeded at 1.
done
gmake[1]: *** [/data/iblis/git/julia/deps/llvm.mk:493: /data/iblis/git/julia/deps/srccache/llvm-6.0.0/llvm-rL332302.patch-applied] Error 1
gmake: *** [Makefile:60: julia-deps] Error 2

vchuravy · 2018-05-21T11:37:47Z

Thanks! There is also an issue with the patch. I was noticed in the patch notes that it looks like it was double applying a patch, which had a different name in a different PR, that why I was referencing the stickyness of the build.

…

On Mon, May 21, 2018, 02:02 Iblis Lin ***@***.***> wrote: I manually tuned the patch llvm-rL332682.patch diff --git a/deps/patches/llvm-rL332682.patch b/deps/patches/llvm-rL332682.patch index b6a15c29b0..7bfe1e420f 100644--- a/deps/patches/llvm-rL332682.patch+++ b/deps/patches/llvm-rL332682.patch@@ -24,7 +24,7 @@ Index: llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp for (RegDomain D : LegalDstDomainList) LegalDstDomains.set(D); }-@@ -328,6 +333,27 @@+@@ -328,6 +333,10 @@ return Instrs; } @@ -69,7 +69,7 @@ Index: llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp for (unsigned Idx = 0; Idx < MRI->getNumVirtRegs(); ++Idx) { unsigned Reg = TargetRegisterInfo::index2VirtReg(Idx); -@@ -717,20 +744,22 @@+@@ -717,20 +744,21 @@ continue; // Calculate closure starting with Reg. Then, the error message changed. It's from llvm-rL332302.patch now. Any idea? Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |Index: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp |=================================================================== |--- a/lib/Transforms/InstCombine/InstructionCombining.cpp (revision 332301) |+++ b/lib/Transforms/InstCombine/InstructionCombining.cpp (revision 332302) -------------------------- Patching file lib/Transforms/InstCombine/InstructionCombining.cpp using Plan A... Hunk #1 failed at 1. 1 out of 1 hunks failed--saving rejects to lib/Transforms/InstCombine/InstructionCombining.cpp.rej Hmm... The next patch looks like a unified diff to me... The text leading up to this was: -------------------------- |Index: llvm/trunk/test/Transforms/InstCombine/gep-addrspace.ll |=================================================================== |--- a/test/Transforms/InstCombine/gep-addrspace.ll (revision 332301) |+++ b/test/Transforms/InstCombine/gep-addrspace.ll (revision 332302) -------------------------- Patching file test/Transforms/InstCombine/gep-addrspace.ll using Plan A... Hunk #1 succeeded at 1. done gmake[1]: *** [/data/iblis/git/julia/deps/llvm.mk:493: /data/iblis/git/julia/deps/srccache/llvm-6.0.0/llvm-rL332302.patch-applied] Error 1 gmake: *** [Makefile:60: julia-deps] Error 2 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27182 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAI3aovX_46d3_yi2w45ir5gQZcjlCefks5t0lhWgaJpZM4UGMQA> .

Required by: JuliaLang#27182

vchuravy · 2018-05-22T02:30:11Z

Ok let's try this again, I had to rewrite your patch @JeffBezanson and I created a new LLVMBuilder release.
I am a bit bewildered to why I was under the impression that the patches applied and worked.
LLVMBuilder is now a bit more robust against failure and I chalk that up to a stale cache or something...

Keno

LGTM, but I do agree that we should change the flag name, since LLVM_DEBUG=1 is the debug build. If you want to shorted it, I think BB_LLVM_REL_DBG=1 would be fine.

vchuravy · 2018-05-22T18:37:54Z

Address review comments and rebased.

…

On Tue, May 22, 2018, 14:26 Keno Fischer ***@***.***> wrote: ***@***.**** approved this pull request. LGTM, but I do agree that we should change the flag name, since LLVM_DEBUG=1 is the debug build. If you want to shorted it, I think BB_LLVM_REL_DBG=1 would be fine. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27182 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAI3amyiPJRs-5vF61Q7pp_pIzcV8iQBks5t1Fg8gaJpZM4UGMQA> .

Keno · 2018-05-23T01:12:17Z

Seems to be missing AppVeyor CI?

vchuravy · 2018-05-23T12:08:16Z

Appveyor seems to have timed out after 2h, what is our normal time on AV?

vchuravy · 2018-05-23T17:11:36Z

Timed out again after 2h I hope that #27222, helps otherwise I will try and use the non-debug build on AV.

vchuravy · 2018-05-23T18:22:49Z

@Keno What should the output be for:

llc -mcpu=skylake-avx512 -mtriple=x86_64-unknown-linux-gnu %s -o - | llvm-mc

in the domain-reassignment test?

        .text
        .file   "domain-reassignment-test.ll"
        .globl  japi1_foo2_34617
        .p2align        4, 0x90
        .type   japi1_foo2_34617,@function
japi1_foo2_34617:
        .cfi_startproc
        vpxor   %xmm0, %xmm0, %xmm0
<stdin>:10:2: error: instruction requires: AVX-512 ISA AVX-512 VL ISA
        vpcmpgtq        %xmm0, %xmm0, %k0
        ^
<stdin>:11:2: error: instruction requires: AVX-512 ISA
        kshiftrw        $1, %k0, %k1
        ^
<stdin>:12:2: error: instruction requires: AVX-512 BW ISA
        kmovd   %k0, %eax
        ^
        testq   %rax, %rax
        setg    %cl
<stdin>:15:2: error: instruction requires: AVX-512 BW ISA
        kmovd   %k1, %edx
        ^
        andl    %eax, %edx
        testb   %dl, %cl
        je      .LBB0_1
        retq
.LBB0_1:
.Lfunc_end0:
        .size   japi1_foo2_34617, .Lfunc_end0-japi1_foo2_34617
        .cfi_endproc


        .section        ".note.GNU-stack","",@progbits

Keno · 2018-05-23T18:24:59Z

You may be missing llvm-mirror/llvm@f5e705e? llvm-mc should not error

vchuravy · 2018-05-23T18:30:13Z

No I have that one, but I need to pass -mcpu=skylake-avx512 aswell.

Keno · 2018-05-31T14:39:12Z

Bump! There've been two issues filed in the last 24 hours for issues fixed by this PR.

vchuravy · 2018-06-01T20:38:51Z

Working on it. I want to make sure that LLVM passes tests properly with LLVMBuilder.

…

On Thu, May 31, 2018, 10:39 Keno Fischer ***@***.***> wrote: Bump! There've been two issues filed in the last 24 hours for issues fixed by this PR. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#27182 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAI3ajgSkZYs3ZwC1bcWW3v1oD0wDwomks5t4ACkgaJpZM4UGMQA> .

vchuravy · 2018-06-03T03:08:34Z

It turns out that in the original trial I didn't set a flag correctly and didn't actually build a RelWithDebInfo build. Those are about 2GB and I think therefore unfeasable for CI.
I am switching things over to only Asserts now.

vchuravy · 2018-06-07T14:37:26Z

@Keno genuine assertion error on Windows:

Allocations: 5256 (Pool: 5256; Big: 0); GC: 0
Assertion failed!
Program: C:\projects\julia\usr\bin\julia-debug.exe
File: /workspace/srcdir/llvm-6.0.0.src/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp, Line 2551
Expression: InChain.getValueType() == MVT::Other && "Not a chain"
*** This error is usually fixed by running `make clean`. If the error persists, try `make cleanall`. ***
make[1]: *** [Makefile:203: /cygdrive/c/projects/julia/usr/lib/julia/sys-debug-o.a] Error

Adds patches for: - #27055 - #27080 - #27032 furthermore this adds the ability to use assertion builds on CI. Which is used on OSX and Windows.

vchuravy · 2018-06-19T14:27:37Z

First a straight flush and then a spurious merge conflict on a file that this PR doesn't touch...
This is ready to get merged once CI passes.

JeffBezanson reviewed May 20, 2018

View reviewed changes

vchuravy force-pushed the vc/bbpatch branch from 9a36c83 to 252e469 Compare May 20, 2018 20:14

iblislin added a commit to iblislin/julia that referenced this pull request May 21, 2018

freebsdci: enable ccache and add cleanup stage

363eacc

Required by: JuliaLang#27182

iblislin mentioned this pull request May 21, 2018

freebsdci: enable ccache and add cleanup stage #27186

Merged

vchuravy force-pushed the vc/bbpatch branch from 252e469 to 3be9714 Compare May 22, 2018 02:25

vchuravy requested a review from Keno May 22, 2018 18:23

Keno approved these changes May 22, 2018

View reviewed changes

vchuravy force-pushed the vc/bbpatch branch from 3be9714 to e5e1b01 Compare May 22, 2018 18:35

vchuravy closed this May 23, 2018

vchuravy reopened this May 23, 2018

vchuravy force-pushed the vc/bbpatch branch 2 times, most recently from 2c78a29 to 5be5329 Compare May 24, 2018 18:33

kshyatt added the ci Continuous integration label May 27, 2018

Keno mentioned this pull request May 31, 2018

Segfault with @code_warntype on 0.7.0-alpha #27348

Closed

Keno added this to the 0.7 milestone Jun 1, 2018

vchuravy force-pushed the vc/bbpatch branch from 5be5329 to 32d4445 Compare June 3, 2018 02:44

vchuravy force-pushed the vc/bbpatch branch from 32d4445 to a0fc039 Compare June 7, 2018 13:20

This was referenced Jun 7, 2018

Overly restricting package versions by julia compatibility requirements JuliaLang/Pkg.jl#354

Closed

Wrong result when simd-ing over BitArrays #27482

Closed

vchuravy force-pushed the vc/bbpatch branch from a0fc039 to c704899 Compare June 15, 2018 19:27

vchuravy mentioned this pull request Jun 15, 2018

Windows triggers assertion in debug mode with ssp #27603

Closed

vchuravy force-pushed the vc/bbpatch branch from c704899 to efeb35a Compare June 19, 2018 06:24

JeffBezanson and others added 2 commits June 19, 2018 10:21

bump LLVM BB version

6f15fff

Adds patches for: - #27055 - #27080 - #27032 furthermore this adds the ability to use assertion builds on CI. Which is used on OSX and Windows.

fix undef warning coming from LLVM

1abdab4

vchuravy force-pushed the vc/bbpatch branch from efeb35a to 1abdab4 Compare June 19, 2018 14:21

vchuravy merged commit a169b34 into master Jun 19, 2018

vchuravy deleted the vc/bbpatch branch June 19, 2018 17:25

This was referenced Jun 19, 2018

Revert "Revert "enable the SLP Vectorizer optimization pass by default"" #27659

Closed

undef warning during build after LLVM upgrade #27019

Closed

non-Jedi mentioned this pull request Sep 21, 2018

[WIP][DO NOT MERGE] preparation work for llvm 7.0.0 #28809

Closed

Uh oh!

bump LLVM BB version and use assertion builds on CI #27182

bump LLVM BB version and use assertion builds on CI #27182

Uh oh!

Conversation

vchuravy commented May 20, 2018

Uh oh!

JeffBezanson May 20, 2018

Choose a reason for hiding this comment

Uh oh!

vchuravy May 20, 2018

Choose a reason for hiding this comment

Uh oh!

JeffBezanson May 20, 2018

Choose a reason for hiding this comment

Uh oh!

vchuravy May 20, 2018

Choose a reason for hiding this comment

Uh oh!

staticfloat May 20, 2018

Choose a reason for hiding this comment

Uh oh!

vchuravy May 22, 2018

Choose a reason for hiding this comment

Uh oh!

vchuravy commented May 20, 2018

Uh oh!

iblislin commented May 21, 2018

Uh oh!

iblislin commented May 21, 2018

Uh oh!

iblislin commented May 21, 2018

Uh oh!

iblislin commented May 21, 2018

Uh oh!

vchuravy commented May 21, 2018 via email

Uh oh!

vchuravy commented May 22, 2018

Uh oh!

Keno left a comment

Choose a reason for hiding this comment

Uh oh!

vchuravy commented May 22, 2018 via email

Uh oh!

Keno commented May 23, 2018

Uh oh!

vchuravy commented May 23, 2018

Uh oh!

vchuravy commented May 23, 2018

Uh oh!

vchuravy commented May 23, 2018

Uh oh!

Keno commented May 23, 2018

Uh oh!

vchuravy commented May 23, 2018

Uh oh!

Keno commented May 31, 2018

Uh oh!

vchuravy commented Jun 1, 2018 via email

Uh oh!

vchuravy commented Jun 3, 2018

Uh oh!

vchuravy commented Jun 7, 2018

Uh oh!

vchuravy commented Jun 19, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants