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.

Sign up for GitHub

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

Jump to bottom

Backporting the libblas and vendor() improvements to 1.8 #44467

Closed
wants to merge 26 commits into from
Closed

Backporting the libblas and vendor() improvements to 1.8 #44467

wants to merge 26 commits into from

Conversation

ViralBShah
Copy link
Member

@ViralBShah ViralBShah commented Mar 5, 2022 •
edited
Loading

This PR includes #44360, #44379, and #42957 all of which are required together. The crash reported in #44379 (comment) is fixed as a result.

jipolanco and others added 24 commits February 24, 2022 08:41
@jipolanco @KristofferC
Make sure range(start; step, length) uses TwicePrecision when possi…
b9982a8 
…ble, fixes #44292. (#44313)

(cherry picked from commit 1b47efb)
@timholy @KristofferC
Cache external CodeInstances (#43990)
7731773 
Prior to this PR, Julia's precompiled `*.ji` files saved just two
categories of code: unspecialized method definitions and
type-specialized code for the methods defined by the package.  Any
novel specializations of methods from Base or previously-loaded
packages were not saved, and therefore effectively thrown away.

This PR caches all the code---internal or external---called during
package definition that hadn't been previously inferred, as long
as there is a backedge linking it back to a method owned by
a module being precompiled. (The latter condition ensures it will
actually be called by package methods, and not merely transiently
generated for the purpose of, e.g., metaprogramming or variable
initialization.) This makes precompilation more intuitive (now it
saves all relevant inference results), and substantially reduces
latency for inference-bound packages.

Closes #42016
Fixes #35972

Issue #35972 arose because codegen got started without re-inferring
some discarded CodeInstances. This forced the compiler to insert a
`jl_invoke`. This PR fixes the issue because needed CodeInstances are
no longer discarded by precompilation.

(cherry picked from commit df81bf9)
@IanButterworth
Add Pkg 1.8 news (#44370)
456e76c
@bluesmoon
Fix hyperlinks in 1.8 news (#44371)
8919242 
(cherry-picked from 675911a)
@timholy
Add NEWS on precompilation (#44325)
db0cdf8
@ViralBShah
Update LBT to 5.0.1 for source build (#44258)
8615223 
(cherry picked from commit 627016a)
@ViralBShah
Regenerate all the checksums (#44275)
6681008 
(cherry picked from commit ac922b0)
@ViralBShah
Documentation: added example to the function hasfield. (From #41782) (#…
583f2e9 
…44274)

Co-authored-by: Johnny Chen <johnnychen94@hotmail.com>
(cherry picked from commit 9c0e5b0d186ea95a06d5b0bdc4bc19d1a17b444d)
(cherry picked from commit aa2421d)
@giordano
[OpenBLAS_jll] Update to v0.3.20 (#44321)
51db9bd 
(cherry picked from commit 6850940)
@JeffBezanson
fix #43411, wrapped NamedTuple can be bitstype more often (#44311)
deb3049 
(cherry picked from commit f20d5de)
@staticfloat
[macOS] Codesign binary-dist tarballs (#44305)
e3a126a 
Because we're starting to distribute macOS tarballs as well, let's
codesign them by default, when possible.

(cherry picked from commit 6b29ebd)
@yuyichao
Make sure all the relocations are filled in for partially cloned targ…
10c27af 
…et (#44262)

We collect the relocations (i.e. the GOT slots that is used in the code) for each target
in `tgt.relocs`. Needing a relocation, however, does not imply that the function is cloned
for this target within the group (It does mean that at least one target
in the group has it cloned). The previous version would miss the relocation in this case.

This was triggerred with the following cloning situation

    caller: clone_1
    callee: clone_1, clone_1.clone_3

Since caller.clone_1 may call either callee.clone_1 or callee.clone_1.clone_3 a relocation
for callee will be used and is required to be initialized.
In addition to target 1, target 2 (and in fact target 3) within group 1
will also use caller.clone_1. However, since callee isn't cloned for target 2
the previous version wouldn't have saved this slot in the relocation array.

(cherry picked from commit 76fc067)
@simeonschaub
fix #44328: method validation for opaque closures (#44335)
5db0041 
I believe it's intentional that for these methods, the `sig` field is
just ignored and always set to `Tuple`. Also fixes a lowering bug I
discovered that would cause errors if `Union` was shadowed.

I have verified that this fixes the reported warnings.

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit 5deb503)
@ViralBShah
Update BLAS.vendor() to return :lbt
e54b04a 
`vendor()` returns `:lbt`
`libblas_name` and `liblapack_name` are set to "libblastrampoline"

(cherry picked from commit bf6d9de)
@timholy
Fix memory error during precompilation (#44345)
5bb9e64 
Fixes #44338

(cherry picked from commit b4ea0f7)
@vtjnash @KristofferC
asan,build: fix linker flags for -fsanitize builds (#44420)
b2dd618 
Fix #44361
Fix #42540

(cherry picked from commit 7324966)
@vtjnash @KristofferC
add jl_egal and jl_gc_safepoint back to exports list (#44419)
b1312f6 
Simple oversight when these were turned into macros. I could not find
any others that seemed applicable, so just these two appeared to need
fixing right now.

Fix #44373

(cherry picked from commit 8eb872c)
@tkf @KristofferC
Clarify the behavior of @threads for (#44168)
d611460 
* Clarify the behavior of `@threads for`

Co-authored-by: Ian Butterworth <i.r.butterworth@gmail.com>
(cherry picked from commit 2f67b51)
@oscardssmith @KristofferC
fix error show edge case (#44319)
62e55d7 
* fix errors

Co-authored-by: Simeon Schaub <simeondavidschaub99@gmail.com>
(cherry picked from commit a9d8c85)
@vtjnash @KristofferC
optimizer: bail out of inlining if ir_inline_unionsplit will fail (#4…
4b33571 
…4416)

Intersection cannot deal with this `metharg`, so it does not simplify
the type at all when handling this case. This can cause us to run into
an assertion later, where we assume the intersection of a non-Varags
type will always return a simple DataType without Varargs.

Fixes #44238

    atype = Tuple{typeof(Base.similar),
      Tuple{Union{Polyhedra.Polyhedron{T}, Polyhedra.Representation{T}} where T},
      Array{_A, 1} where _A,
      Array{_C, 1} where _C,
      Array{_B, 1} where _B}

    metharg = Tuple{typeof(Base.similar),
      Tuple{Vararg{Union{Polyhedra.Polyhedron{T}, Polyhedra.Representation{T}} where T}},
      Vararg{Union{Union{AbstractArray{var"#s14", 1}, Polyhedra.AbstractRepIterator{var"#s13", var"#s14"} where var"#s13", Polyhedra.AllRepIterator{var"#s14", var"#s14", LinElemT, LRT, RT} where RT<:Polyhedra.AbstractRepIterator{var"#s14", var"#s14"} where LRT<:Polyhedra.AbstractRepIterator{var"#s14", LinElemT} where LinElemT where var"#s14"} where var"#s14"<:(Polyhedra.HyperPlane{T, AT} where AT<:AbstractArray{T, 1}), Union{AbstractArray{var"#s14", 1}, Polyhedra.AbstractRepIterator{var"#s13", var"#s14"} where var"#s13", Polyhedra.AllRepIterator{var"#s14", var"#s14", LinElemT, LRT, RT} where RT<:Polyhedra.AbstractRepIterator{var"#s14", var"#s14"} where LRT<:Polyhedra.AbstractRepIterator{var"#s14", LinElemT} where LinElemT where var"#s14"} where var"#s14"<:(Polyhedra.HalfSpace{T, AT} where AT<:AbstractArray{T, 1}), Union{AbstractArray{var"#s14", 1}, Polyhedra.AbstractRepIterator{var"#s13", var"#s14"} where var"#s13", Polyhedra.AllRepIterator{var"#s14", var"#s14", LinElemT, LRT, RT} where RT<:Polyhedra.AbstractRepIterator{var"#s14", var"#s14"} where LRT<:Polyhedra.AbstractRepIterator{var"#s14", LinElemT} where LinElemT where var"#s14"} where var"#s14"<:AbstractArray{T, 1}, Union{AbstractArray{var"#s14", 1}, Polyhedra.AbstractRepIterator{var"#s13", var"#s14"} where var"#s13", Polyhedra.AllRepIterator{var"#s14", var"#s14", LinElemT, LRT, RT} where RT<:Polyhedra.AbstractRepIterator{var"#s14", var"#s14"} where LRT<:Polyhedra.AbstractRepIterator{var"#s14", LinElemT} where LinElemT where var"#s14"} where var"#s14"<:(Polyhedra.Line{T, AT} where AT<:AbstractArray{T, 1}), Union{AbstractArray{var"#s14", 1}, Polyhedra.AbstractRepIterator{var"#s13", var"#s14"} where var"#s13", Polyhedra.AllRepIterator{var"#s14", var"#s14", LinElemT, LRT, RT} where RT<:Polyhedra.AbstractRepIterator{var"#s14", var"#s14"} where LRT<:Polyhedra.AbstractRepIterator{var"#s14", LinElemT} where LinElemT where var"#s14"} where var"#s14"<:(Polyhedra.Ray{T, AT} where AT<:AbstractArray{T, 1})} where T}}

Currently `typeintersection(atype, metharg) === metharg`

(cherry picked from commit ffc5ffa)
Revert "Update BLAS.vendor() to return :lbt"
fb69baf 
This reverts commit e54b04a.
@ViralBShah
Simplification of libblastrampoline stuff in LinearAlgebra (#44360)
dc38ba5 
* Report libblastrampoline as Base.libblas_name and Base.liblapack_name

* Cleanup a lot of the LBT stuff

* Fix

* Remove test for LinearAlgebra.LAPACK.liblapack

* Fix

* Fix the precompile tests

(cherry picked from commit e84b06c)
@ViralBShah
Update BLAS.vendor() to return :lbt
0adbba6 
`vendor()` returns `:lbt`
`libblas_name` and `liblapack_name` are set to "libblastrampoline"

(cherry picked from commit bf6d9de)
(cherry picked from commit e54b04a)
@ViralBShah
cleanup
a308435
@ViralBShah ViralBShah added the linear algebra Linear algebra label Mar 5, 2022
@ViralBShah
Copy link
Member Author

Some of the 0-stride tests are failing - no exception is being thrown. Maybe something changed in openblas 0.3.20. I'll look into it a bit more and should be easy to fix.

@ViralBShah
Copy link
Member Author

ViralBShah commented Mar 6, 2022 •
edited
Loading

I think we need #42957 to fix the test failures with 0 strides - which seems to be necessary since we updated to openblas 0.3.20, added tests, but did not pull in the relevant fixes that got merged just after branching.

@ViralBShah
Copy link
Member Author

win32 test failure seems unrelated.

@ViralBShah ViralBShah added this to the 1.8 milestone Mar 7, 2022
@ViralBShah ViralBShah requested a review from a team as a code owner March 7, 2022 09:20
@KristofferC
Copy link
Member

Why is this so important to get into 1.8 (and why is it on the milestone). And #42957 looks like a feature.

@ViralBShah
Copy link
Member Author

This has been a lot of hassle for 1.8. Let's not bother about getting this to 1.8. We'll just focus on 1.9. There are some annoyances where 1.8 will unconditionally (and incorrectly when MKL is loaded) claim that Base.libblas_name and vendor is openblas - but that was also the case in 1.7.

Best to focus on 1.9 to get it fixed ecosystem-wide.

@ViralBShah ViralBShah closed this Mar 12, 2022
@giordano giordano deleted the vs/libblas-1.8 branch March 31, 2022 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
linear algebra Linear algebra
Projects
None yet
Milestone
1.8
Development

Successfully merging this pull request may close these issues.