Skip to content

Commit 3a5a652

Browse files
committed
measured leading dimensions: bound the addressed span, and bound it everywhere
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.
1 parent 25f0b91 commit 3a5a652

3 files changed

Lines changed: 110 additions & 25 deletions

File tree

‎src/TiledArray/math/blas.h‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@
2929
#include <TiledArray/external/eigen.h>
3030
#include <TiledArray/type_traits.h>
3131

32+
#include <blas/config.h> // blas_int
3233
#include <blas/dot.hh>
3334
#include <blas/gemm.hh>
3435
#include <blas/scal.hh>
3536
#include <blas/util.hh>
3637
#include <blas/wrappers.hh>
3738

3839
#include <cstdint>
40+
#include <limits>
3941

4042
namespace TiledArray::math::blas {
4143

@@ -48,6 +50,32 @@ static constexpr auto NoTranspose = Op::NoTrans;
4850
static constexpr auto Transpose = Op::Trans;
4951
static constexpr auto ConjTranspose = Op::ConjTrans;
5052

53+
/// The largest leading dimension the linked BLAS can accept. `integer` is
54+
/// 64-bit here unconditionally, but BLAS++ narrows every dimension to its own
55+
/// `blas_int` -- 32-bit under an LP64 BLAS -- and throws
56+
/// (`blas::Error: ldb, in function to_blas_int_`) past this value. Callers that
57+
/// MEASURE a leading dimension (as a distance between two addresses, rather
58+
/// than deriving it from an extent) have to check it themselves; see
59+
/// ld_fits().
60+
inline constexpr integer max_ld() {
61+
return static_cast<integer>(std::numeric_limits<blas_int>::max());
62+
}
63+
64+
/// True if a row-major matrix of `nslab` rows and `extent` columns at leading
65+
/// dimension `ld` addresses only elements a `blas_int` can index. max_ld()
66+
/// alone bounds ONE row step; the GEMM reaches element
67+
/// (nslab-1)*ld + extent-1, and a 32-bit-indexed BLAS must be able to form
68+
/// THAT index too, so a stride comfortably under the cap still overflows it
69+
/// once there are enough rows. Phrased as a division so the bound itself
70+
/// cannot overflow when `blas_int` is 64-bit (ILP64), where max_ld() is
71+
/// INT64_MAX.
72+
inline constexpr bool ld_fits(integer ld, integer nslab, integer extent) {
73+
if (ld < 0 || extent < 1 || extent - 1 > max_ld() || ld > max_ld())
74+
return false;
75+
if (nslab <= 1) return true;
76+
return ld <= (max_ld() - (extent - 1)) / (nslab - 1);
77+
}
78+
5179
/// converts Op to ints in manner useful for bit manipulations
5280
/// NoTranspose -> 0, Transpose->1, ConjTranspose->2
5381
inline int64_t to_int(Op op) {

‎src/TiledArray/tensor/arena_einsum.h‎

Lines changed: 59 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
#include "TiledArray/tensor/type_traits.h"
1313
#include "TiledArray/util/annotation.h"
1414

15-
#include <blas/config.h> // blas_int
16-
1715
#include <algorithm>
1816
#include <atomic>
1917
#include <chrono>
@@ -22,7 +20,6 @@
2220
#include <cstdlib>
2321
#include <iomanip>
2422
#include <iostream>
25-
#include <limits>
2623
#include <map>
2724
#include <mutex>
2825
#include <optional>
@@ -282,32 +279,38 @@ inline void phase_stop(std::atomic<std::uint64_t>& acc,
282279
/// 2 = nonuniform: present, but inner-cell sizes differ along the run
283280
/// 3 = stride : present + uniform, but cells are NOT at a constant
284281
/// page-jump-free stride (the strided-GEMM precondition)
282+
/// 4 = ld range : constant stride, but the run spans more elements than the
283+
/// BLAS integer can index (see ld_fits)
285284
/// 0 = run looks clean (so the rejection came from the OTHER operand run)
286285
inline std::atomic<std::uint64_t> g_fall_runs_ce_ce{0};
287286
inline std::atomic<std::uint64_t> g_fall_res_absent_ce_ce{0}; // 1
288287
inline std::atomic<std::uint64_t> g_fall_res_nonuniform_ce_ce{0}; // 2
289288
inline std::atomic<std::uint64_t> g_fall_res_stride_ce_ce{0}; // 3
289+
inline std::atomic<std::uint64_t> g_fall_res_ldrange_ce_ce{0}; // 4
290290
inline std::atomic<std::uint64_t> g_fall_op_absent_ce_ce{0}; // 13
291291
inline std::atomic<std::uint64_t> g_fall_op_nonuniform_ce_ce{0}; // 14
292292
inline std::atomic<std::uint64_t> g_fall_op_stride_ce_ce{0}; // 15
293293
inline std::atomic<std::uint64_t> g_fall_op_acrossk_ce_ce{0}; // 16
294294
inline std::atomic<std::uint64_t> g_fall_both_clean_ce_ce{0}; // 17
295-
296-
/// The largest cell-to-cell stride the linked BLAS accepts as a leading
297-
/// dimension. The strided kernels below MEASURE their leading dimensions as
298-
/// pointer differences between neighbouring cells; blaspp converts every ld to
299-
/// blas_int (32-bit under LP64) and throws (`blas::Error: ldb, in function
300-
/// to_blas_int_`) past its maximum. Two present cells that are not arena
301-
/// neighbours -- e.g. individually allocated inner tensors that the allocator
302-
/// placed gigabytes apart -- satisfy every other precondition of a 2-cell run
303-
/// (uniform size, stride >= cell size, trivially constant) yet yield an
304-
/// address delta no BLAS can take. Such a run is not strided-GEMM material:
305-
/// the walkers break it up and the per-cell path handles it.
306-
inline constexpr long max_blas_ld() {
307-
return static_cast<long>(std::numeric_limits<blas_int>::max());
308-
}
309-
310-
/// Classify a strided run: 1=absent, 2=nonuniform size, 3=bad stride, 0=clean.
295+
inline std::atomic<std::uint64_t> g_fall_op_ldrange_ce_ce{0}; // 18
296+
297+
/// Does a measured strided run address only elements the BLAS can index?
298+
/// The strided kernels below MEASURE their leading dimensions as pointer
299+
/// differences between neighbouring cells, so -- unlike a kernel that derives
300+
/// them from extents -- they can hand blaspp an ld it cannot convert to
301+
/// `blas_int` (`blas::Error: ldb, in function to_blas_int_`). Two present
302+
/// cells that are not arena neighbours -- e.g. individually allocated inner
303+
/// tensors the allocator placed gigabytes apart -- satisfy every other
304+
/// precondition of a 2-cell run (uniform size, stride >= cell size, trivially
305+
/// constant) yet yield exactly such an address delta; and a stride well inside
306+
/// `blas_int` still overflows the BLAS index once the run is long enough,
307+
/// because the GEMM reaches element (nslab-1)*ld + extent-1. Such a run is not
308+
/// strided-GEMM material: the walkers break it up and the per-cell path
309+
/// handles it.
310+
using math::blas::ld_fits;
311+
312+
/// Classify a strided run: 1=absent, 2=nonuniform size, 3=bad stride,
313+
/// 4=stride past the BLAS index range (see ld_fits), 0=clean.
311314
template <typename GetCell>
312315
inline int classify_run(GetCell getcell, std::size_t n) {
313316
if (n == 0) return 0;
@@ -330,14 +333,17 @@ inline int classify_run(GetCell getcell, std::size_t n) {
330333
for (std::size_t i = 0; i < n; ++i)
331334
if (getcell(i).data() != base + static_cast<std::ptrdiff_t>(i) * st)
332335
return 3; // non-constant stride
336+
// ...and the whole run must stay inside the BLAS index range
337+
if (!ld_fits(st, static_cast<long>(n), s0)) return 4;
333338
return 0;
334339
}
335340

336341
/// Diagnose the OPERAND side (called when the result run is clean). getR(k,i)
337342
/// is the strided operand run (length `nrun`, per outer-contraction k); getL(k)
338343
/// is the per-k single (non-strided) operand cell, expected size P*Q. Returns
339344
/// 13=absent/size, 14=nonuniform, 15=bad stride within a k, 16=stride varies
340-
/// across k, 17=clean (gate rejected a run this re-check finds valid).
345+
/// across k, 18=stride past the BLAS index range (see ld_fits), 17=clean (gate
346+
/// rejected a run this re-check finds valid).
341347
template <typename GetR, typename GetL>
342348
inline int classify_operand(GetR getR, GetL getL, std::size_t nrun,
343349
std::size_t nK, long P) {
@@ -369,6 +375,7 @@ inline int classify_operand(GetR getR, GetL getL, std::size_t nrun,
369375
for (std::size_t i = 0; i < nrun; ++i)
370376
if (getR(k, i).data() != base + static_cast<std::ptrdiff_t>(i) * sk)
371377
return 15;
378+
if (!ld_fits(sk, static_cast<long>(nrun), Q)) return 18;
372379
if (k == 0)
373380
sR = sk;
374381
else if (sk != sR)
@@ -520,10 +527,12 @@ inline void measure_segments(GetC getC, GetR getR, GetL getL, std::size_t nrun,
520527
sC = dc;
521528
sR = dr;
522529
if (sC < P || sR < Q) break;
523-
if (sC > max_blas_ld() || sR > max_blas_ld()) break;
524530
} else if (dc != off * sC || dr != off * sR) {
525531
break;
526532
}
533+
// admitting this cell makes the segment off+1 slabs long; the GEMM
534+
// would then reach element off*sC+P-1 of C and off*sR+Q-1 of R.
535+
if (!ld_fits(sC, off + 1, P) || !ld_fits(sR, off + 1, Q)) break;
527536
++end;
528537
}
529538
const std::size_t len = end - mu;
@@ -546,10 +555,12 @@ inline void record_ce_ce_fallback(int why) {
546555
case 1: g_fall_res_absent_ce_ce.fetch_add(1, std::memory_order_relaxed); break;
547556
case 2: g_fall_res_nonuniform_ce_ce.fetch_add(1, std::memory_order_relaxed); break;
548557
case 3: g_fall_res_stride_ce_ce.fetch_add(1, std::memory_order_relaxed); break;
558+
case 4: g_fall_res_ldrange_ce_ce.fetch_add(1, std::memory_order_relaxed); break;
549559
case 13: g_fall_op_absent_ce_ce.fetch_add(1, std::memory_order_relaxed); break;
550560
case 14: g_fall_op_nonuniform_ce_ce.fetch_add(1, std::memory_order_relaxed); break;
551561
case 15: g_fall_op_stride_ce_ce.fetch_add(1, std::memory_order_relaxed); break;
552562
case 16: g_fall_op_acrossk_ce_ce.fetch_add(1, std::memory_order_relaxed); break;
563+
case 18: g_fall_op_ldrange_ce_ce.fetch_add(1, std::memory_order_relaxed); break;
553564
default: g_fall_both_clean_ce_ce.fetch_add(1, std::memory_order_relaxed); break;
554565
}
555566
}
@@ -558,17 +569,20 @@ inline void record_ce_ce_fallback(int why) {
558569
// ce+e phase timers + fallback diagnosis (mirror of the ce+ce instrumentation).
559570
// In ce+e a "run" is one result cell (m,n); the clean check is over the k-slabs
560571
// of L (stride ldA) and R (stride ldB). Fallback reasons classify those k-runs:
561-
// L-run: 1 absent / 2 nonuniform / 3 stride; R-run: 11 / 12 / 13; 17 clean.
572+
// L-run: 1 absent / 2 nonuniform / 3 stride / 4 ld range;
573+
// R-run: 11 / 12 / 13 / 14; 17 clean.
562574
inline std::atomic<std::uint64_t> g_kernel_ns_ce_e{0};
563575
inline std::atomic<std::uint64_t> g_check_ns_ce_e{0};
564576
inline std::atomic<std::uint64_t> g_fallback_ns_ce_e{0};
565577
inline std::atomic<std::uint64_t> g_e_fall_runs{0};
566578
inline std::atomic<std::uint64_t> g_e_l_absent{0};
567579
inline std::atomic<std::uint64_t> g_e_l_nonuniform{0};
568580
inline std::atomic<std::uint64_t> g_e_l_stride{0};
581+
inline std::atomic<std::uint64_t> g_e_l_ldrange{0};
569582
inline std::atomic<std::uint64_t> g_e_r_absent{0};
570583
inline std::atomic<std::uint64_t> g_e_r_nonuniform{0};
571584
inline std::atomic<std::uint64_t> g_e_r_stride{0};
585+
inline std::atomic<std::uint64_t> g_e_r_ldrange{0};
572586
inline std::atomic<std::uint64_t> g_e_both_clean{0};
573587

574588
inline void record_ce_e_fallback(int why) {
@@ -578,9 +592,11 @@ inline void record_ce_e_fallback(int why) {
578592
case 1: g_e_l_absent.fetch_add(1, std::memory_order_relaxed); break;
579593
case 2: g_e_l_nonuniform.fetch_add(1, std::memory_order_relaxed); break;
580594
case 3: g_e_l_stride.fetch_add(1, std::memory_order_relaxed); break;
595+
case 4: g_e_l_ldrange.fetch_add(1, std::memory_order_relaxed); break;
581596
case 11: g_e_r_absent.fetch_add(1, std::memory_order_relaxed); break;
582597
case 12: g_e_r_nonuniform.fetch_add(1, std::memory_order_relaxed); break;
583598
case 13: g_e_r_stride.fetch_add(1, std::memory_order_relaxed); break;
599+
case 14: g_e_r_ldrange.fetch_add(1, std::memory_order_relaxed); break;
584600
default: g_e_both_clean.fetch_add(1, std::memory_order_relaxed); break;
585601
}
586602
}
@@ -677,12 +693,16 @@ struct GemmTimingDumper {
677693
<< " (" << fp(L(g_e_l_nonuniform)) << "%)\n";
678694
std::cerr << "[ce+e-fallback] L-run bad stride : " << L(g_e_l_stride)
679695
<< " (" << fp(L(g_e_l_stride)) << "%)\n";
696+
std::cerr << "[ce+e-fallback] L-run ld range : " << L(g_e_l_ldrange)
697+
<< " (" << fp(L(g_e_l_ldrange)) << "%)\n";
680698
std::cerr << "[ce+e-fallback] R-run absent : " << L(g_e_r_absent)
681699
<< " (" << fp(L(g_e_r_absent)) << "%)\n";
682700
std::cerr << "[ce+e-fallback] R-run nonuniform : " << L(g_e_r_nonuniform)
683701
<< " (" << fp(L(g_e_r_nonuniform)) << "%)\n";
684702
std::cerr << "[ce+e-fallback] R-run bad stride : " << L(g_e_r_stride)
685703
<< " (" << fp(L(g_e_r_stride)) << "%)\n";
704+
std::cerr << "[ce+e-fallback] R-run ld range : " << L(g_e_r_ldrange)
705+
<< " (" << fp(L(g_e_r_ldrange)) << "%)\n";
686706
std::cerr << "[ce+e-fallback] both runs clean : " << L(g_e_both_clean)
687707
<< " (" << fp(L(g_e_both_clean)) << "%)\n";
688708
}
@@ -701,6 +721,9 @@ struct GemmTimingDumper {
701721
std::cerr << "[ce+ce-fallback] result bad stride : "
702722
<< L(g_fall_res_stride_ce_ce) << " ("
703723
<< fp(L(g_fall_res_stride_ce_ce)) << "%)\n";
724+
std::cerr << "[ce+ce-fallback] result ld range : "
725+
<< L(g_fall_res_ldrange_ce_ce) << " ("
726+
<< fp(L(g_fall_res_ldrange_ce_ce)) << "%)\n";
704727
std::cerr << "[ce+ce-fallback] operand absent/size : "
705728
<< L(g_fall_op_absent_ce_ce) << " ("
706729
<< fp(L(g_fall_op_absent_ce_ce)) << "%)\n";
@@ -713,6 +736,9 @@ struct GemmTimingDumper {
713736
std::cerr << "[ce+ce-fallback] operand stride X k : "
714737
<< L(g_fall_op_acrossk_ce_ce) << " ("
715738
<< fp(L(g_fall_op_acrossk_ce_ce)) << "%)\n";
739+
std::cerr << "[ce+ce-fallback] operand ld range : "
740+
<< L(g_fall_op_ldrange_ce_ce) << " ("
741+
<< fp(L(g_fall_op_ldrange_ce_ce)) << "%)\n";
716742
std::cerr << "[ce+ce-fallback] both runs clean (!) : "
717743
<< L(g_fall_both_clean_ce_ce) << " ("
718744
<< fp(L(g_fall_both_clean_ce_ce)) << "%)\n";
@@ -1267,7 +1293,9 @@ void arena_strided_gemm_ce_e(ResultOuter& C, const LeftOuter& L,
12671293
ldA = static_cast<long>(lc[lbase + a_off(m, 1)].data() - l0.data());
12681294
ldB = static_cast<long>(rc[rbase + b_off(1, n)].data() - r0.data());
12691295
if (ldA < P || ldB < Q) clean = false;
1270-
if (ldA > max_blas_ld() || ldB > max_blas_ld()) clean = false;
1296+
if (!ld_fits(ldA, static_cast<long>(K), P) ||
1297+
!ld_fits(ldB, static_cast<long>(K), Q))
1298+
clean = false;
12711299
for (std::size_t k = 0; clean && k < K; ++k) {
12721300
if (lc[lbase + a_off(m, k)].data() !=
12731301
l0.data() + static_cast<std::ptrdiff_t>(k) * ldA)
@@ -1569,10 +1597,13 @@ void arena_strided_gemm_ce_ce_right(ResultOuter& C, const LeftOuter& L,
15691597
sR = dR;
15701598
sC = dC;
15711599
if (sR < Q || sC < P) break; // page-jump / overlap
1572-
if (sR > max_blas_ld() || sC > max_blas_ld()) break;
15731600
} else if (dR != off * sR || dC != off * sC) {
15741601
break;
15751602
}
1603+
// admitting this cell makes the segment off+1 slabs long; the
1604+
// GEMM would then reach element off*sR+Q-1 of A and off*sC+P-1
1605+
// of C.
1606+
if (!ld_fits(sR, off + 1, Q) || !ld_fits(sC, off + 1, P)) break;
15761607
++end;
15771608
}
15781609
const std::size_t Mseg = end - mu;
@@ -1813,10 +1844,13 @@ void arena_strided_gemm_ce_ce_left(ResultOuter& C, const LeftOuter& L,
18131844
sA = dA;
18141845
sC = dC;
18151846
if (sA < Q || sC < P) break; // page-jump / overlap
1816-
if (sA > max_blas_ld() || sC > max_blas_ld()) break;
18171847
} else if (dA != off * sA || dC != off * sC) {
18181848
break;
18191849
}
1850+
// admitting this cell makes the segment off+1 slabs long; the
1851+
// GEMM would then reach element off*sA+Q-1 of A and off*sC+P-1
1852+
// of C.
1853+
if (!ld_fits(sA, off + 1, Q) || !ld_fits(sC, off + 1, P)) break;
18201854
++end;
18211855
}
18221856
const std::size_t Mseg = end - m;

‎src/TiledArray/tensor/tensor.h‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3704,6 +3704,20 @@ class Tensor {
37043704
if (N > 1)
37053705
ldc = static_cast<integer>(rc0[1].data() - rc0[0].data());
37063706
if (ldb < A || ldc < A) clean = false; // sanity
3707+
// ...and the strides must be indexable by the BLAS. ldb/ldc
3708+
// are MEASURED (a distance between two cell addresses), not
3709+
// derived from an extent, so an un-compacted ToT tile whose
3710+
// cells the allocator placed far apart can produce one that
3711+
// BLAS++ cannot narrow to blas_int -- `blas::Error: ldb, in
3712+
// function to_blas_int_` under LP64. The gemm below reads
3713+
// element (K-1)*ldb*cw + A*cw-1 of the left slab and writes
3714+
// (N-1)*ldc*cw + A*cw-1 of the result slab, so bound the whole
3715+
// span, not just one step.
3716+
if (!TiledArray::math::blas::ld_fits(
3717+
ldb * cw, K, static_cast<integer>(A) * cw) ||
3718+
!TiledArray::math::blas::ld_fits(
3719+
ldc * cw, N, static_cast<integer>(A) * cw))
3720+
clean = false;
37073721
const std::ptrdiff_t sb = ldb, sc = ldc;
37083722
for (integer k = 0; clean && k != K; ++k)
37093723
if (lc(k).data() != lc(0).data() + k * sb) clean = false;
@@ -3903,6 +3917,15 @@ class Tensor {
39033917
ldc = static_cast<integer>(this_data[N + n].data() -
39043918
this_data[n].data());
39053919
if (sbc < A || ldc < A) clean = false;
3920+
// measured strides, so bound their BLAS-indexable span too
3921+
// (see the ToT x scalar mirror above): the gemm reads element
3922+
// (K-1)*sbc*cw + A*cw-1 of the right slab and writes
3923+
// (M-1)*ldc*cw + A*cw-1 of the result slab.
3924+
if (!TiledArray::math::blas::ld_fits(
3925+
sbc * cw, K, static_cast<integer>(A) * cw) ||
3926+
!TiledArray::math::blas::ld_fits(
3927+
ldc * cw, M, static_cast<integer>(A) * cw))
3928+
clean = false;
39063929
const std::ptrdiff_t sb = sbc, sc = ldc;
39073930
for (integer k = 0; clean && k != K; ++k)
39083931
if (right_data[rcell(k, n)].data() !=

0 commit comments

Comments
 (0)