Add BB downloading for OpenBLAS #30497

staticfloat · 2018-12-23T09:09:59Z

Also make it easier to use BinaryBuilder tarballs for dependencies.

vchuravy

Thanks! That looks good.

vchuravy · 2018-12-23T17:59:36Z

Makefile

@@ -184,7 +184,7 @@ COMPILER_SRCS += $(shell find $(JULIAHOME)/base/compiler -name \*.jl)
 # sort these to remove duplicates
 BASE_SRCS := $(sort $(shell find $(JULIAHOME)/base -name \*.jl -and -not -name sysimg.jl) \
 $(shell find $(BUILDROOT)/base -name \*.jl -and -not -name sysimg.jl))
-STDLIB_SRCS := $(JULIAHOME)/base/sysimg.jl $(shell find $(build_datarootdir)/julia/stdlib/$(VERSDIR)/*/src -name \*.jl)
+STDLIB_SRCS := $(JULIAHOME)/base/sysimg.jl $(shell find $(JULIAHOME)/stdlib/*/src -name \*.jl)


Have you tested this with an out-of-tree build? It also seems orthogonal got this PR.

Yeah, this is a question I had for Jameson that I posted as a comment on the commit it was introduced in. Hopefully when @vtjnash reads this, he can explain to me why this is wrong. :)

I've put this in a separate PR: #30518

vchuravy · 2018-12-27T11:03:22Z

deps/openblas.version

@@ -1,2 +1,4 @@
 OPENBLAS_BRANCH=v0.3.3
 OPENBLAS_SHA1=fd8d1868a126bb9f12bbc43b36ee30d1ba943fbb
+OPENBLAS_VER=0.3.0


Is this 0.3.0 or 0.3.3

Great catch; I'll build v0.3.3 in Yggdrasil, update this PR, then also submit a PR to bump to v0.3.4, since that looks like it has some important fixes.

Also make it easier to add more BB-cached versions of dependencies in the future

staticfloat · 2018-12-28T22:01:33Z

Alright I think this is good to go, as long as it passes green on our CI.

contrib/fixup-libgfortran.sh

contrib/normalize_triplet.py

ararslan · 2018-12-28T22:19:49Z

contrib/fixup-libgfortran.sh

+ # only get something like `@rpath/libgfortran.5.dylib` when inspecting the
+ # libraries. We can, as a last resort, ask `$FC` directly what the full
+ # filepath for this library is, but only if we don't have a direct path to it:
+ if [ $(dirname "$1") == "@rpath" ]; then


Isn't == also a Bash thing? And what about ${FC:-gfortran}?

${FC:-gfortran} should be fine, according to this.

Okay, cool. As for the conditional, you could make it if [ "$(dirname "$1")" = "@rpath" ].

Also allow the build system to auto-guess the triplet

vtjnash · 2018-12-30T07:02:17Z

deps/tools/bb-install.mk

+configure-$(1):
+compile-$(1):
+fastcheck-$(1):
+check-$(1):


these targets are incorrect and/or should not be here, they should not be empty

They're here to ensure that if someone uses bb-install within an ifeq/else statement, you don't run into a target fastcheck-openblas undefined error. If the target is already defined, this won't do anything.

vtjnash · 2018-12-30T07:03:02Z

deps/tools/bb-install.mk

+ $$(JLDOWNLOAD) $$@ $$($(2)_BB_URL)
+
+$$(BUILDDIR)/$(1)-$$($(2)_BB_NAME)/build-compiled: | $$(BUILDDIR)/$(1)-$$($(2)_BB_NAME)/$(2).$$($(2)_BB_TRIPLET).tar.gz
+ echo 1 > $$@


the checksum verification should happen here before the echo 1 step

You're right; we haven't been verifying checksums for the BB install tarballs. Whoopsies.

* Auto-detect binarybuilder triplet * Add OpenBLAS BinaryBuilder installation scaffolding Also make it easier to add more BB-cached versions of dependencies in the future * Enable `fixup-libgfortran.sh` to directly ask `$FC` for paths * Tell Appveyor and Travis to use BinaryBuilder OpenBLAS Also allow the build system to auto-guess the triplet

This reverts commit 87c18d8.

vchuravy · 2019-01-02T09:03:25Z

contrib/windows/appveyor_build.sh

@@ -198,6 +196,7 @@ if [ -n "$USEMSVC" ]; then
 else
 # Use BinaryBuilder
 echo 'USE_BINARYBUILDER_LLVM = 1' >> Make.user
+ echo 'USE_BINARYBUILDER_OPENBLAS = 1' >> Make.user
 echo 'BINARYBUILDER_LLVM_ASSERTS = 1' >> Make.user
 echo 'override DEP_LIBS += llvm openlibm' >> Make.user


blas needs to be added here. Right now we are still pulling it from latest

What do you mean pulling it from latest?

julia/contrib/windows/appveyor_build.sh

Line 92 in 2c0a919

$curlflags -O https://julialangnightlies-s3.julialang.org/bin/winnt/x$archsuffix/$f

I see; so this is like a hacky way of working around USE_SYSTEM_OPENBLAS?

Yeah... we pull in dependencies from the nightly build, then pretend that they are system libraries, but for the binarybuilder we then need too add those back... Don't ask me why I just work here... ;)

* Auto-detect binarybuilder triplet * Add OpenBLAS BinaryBuilder installation scaffolding Also make it easier to add more BB-cached versions of dependencies in the future * Enable `fixup-libgfortran.sh` to directly ask `$FC` for paths * Tell Appveyor and Travis to use BinaryBuilder OpenBLAS Also allow the build system to auto-guess the triplet

