arena strided kernels: a measured cell stride must fit the BLAS integer - #589
Draft
kshitij-05 wants to merge 2 commits into
Draft
kshitij-05 wants to merge 2 commits into
kshitij-05 wants to merge 2 commits into
Conversation
The ce+e / ce+ce strided-GEMM kernels take their leading dimensions from pointer differences between neighbouring inner cells. Two present cells that are not arena neighbours (individually allocated inner tensors placed gigabytes apart) pass every other precondition of a 2-cell run -- uniform size, stride >= cell size, trivially constant -- and hand blaspp an ld it cannot convert under LP64: `blas::Error: ldb, in function to_blas_int_` (uranyl/dyall-v3z PNS-MP1 union path, tpns <= 1e-7, MKL LP64). Such a run is not strided-GEMM material: break the segment (or mark the k-run unclean) when a stride exceeds numeric_limits<blas_int>::max(), and let the per-cell path handle it.
…verywhere Three gaps in the previous commit's guard. Scope. Tensor::gemm's two arena "scale" strided paths measure their leading dimensions the same way the arena kernels do -- lc(1).data() - lc(0).data() for the ToT x plain-scalar row slab, right_data[rcell(1,n)].data() - right_data[rcell(0,n)].data() for the plain-scalar x ToT column slab -- into math::blas::integer, which is 64-bit, so nothing truncates and the value reaches blaspp unchecked. The `ld < A` test rejects only negative/overlapping strides, and with K == 2 (or N == 2, M == 2) the constant-stride verification loop is satisfied by the very pair the stride was measured from. The same un-compacted ToT tile that motivated the arena fix therefore still throws `blas::Error: ldb, in function to_blas_int_` through those two paths. Bound. A leading dimension is not the largest index the GEMM forms: it reaches element (nslab-1)*ld + extent-1. blaspp's to_blas_int_ checks each argument in isolation and never that span, so a stride comfortably under numeric_limits<blas_int>::max() overflows a 32-bit-indexed BLAS once the run is long enough -- wrong results or a segfault inside the BLAS rather than a clean throw. Since the motivating workload demonstrably produces strides above the cap, strides just below it are equally reachable there. Home. The cap is a property of the BLAS binding, not of arena einsum, and tensor.h cannot include arena_einsum.h. Both now share math::blas::max_ld() / ld_fits(ld, nslab, extent) in math/blas.h. ld_fits is phrased as a division so the bound itself cannot overflow under ILP64, where blas_int is 64-bit and max_ld() is INT64_MAX (every check then correctly degenerates to a no-op). Typed math::blas::integer -- what each ld is cast to at the call -- rather than long, which is only accidentally 64-bit here. In the two ce+ce segment walkers the check moves out of the `off == 1` branch so it re-evaluates as the segment grows; a segment can then only come out shorter, never longer, and the remainder resumes at the next cell as before. Finally, classify_run / classify_operand still scored an oversized stride as clean, so a run rejected solely by the new gate was re-diagnosed as 17 and counted in g_e_both_clean / g_fall_both_clean_ce_ce -- the bucket documented as "gate rejected a run this re-check finds valid", i.e. the signal for a gate bug. They gain reason codes 4 and 18 with their own counters and dump lines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The ce+e / ce+ce strided-GEMM kernels in
arena_einsum.htake their leading dimensions from pointer differences between neighbouring inner cells. Two present cells that are not arena neighbours (individually allocated inner tensors placed gigabytes apart) pass every other precondition of a 2-cell run (uniform size, stride >= cell size, trivially constant) and hand blaspp a leading dimension that does not fit LP64:blas::Error: ldb, in function to_blas_int_(uranyl/dyall-v3z PNS-MP1, MKL LP64).Fix: a stride above
numeric_limits<blas_int>::max()breaks the segment (or marks the k-run unclean) so the per-cell path handles those cells. Arena-contiguous cells are unaffected.