Amend matrix * vector specialization for strided arrays

[ararslan]

This method entirely duplicates the one below it with the exception that it also does a convert on one of the inputs with the result of promote_op, which doesn't make a whole lot of sense in some cases. By virtue of calling mul!, strided arrays that go through the abstract array method will still hit the same optimized methods that use BLAS.

Fixes JuliaLang/LinearAlgebra.jl#632.

It would be great if this change could be backported, since it fixes an issue in private code that occurs on every version >= 1.1.0. Help coming up with a minimal test case for this would be much appreciated as well.

[ararslan]

@nanosoldier runbenchmarks("linalg", vs=":master")

[nanosoldier]

Your benchmark job has completed - no performance regressions were detected. A full report can be found here. cc @ararslan

[ararslan]

Marking for triage because I'd like to know:

• Can this make it into 1.2?
• Can this be backported to 1.0 and/or 1.1?
• Can anyone help me come up with a good test? 😬

[mbauman]

strided arrays that go through the abstract array method will still hit the same optimized methods that use BLAS

I don't think that's the case. For example, given rand(5, 5) * rand(1:10, 5), we previously would allocate a Float64 result, convert the vector to Float64s, and then call mul!(::Vector{Float64}, ::Matrix{Float64}, ::Vector{Float64}), which dispatches to gemv!.

Without this method, we just allocate a Float64 result and then call mul!(::Vector{Float64}, ::Matrix{Float64}, ::Vector{Int})... which dispatches to generic_matvecmul!.

This conversion is there for a reason. Now, there's probably a better way to do what it's trying to do, but I don't think this is it.

[ararslan]

Can we restrict the input vector eltype to Real then, perhaps? That would fix my use case, at least.

[mbauman]

Or how about only doing the conversion if Base.isconcretetype(TS)? If it's abstract, we're definitely not going to be hitting BLAS. That should also fix your case and probably gains us a bit more generality (there could be Reals like yours, too).

Edit: or both?

[ararslan]

Implemented that. Still needs tests though.

[JeffBezanson]

Triage: seems safe to backport.

[ararslan]

I was going to add a test before this was merged, but ¯\_(ツ)_/¯

[mbauman]

Would still be nice to do separately.

[ararslan]

The test should probably be backported with the fix itself, I would think. But yeah, can do.

[ararslan]

Test added in #32124.