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

Type instability and allocations in mul! with Hermitians and Adjoint arguments #53951

Closed
franckgaga opened this issue Apr 4, 2024 · 1 comment · Fixed by #54303
Closed

Type instability and allocations in mul! with Hermitians and Adjoint arguments #53951

franckgaga opened this issue Apr 4, 2024 · 1 comment · Fixed by #54303

Comments

@franckgaga
Copy link

franckgaga commented Apr 4, 2024 •
edited

I notice unusual allocations with mul!. If I do:

using LinearAlgebra
using BenchmarkTools
A = Hermitian([1.0 2.0; 2.0 3.0])
B = [4.0 5.0; 6.0 7.0]
Y = similar(B)
@btime mul!($Y, $A, $B)

there is no allocation as expected:

  59.582 ns (0 allocations: 0 bytes)

But with the adjoint matrix:

Badj = B'
@btime mul!($Y, $A, $Badj)

it gives 4 allocations:

  171.889 ns (4 allocations: 20.55 KiB)

It seems to be causes by an internal type instability:

using JET
@report_opt mul!(Y, A, Badj)

the output is very long but there are runtime dispatches detected at the beginning:

═════ 202 possible errors found ═════
┌ mul!(C::Matrix{Float64}, A::Hermitian{Float64, Matrix{Float64}}, B::Adjoint{Float64, Matrix{Float64}}) @ LinearAlgebra /cache/build/builder-amdci5-1/julialang/julia-release-1-dot-10/usr/share/julia/stdlib/v1.10/LinearAlgebra/src/matmul.jl:237
│┌ mul!(C::Matrix{Float64}, A::Hermitian{Float64, Matrix{Float64}}, B::Adjoint{Float64, Matrix{Float64}}, α::Bool, β::Bool) @ LinearAlgebra /cache/build/builder-amdci5-1/julialang/julia-release-1-dot-10/usr/share/julia/stdlib/v1.10/LinearAlgebra/src/matmul.jl:263
││┌ generic_matmatmul!(C::Matrix{Float64}, tA::Char, tB::Char, A::Matrix{Float64}, B::Matrix{Float64}, _add::LinearAlgebra.MulAddMul{true, true, Bool, Bool}) @ LinearAlgebra /cache/build/builder-amdci5-1/julialang/julia-release-1-dot-10/usr/share/julia/stdlib/v1.10/LinearAlgebra/src/matmul.jl:367
│││┌ _generic_matmatmul!(C::Matrix{Float64}, tA::Char, tB::Char, A::AbstractVecOrMat{T}, B::Transpose{Float64, Matrix{Float64}}, _add::LinearAlgebra.MulAddMul{true, true, Bool, Bool}) where T @ LinearAlgebra /cache/build/builder-amdci5-1/julialang/julia-release-1-dot-10/usr/share/julia/stdlib/v1.10/LinearAlgebra/src/matmul.jl:789
││││┌ require_one_based_indexing(::Matrix{Float64}, ::AbstractVecOrMat, ::Transpose{Float64, Matrix{Float64}}) @ Base ./abstractarray.jl:131
│││││ runtime dispatch detected: Base.has_offset_axes(%1::Matrix{Float64}, %2::AbstractVecOrMat, %3::Transpose{Float64, Matrix{Float64}})::Any
││││└────────────────────
││││┌ require_one_based_indexing(::Matrix{Float64}, ::AbstractVecOrMat, ::Transpose{Float64, Matrix{Float64}}) @ Base ./abstractarray.jl:131
│││││ runtime dispatch detected: !(%4::Any)::Any
││││└────────────────────
│││┌ _generic_matmatmul!(C::Matrix{Float64}, tA::Char, tB::Char, A::AbstractVecOrMat{T}, B::Transpose{Float64, Matrix{Float64}}, _add::LinearAlgebra.MulAddMul{true, true, Bool, Bool}) where T @ LinearAlgebra /cache/build/builder-amdci5-1/julialang/julia-release-1-dot-10/usr/share/julia/stdlib/v1.10/LinearAlgebra/src/matmul.jl:806

<...>

The macro @code_warntype does not detect the instability. There is no instability reported with @report_opt mul!(Y, A, B). My setup is:

  1. The output of versioninfo():
julia> versioninfo()
Julia Version 1.10.2
Commit bd47eca2c8a (2024-03-01 10:14 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 16 × 12th Gen Intel(R) Core(TM) i7-1270P
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, alderlake)
Threads: 1 default, 0 interactive, 1 GC (on 16 virtual cores)
  1. How you installed Julia: with juliaup
@jishnub
Copy link
Contributor

jishnub commented Apr 5, 2024 •
edited

It looks like a constant-propagation failure in generic_matmatmul! (despite the aggressive constprop annotations)

The constprop fails because of the branches in

julia/stdlib/LinearAlgebra/src/LinearAlgebra.jl

Lines 523 to 525 in 6023ad6

wrapper_char(A::Hermitian) = A.uplo == 'U' ? 'H' : 'h'
wrapper_char(A::Hermitian{<:Real}) = A.uplo == 'U' ? 'S' : 's'
wrapper_char(A::Symmetric) = A.uplo == 'U' ? 'S' : 's'

@jishnub jishnub mentioned this issue Apr 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

LinearAlgebra: improve type-inference in Symmetric/Hermitian matmul JuliaLang/julia
2 participants
@jishnub @franckgaga