* Auto-detect binarybuilder triplet * Add OpenBLAS BinaryBuilder installation scaffolding Also make it easier to add more BB-cached versions of dependencies in the future * Enable `fixup-libgfortran.sh` to directly ask `$FC` for paths * Tell Appveyor and Travis to use BinaryBuilder OpenBLAS Also allow the build system to auto-guess the triplet (cherry picked from commit 87c18d8)

JeffBezanson · 2019-01-31T20:09:13Z

Is this needed for 1.0 releases?

staticfloat · 2019-01-31T23:30:51Z

I don’t think so. Let’s not backport build fixes unless someone needs them.

staticfloat requested review from vchuravy and vtjnash December 23, 2018 09:10

vchuravy reviewed Dec 23, 2018

View reviewed changes

Auto-detect binarybuilder triplet

67e610e

staticfloat force-pushed the sf/openblas_bb branch from 11aa236 to e005a04 Compare December 26, 2018 21:53

staticfloat changed the title ~~[WIP] Add BB downloading for OpenBLAS~~ Add BB downloading for OpenBLAS Dec 27, 2018

staticfloat force-pushed the sf/openblas_bb branch 2 times, most recently from c136b32 to d85cb84 Compare December 27, 2018 10:17

vchuravy reviewed Dec 27, 2018

View reviewed changes

staticfloat force-pushed the sf/openblas_bb branch from d85cb84 to 6fa8ed8 Compare December 27, 2018 23:27

staticfloat changed the title ~~Add BB downloading for OpenBLAS~~ [WIP] Add BB downloading for OpenBLAS Dec 28, 2018

Add OpenBLAS BinaryBuilder installation scaffolding

5c3f8c1

Also make it easier to add more BB-cached versions of dependencies in the future

staticfloat force-pushed the sf/openblas_bb branch from 000d7c7 to 628f578 Compare December 28, 2018 22:01

staticfloat changed the title ~~[WIP] Add BB downloading for OpenBLAS~~ Add BB downloading for OpenBLAS Dec 28, 2018

ararslan requested changes Dec 28, 2018

View reviewed changes

contrib/fixup-libgfortran.sh Outdated Show resolved Hide resolved

contrib/normalize_triplet.py Show resolved Hide resolved

staticfloat force-pushed the sf/openblas_bb branch from 628f578 to 5df6758 Compare December 28, 2018 22:17

ararslan reviewed Dec 28, 2018

View reviewed changes

staticfloat added 2 commits December 28, 2018 14:21

Enable fixup-libgfortran.sh to directly ask $FC for paths

b78ef16

Tell Appveyor and Travis to use BinaryBuilder OpenBLAS

2c0a919

Also allow the build system to auto-guess the triplet

staticfloat force-pushed the sf/openblas_bb branch from 5df6758 to 2c0a919 Compare December 28, 2018 22:26

ararslan approved these changes Dec 29, 2018

View reviewed changes

staticfloat merged commit 87c18d8 into master Dec 29, 2018

ararslan deleted the sf/openblas_bb branch December 29, 2018 07:57

vtjnash reviewed Dec 30, 2018

View reviewed changes

Keno added a commit that referenced this pull request Dec 30, 2018

Revert "Add BB downloading for OpenBLAS (#30497)"

a3816b4

This reverts commit 87c18d8.

ararslan added a commit that referenced this pull request Dec 31, 2018

Revert "Add BB downloading for OpenBLAS (#30497)"

b09d3cb

This reverts commit 87c18d8.

vchuravy reviewed Jan 2, 2019

View reviewed changes

KristofferC added backport pending 1.0 labels Jan 11, 2019

This was referenced Jan 11, 2019

Backports for 1.1.0-rc2 #30607

Merged

Backports for 1.0.4 #30536

Closed

KristofferC removed the backport pending 1.1.0 label Jan 30, 2019

StefanKarpinski added status:triage This should be discussed on a triage call backport 1.0 and removed backport 1.0 status:triage This should be discussed on a triage call labels Jan 31, 2019

JeffBezanson removed the triage backport pending 1.0 label Jan 31, 2019

KristofferC removed backport 1.0 status:triage This should be discussed on a triage call labels Feb 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BB downloading for OpenBLAS #30497

Add BB downloading for OpenBLAS #30497

staticfloat commented Dec 23, 2018

vchuravy left a comment

vchuravy Dec 23, 2018

staticfloat Dec 23, 2018

staticfloat Dec 26, 2018

vchuravy Dec 27, 2018

staticfloat Dec 27, 2018

staticfloat commented Dec 28, 2018

ararslan Dec 28, 2018

staticfloat Dec 28, 2018

ararslan Dec 28, 2018

vtjnash Dec 30, 2018

staticfloat Dec 30, 2018

vtjnash Dec 30, 2018

staticfloat Dec 30, 2018

vchuravy Jan 2, 2019

staticfloat Jan 2, 2019

vchuravy Jan 2, 2019

staticfloat Jan 2, 2019

vchuravy Jan 3, 2019

JeffBezanson commented Jan 31, 2019

staticfloat commented Jan 31, 2019

Add BB downloading for OpenBLAS #30497

Add BB downloading for OpenBLAS #30497

Conversation

staticfloat commented Dec 23, 2018

vchuravy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

staticfloat commented Dec 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented Jan 31, 2019

staticfloat commented Jan 31, 2019