Skip to content

Fix CORTEXA72 Level-3 correctness (SYMM/TRMM/SYRK) - #6002

Open
hugomeiland wants to merge 6 commits into
OpenMathLib:developfrom
hugomeiland:cortexa72-l3-fix
Open

Fix CORTEXA72 Level-3 correctness (SYMM/TRMM/SYRK)#6002
hugomeiland wants to merge 6 commits into
OpenMathLib:developfrom
hugomeiland:cortexa72-l3-fix

Conversation

@hugomeiland

@hugomeiland hugomeiland commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #5997 — after #5970, TARGET=CORTEXA72 DGEMM 6×8 still produced wrong DSYMM / DTRMM / DSYRK / DSYR2K on real Cortex-A72 (Pi 400). DGEMM/TRSM were already addressed; these cousins share the GEMM unroll and were left inconsistent.

Root causes

  1. DSYMMgeneric/symm_{l,u}copy_6.c remainders used (n - n/6) & {4,2,1}, which is wrong for non-power-of-two width 6 (e.g. n=7 → treats remainder as 4). trmm_*copy_6.c already used n % 6.
  2. DTRMMKERNEL.CORTEXA72 selected trmmkernel_2x2.c while packs are contiguous MR=6 / NR=8.
  3. DSYRK / DSYR2K — with UNROLL_M=6, UNROLL_N=8, default UNROLL_MN = MAX(6,8) = 8 makes every SYRK diagonal tile an 8×8 GEMM against 6-wide A packs (always 6+2 edges). Set DGEMM_DEFAULT_UNROLL_MN 6 so diagonal stepping matches MR=6. Thread width math uses (mask+1) with mask=MN-1 (works for non-PoT); syr2k_kernel must not use ~(MN-1) bit clearing.

Changes

  • symm_lcopy_6.c / symm_ucopy_6.c: remainder → n % 6
  • New generic/trmmkernel_6x8.c (offset-aware; M rem 6→4→2→1, N rem 8→4→2→1) wired in KERNEL.CORTEXA72
  • param.h (CORTEXA72): #define DGEMM_DEFAULT_UNROLL_MN 6
  • syr2k_kernel.c: division form for mm stepping (same idea as syrk_kernel.c)

DGEMM 6×8 ukernel / panel packers unchanged (HPL path kept).

Test plan

  • Local smoke build: make TARGET=CORTEXA72 USE_THREAD=0 NO_FORTRAN=1 NO_LAPACK=1 libs (arm64)
  • Real A72 / Graviton1: cblas Level-3 for DSYMM, DTRMM, DSYRK, DSYR2K (+ DGEMM/DTRSM regression) with reporter flags from Cortex-A72 target produces wrong BLAS Level-3 results #5997 (OPENBLAS_NUM_THREADS=1)
  • Optional: OPENBLAS_NUM_THREADS>1 SYRK pass (MN=6 thread splits)
  • Optional short HPL check that DGEMM throughput is unchanged

Remaining risks

  • trmmkernel_6x8.c is correctness-first C (not a tuned dtrmm_kernel_6x8.S); DTRMM may be slower until an asm TRMM exists.
  • Full confirmation needs the hardware cblas run above.

Made with Cursor

hugomeiland and others added 2 commits August 28, 2026 20:25
The A72 6x8 DGEMM path left SYMM packer remainders as (n-n/6), wired
DTRMM to a 2x2 ukernel against 6x8 packs, and left GEMM_UNROLL_MN at
MAX(6,8)=8. Correct the packers, add a contiguous 6x8 TRMM ukernel,
and set UNROLL_MN=6. Fixes OpenMathLib#5997.

Co-authored-by: Cursor <cursoragent@cursor.com>
CORTEXA72 sets DGEMM_UNROLL_MN=6 so SYRK diagonals match MR=6 packs.
The ~(MN-1) form only works for power-of-two MN; use the same division
as syrk_kernel.c.

Co-authored-by: Cursor <cursoragent@cursor.com>
@hugomeiland

Copy link
Copy Markdown
Contributor Author

Graviton1 verification (a1.metal, Cortex-A72 ×16)

Tested HEAD 528e8737 with:

make libs TARGET=CORTEXA72 DYNAMIC_ARCH=0 USE_THREAD=0 NO_SHARED=1 NO_LAPACK=1 NUM_THREADS=1
OPENBLAS_NUM_THREADS=1 ./dblat3 < dblat3.dat
Routine develop this PR
DGEMM PASS PASS
DSYMM FAIL PASS
DTRMM FAIL PASS
DTRSM PASS PASS
DSYRK FAIL still FAIL ('L','N',31,2)
DSYR2K FAIL still FAIL ('U','N',31,2)

