The Wayback Machine - https://web.archive.org/web/20220301045726/https://github.com/JuliaLang/julia/pull/44360
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

Jump to bottom

Simplification of libblastrampoline stuff in LinearAlgebra #44360

Merged
merged 6 commits into from Feb 28, 2022
Merged

Simplification of libblastrampoline stuff in LinearAlgebra #44360

merged 6 commits into from Feb 28, 2022
+15 −68

Conversation

ViralBShah
Copy link
Member

@ViralBShah ViralBShah commented Feb 26, 2022 •
edited

We traditionally hardcode libblas_name and liblapack_name into the system image. These do not get updated, even when you use an alternate set of libraries through MKL.jl. build_h.jl was reporting different names than what was in the BLAS and LAPACK modules in LinearAlgebra.

Now that we use LBT, it is best to mention libblastrampoline as the provider of these libraries. This revealed a lot of old code that could be simplified and cleaned up.

Perhaps we can remove these things altogether from build_h.jl, since these are no longer build time choices, but runtime ones.

Also undo the compatibility symbols added in #39845 - since the new APIs now have been around for a while.

cc @staticfloat
@ViralBShah ViralBShah requested a review from staticfloat Feb 26, 2022
@ViralBShah ViralBShah changed the title Report libblastrampoline as Base.libblas_name and Base.liblapack_name Feb 26, 2022
@staticfloat
Copy link
Sponsor Member

@staticfloat staticfloat commented Feb 27, 2022

I think this makes sense, overall. It's probably worth a NEWS.md entry to state that we're now wholly reliant upon LBT, and that we straight up don't support building against any other BLAS/LAPACK library, and that any other usage should be done through LBT configuration at runtime.
@ViralBShah
Copy link
Sponsor Member Author

@ViralBShah ViralBShah commented Feb 27, 2022

How do you feel about removing blas and LAPACK from build_h.jl altogether?
@staticfloat
Copy link
Sponsor Member

@staticfloat staticfloat commented Feb 27, 2022

Yeah, I think that's the right thing to do. Those values are easily made into lies.
@ViralBShah
Copy link
Sponsor Member Author

@ViralBShah ViralBShah commented Feb 27, 2022 •
edited

@staticfloat Any thoughts on why this PR might cause the precompile tests to fail? I made some changes to a couple of JLL packages in terms of dependencies. Do I need to update some package related metadata?

Error in testset precompile:
Test Failed at /Users/viral/julia/test/precompile.jl:352
  Expression: Dict(modules) == merge(Dict((let m = Base.PkgId(s)
                m => Base.module_build_id(Base.root_module(m))
            end for s = ["Base", "Core", "Main", string(Foo2_module), string(FooBase_module)])), Dict((let m = Base.root_module(Base, s)
                Base.PkgId(m) => Base.module_build_id(m)
            end for s = [:ArgTools, :Artifacts, :Base64, :CRC32c, :Dates, :DelimitedFiles, :Distributed, :Downloads, :FileWatching, :Future, :InteractiveUtils, :LazyArtifacts, :LibCURL, :LibCURL_jll, :LibGit2, :Libdl, :LinearAlgebra, :Logging, :Markdown, :Mmap, :MozillaCACerts_jll, :NetworkOptions, :Pkg, :Printf, :Profile, :p7zip_jll, :REPL, :Random, :SHA, :Serialization, :SharedArrays, :Sockets, :SparseArrays, :Statistics, :SuiteSparse, :TOML, :Tar, :Test, :UUIDs, :Unicode, :nghttp2_jll])))
   Evaluated: Dict{Base.PkgId, UInt64}(Statistics [10745b16-79ce-11e8-11f9-7d13ad32a3b2] => 0x0002eb23c71144be, SparseArrays [2f01184e-e22b-5df5-ae63-d93ebab69eaf] => 0x0002eb2214c670f7, Downloads [f43a241f-c20a-4ad4-852c-f6b1247861c6] => 0x0002eb244b8f16f8, Base64 [2a0f44e3-6c83-55bd-87e4-b1978d98bd5f] => 0x0002eb1e01380cf9, F2oo4b3a94a1a081a8cb [top-level] => 0x0003278108320f26, Markdown [d6f4376e-aef5-505a-96c1-9c027394607a] => 0x0002eb206cfe7dba, Unicode [4ec0a83e-493e-50e2-b9ac-8f72acf5a8f5] => 0x0002eb1e4ee6f142, FooBase4b3a94a1a081a8cb [top-level] => 0x0003278129bca2c7, CompilerSupportLibraries_jll [e66e0078-7015-5450-92f7-15fbd957f2ae] => 0x0002eb1e5ba137d5, MozillaCACerts_jll [14a3606d-f60d-562e-9121-12d972cd8159] => 0x0002eb2436d83a53…) == Dict{Base.PkgId, UInt64}(Downloads [f43a241f-c20a-4ad4-852c-f6b1247861c6] => 0x0002eb244b8f16f8, Unicode [4ec0a83e-493e-50e2-b9ac-8f72acf5a8f5] => 0x0002eb1e4ee6f142, FooBase4b3a94a1a081a8cb [top-level] => 0x0003278129bca2c7, Main [top-level] => 0x0002eb05679d7af1, REPL [3fa0cd96-eef1-5676-8a61-b3b8758bbffb] => 0x0002eb22cf73e795, NetworkOptions [ca575930-c2e3-43a9-ace4-1e988b2c1908] => 0x0002eb1e14f739f3, DelimitedFiles [8bb1440f-4735-579b-a4ab-409b98df4dab] => 0x0002eb1e55dfc8eb, Serialization [9e88b42a-f829-5b0c-bbe9-9e923198166b] => 0x0002eb1e2c091fd6, Profile [9abbd945-dff8-562f-b5e8-e1ebf5ef1b79] => 0x0002eb220120a768, UUIDs [cf7118a7-6976-5b1a-9a39-7adc72f591a4] => 0x0002eb22cc2447a0…)
@ViralBShah
Copy link
Sponsor Member Author

@ViralBShah ViralBShah commented Feb 28, 2022

This is good to merge.
@staticfloat staticfloat merged commit e84b06c into master Feb 28, 2022
16 checks passed
@staticfloat staticfloat deleted the vs/build-h-lbt branch Feb 28, 2022
@ViralBShah
Copy link
Sponsor Member Author

@ViralBShah ViralBShah commented Feb 28, 2022

@staticfloat How do you feel about also removing BLAS.vendor()? It is already deprecated in 1.8, so should be ok to remove in 1.9.
@staticfloat
Copy link
Sponsor Member

@staticfloat staticfloat commented Feb 28, 2022 •
edited

Sure, seems fine to me. A quick code search doesn't reveal too many uses: https://juliahub.com/ui/Search?q=BLAS.vendor%28%29&type=code
@ViralBShah ViralBShah mentioned this pull request Feb 28, 2022
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra
2 participants
@ViralBShah @staticfloat