Skip to content

Fix BLAS calls on views oneMKL can't address - #724

Merged
maleadt merged 3 commits into
mainfrom
tb/blas-views
Oct 20, 2026
Merged

maleadt merged 3 commits into
mainfrom
tb/blas-views

Conversation

@maleadt

@maleadt maleadt commented Oct 19, 2026

Copy link
Copy Markdown
Member

The BLAS-1 methods (norm, dot, BLAS.asum, axpy!, axpby!, rmul!, rotate!, reflect!) and the storage-level mul! and triangular methods pass any strided view to oneMKL with incx = stride(x, 1) and lda = max(1, stride(A, 2)), without checking that oneMKL can address that layout. Reversed views get a negative increment, which BLAS walks backwards from the lowest address while the view's pointer is at its highest element, so the result is silently wrong. Views that skip rows get passed as length(x) elements with a single increment, which reads and writes outside the view. Reversed columns give a negative lda, and oneMKL's invalid_argument escapes the support library's C interface and aborts the process:

x = oneArray(Float32[3, 4]); r = view(x, 2:-1:1)
norm(r)                    # 4.0, expected 5.0
rmul!(r, 2f0); x           # [3, 8], expected [6, 8]
mul!(C, 'N', 'N', view(A, :, 4:-1:1), B, true, false)
# terminate called after throwing an instance of 'oneapi::mkl::invalid_argument'

The triangular case is reachable through public API, e.g. lmul!(UpperTriangular(A), view(x, n:-1:1)).

This ports JuliaGPU/CUDA.jl#3411 and JuliaGPU/CUDA.jl#3409. The level-1 wrappers now take strided vectors or dense arrays only, and pass a pointer to the lowest address together with the signed increment, like LinearAlgebra's vec_pointer_stride; nrm2, asum, scal, iamax and iamin use the absolute increment since the order doesn't matter (oneMKL rejects a negative one for iamax/iamin, so their index is flipped back). The LinearAlgebra BLAS-1 methods fall back to broadcasts for strided arrays that aren't vectors or dense. Storage-level mul! only calls oneMKL when every matrix has a unit row stride and a leading dimension of at least its row count, and every vector a positive stride, falling back to GPUArrays.generic_matmatmul! otherwise as it already did for unsupported element types. The triangular methods run on contiguous copies and copy the result back.

Not covered: the level-2 wrappers (gemv!, symv!, ...) still take raw strides when called directly, as in CUDA.jl, and the support library should turn oneapi::mkl::exception into an error code instead of aborting, which needs a JLL rebuild.

Tested on an Iris Xe (Julia 1.12): the reproducers from both issues fail on main and pass here, and the new tests cover reversed and row-skipping views for every BLAS-1 method, the storage-level mul! paths (gemm, gemv, symm, hemm, symv, hemv) and the triangular products and solves.

Fixes #712
Fixes #700

The BLAS-1 wrappers take any strided array and pass it to oneMKL as
`length(x)` elements with increment `stride(x, 1)`. For a view of a
matrix that skips rows that layout is wrong, so `norm`, `rmul!`,
`axpy!`, `axpby!`, `rotate!` and `reflect!` returned wrong results and
wrote outside the view. Limit the wrappers to strided vectors and
contiguous arrays, and have the LinearAlgebra methods fall back to
broadcasts over the flattened arrays for anything else, since BLAS
pairs elements linearly regardless of shape.

The ComplexF16 variants of `scal!` and `axpy!` compute in ComplexF32
and wrote the result back through `convert`, which only works for a
dense array; use a broadcast instead. `dotc` and `dotu` likewise widen
with a broadcast now, so they accept views.
oneMKL follows BLAS and walks a vector with a negative increment
backwards from its lowest address, while the wrappers passed the
pointer of a reversed view's first element, the highest one. As a result
`norm`, `dot`, `BLAS.asum`, `axpy!`, `axpby!`, `rmul!`, `rotate!` and
`reflect!` on `view(x, n:-1:1)` returned wrong results or accessed the
wrong memory. Pass a pointer to the lowest address together with the
increment, like LinearAlgebra's `vec_pointer_stride`, and use the
absolute increment for nrm2, asum and scal, where the order doesn't
matter. iamax and iamin report the first of several extrema, so they
run on a contiguous copy of a reversed vector instead. This also fixes
direct calls like `oneMKL.nrm2(r)`.

Fixes #712
…thods

The storage-level `mul!` methods passed any strided `SubArray` to
oneMKL with `lda = max(1, stride(A, 2))` and `incx = stride(x, 1)`,
and the triangular `lmul!`/`ldiv!`/`rmul!`/`rdiv!` methods checked
nothing. Views that skip rows therefore gave silently wrong results,
reversed vectors were walked from the wrong end, and reversed columns
made oneMKL throw `invalid_argument` for `lda`, which escapes the
support library's C interface and aborts the process. The triangular
methods were reachable through public API, e.g.
`lmul!(UpperTriangular(A), view(x, n:-1:1))`.

Matrices, and vectors passed as matrices, now need a leading dimension
of at least the number of rows, and vectors a positive stride. `mul!`
falls back to `GPUArrays.generic_matmatmul!` otherwise, as it already
did for other unsupported layouts. The matrix-vector `mul!` now also
accepts a strided output vector, which the layout check keeps away
from oneMKL when needed. The triangular methods run on contiguous
copies and copy the result back.

Fixes #700
@maleadt maleadt added the libraries Vendor library wrappers. label Oct 19, 2026
@codecov

codecov Bot commented Oct 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.91667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.98%. Comparing base (a0adaa1) to head (804dd6b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lib/mkl/linalg.jl 97.56% 1 Missing ⚠️
lib/mkl/wrappers_blas.jl 98.18% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #724      +/-   ##
==========================================
+ Coverage   83.13%   83.98%   +0.85%     
==========================================
  Files          52       51       -1     
  Lines        4376     4322      -54     
==========================================
- Hits         3638     3630       -8     
+ Misses        738      692      -46     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@maleadt
maleadt merged commit c140928 into main Oct 20, 2026
5 checks passed
@maleadt
maleadt deleted the tb/blas-views branch October 20, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libraries Vendor library wrappers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BLAS-1 methods pass reversed views to oneMKL with a negative increment Storage-level mul! gives wrong results or aborts for SubArray operands

1 participant