So this restores the reporter’s DSYMM/DTRMM failures on real A72; DSYRK/DSYR2K still need a follow-up (diagonal tiles / non-PoT UNROLL_MN=6 path).

Artifacts: arm-benchmarks results (cc1363e).

hugomeiland and others added 2 commits August 28, 2026 20:59
SYRK packs B in UNROLL_MN-wide strips then calls GEMM with full panel N.
With MN=6 that layout does not match NR=8, so DSYRK/DSYR2K failed on
real A72. MN must be a multiple of UNROLL_N; MR=6 ICOPY handles width 8
as 6+2. Keeps the syr2k division stepping fix.

Co-authored-by: Cursor <cursoragent@cursor.com>
SYRK/SYR2K diagonal kernels advance packed A/B by UNROLL_MN and require
panel-aligned starts for both MR and NR. MN=8 fixed NR packing but broke
MR=6 A panels; MN=24 is the common multiple. Keeps non-PoT division stepping.

Co-authored-by: Cursor <cursoragent@cursor.com>
@hugomeiland

Copy link
Copy Markdown
Contributor Author

Graviton1 verification (a1.metal, 16 cores)

Correctness: All dblat3 Level-3 tests PASS — DGEMM, DSYMM, DTRMM, DTRSM, DSYRK, DSYR2K.

Fix: DGEMM_UNROLL_MN=24 (LCM of MR=6, NR=8). MN=8 matched NR but stepped into the middle of MR=6 A panels; MN=6 broke NR=8 B packs.

Performance vs stock CORTEXA57:

  • MT DGEMM: +8–10% (n=4096: 103 vs 95 GFLOPS; n=20000: 108 vs 99 GFLOPS)
  • L2 (DGEMV/DSYMV/DGER/DTRMV): parity
  • L3 MT: DSYMM +6–15%; DSYRK parity at n≥1024; DTRMM slower at small n (−8% at n=4096)

Multicore uplift from the 6×8 kernel is preserved.

Replace the correctness-first generic C trmmkernel_6x8 with an ASM
ukernel derived from dgemm_kernel_6x8_cortexa72.S (TRMM offset/K
stepping and overwrite SAVE). Restores DTRMM throughput vs stock A57
on multicore Graviton1.

Co-authored-by: Cursor <cursoragent@cursor.com>
@hugomeiland

Copy link
Copy Markdown
Contributor Author

ASM DTRMM follow-up

Replaced generic trmmkernel_6x8.c with dtrmm_kernel_6x8_cortexa72.S (derived from the 6×8 DGEMM ukernel).

Correctness (a1.metal): dblat3 DTRMM still PASS (2592 calls), all other L3 PASS.

DTRMM perf vs prior C ukernel / stock CORTEXA57 (16 threads):

n ASM A72 A57 vs A57 vs prior C
256 58.0 62.3 −7% +106%
1024 79.4 73.2 +8% +42%
2048 92.5 83.5 +11% +30%
4096 94.8 91.5 +4% +11%

ST DTRMM still ~5–8% behind A57 (same pattern as ST DGEMM); MT DTRMM now matches/beats A57 at n≥1024.

Rem panels of size 4/5 used (m & ~(i-1))-i, which assumes power-of-two
M alignment. With MR=6 that misplaces the rem-4 block (cblas N=5/35).
Walk the rem start position from the end instead.

Co-authored-by: Cursor <cursoragent@cursor.com>
@hugomeiland

Copy link
Copy Markdown
Contributor Author

Full make tests on Graviton1 (a1.metal / Cortex-A72)

After the previous CBLAS miss: Fortran dblat3 only goes to N=31, while CBLAS din3 includes N=5 and N=35 (rem=5 with MR=6). That exposed a leftover PoT assumption in trsm_kernel_LN_UNROLLM6.c rem indexing ((m & ~(i-1))-i).

Fix in c8a6e793: walk rem panel starts from the end (pos -= i) instead of bitmask alignment.

Results (TARGET=CORTEXA72, tip c8a6e793)

Suite Threads Exit
make tests OPENBLAS_NUM_THREADS=1 0
make tests OPENBLAS_NUM_THREADS=16 0

Includes Fortran BLAT2/BLAT3 (S/D/C/Z), CBLAS (incl. cblas_dtrsm col+row 3528 calls), and utest. No FATAL/FAILED lines in either log.

Isolated re-check after the LN rem fix: xdcblat3 < din3 — all Level-3 double CBLAS PASS, including cblas_dtrsm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cortex-A72 target produces wrong BLAS Level-3 results

1 participant