Core: Fix the GEBP nr assumption and make SVE compile above 512 bits
Four pre-existing bugs on master, all one class: a hardcoded assumption that some width is 4, 8, or at most 16. Each stayed invisible until a backend went wider than the author assumed.
Stacked on !2933 (merged), which turns on the SVE CI that finds them. Building on that branch is deliberate: the sve128, sve512 and vl2048 jobs that go red there run against these fixes here, so this pipeline going green is the evidence. The diff against master therefore includes !2933 (merged)'s two commits — review from Core: Gate the GEBP swapped kernel... onward. Merge !2933 (merged) first.
nr is not the column-group width
gebp_kernel's leftover-row loop handles the trailing group of four rhs columns, and uses a vectorized swapped kernel only when a sub-packet of exactly four lanes exists to hold them. It tested that against nr — a tunable: EIGEN_NEON_GEBP_NR, EIGEN_SVE_GEBP_NR and EIGEN_LSX_GEBP_NR all default to 8, AVX-512 sets 8 in its own gebp_traits, and TensorFlow builds NEON with -DEIGEN_NEON_GEBP_NR=4. The comment it carried admitted the assumption: "of the same size as nr (which is currently 4)".
It fails in both directions:
- SVE has no narrower packet, so
unpacket_traits<PacketXd>::halfisPacketXditself. At 512 bits the test becomes8 == 8, wrongly passes, and a four-column group runs through an eight-lane sub-packet.triangularandbunchkaufmanabort on a stack-smashing check; products, eigensolvers and decompositions return wrong answers whenever bothrows % 8 >= 4andcols % 8 >= 4. - AVX-512 has real half and quarter packets, so it gets
4 == 8, wrongly fails, and has been silently taking the scalar path this loop exists to avoid.
Comparing against the group width fixes both. The two sub-packet sizes become constexpr, which is what lets the condition move to EIGEN_IF_CONSTEXPR so the discarded arm is no longer instantiated under C++17.
SVE does not compile above 512 bits
Two independent reasons, both in library code, both invisible in CI until now because the build stopped at the first:
TriangularMatrixMatrix.hputs a(2 * max(mr, nr))^2panel on the stack withmr = 3 * PacketSize—9 * VL^2 / 64bytes for float, so 144 kB at 1024 bits and 576 kB at 2048 against a 128 kB default. Raised to the closed form rather than a tier table, so it cannot drift when the blocking constants move and it covers the whole architectural SVE range.loadQuadToDoublePackethad overloads for<= 8and== 16real lanes and nothing above. Sized offNinstead: a real packet of N lanes carries N/8 complex values each spread over eight lanes, andploadquadrepeats every element four times, so it wants N/4 entries with each value listed twice — exactly what the sixteen-lane version wrote out by hand, and identical there.
Only Eigen's own default is raised. EIGEN_STACK_ALLOCATION_LIMIT is a documented user setting that Macros.h defaults under #ifndef, and Eigen/Core includes it first, so by the time the backend block runs the value carries no sign of where it came from. Every explicit value is a caller policy on stack safety, not just the extremes: nomalloc, bdcsvd, jacobisvd, diagonalview and diagonalmatrices set 0 to switch the check off, and a cap chosen to bound stack use is just as deliberate — at 2048 bits an explicit 200000 would become 589824, and Matrix<float, 256, 256> would then compile where the caller asked it not to. So Macros.h records that it supplied the value, in EIGEN_STACK_ALLOCATION_LIMIT_WAS_DEFAULTED, and the SVE block consumes and undefines that marker. A build that asks for less room than the backend needs now fails to compile and names the object, rather than quietly getting more room than it asked for. Guarding on the computed requirement also leaves every vector length up to 512 untouched, which is all shipping hardware.
The RVV block a hundred lines below has the same defect — an unconditional #undef that rewrites every explicit value — but it is pre-existing and untouched here; separate MR.
Test types capped at 16 lanes
vectorization_logic's Matrix1, Matrix1u and Matrix3 shapes come from a ladder over PacketSize that stops at 16 and falls through to the == 1 arm, yielding a 1x1 or 1x3 matrix the test then asserts a vectorized traversal of. Each ladder already spelled out a closed form; written out, the generated types are unchanged at every size the ladders covered.
Verification
Exhaustive sweep — m ≤ 72, n ≤ 24, k ∈ {1,3,8,17,64}, gemm and trmm, float and double, 17288 shapes per configuration:
| backend | result |
|---|---|
| SSE2, AVX2, AVX-512 (C++14 and C++17) | 0 mismatches |
| generic clang backend, 16/32/64-byte vectors | 0 mismatches |
| NEON | 0 mismatches |
| SVE 128 / 256 / 512 | 0 mismatches |
| SVE 1024 / 2048 | 0 mismatches; previously did not build. No hardware implements these, and !2933 (merged) no longer builds them, but the two compile fixes below are what made them build at all |
sve_vector_length now carries the explicit-limit invariant as a static_assert; it fails to compile without the guard above and passes with it. triangular and bunchkaufman go from 0/8 to 8/8 runs passing on SVE at 512 bits. All nine product and triangular test binaries pass on AVX-512. vectorization_logic passes on SSE2, AVX2, AVX-512 and NEON.
Two things worth stating rather than leaving to be discovered:
- The AVX-512 path this newly enables measures within run-to-run noise — the loop covers only
rows % 4rows of four columns. This is a correctness fix, not a speedup. - Two
test_reduxunrolling expectations invectorization_logicstill assume a packet of at most 16 lanes. Fixing them means either mirroring three pieces ofredux_traitsinto the test or dropping the assertion on every backend, so they are left alone — !2933 (merged) builds only the widths hardware implements, none of which reach them.