Close translation parity gaps - #16
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f99cf44a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| f64 *d0 = orig + 2 * nn; | ||
| f64 *tmp = orig + 3 * nn; | ||
|
|
||
| SLC_DGEMM("T", "N", &n, &n, &n, &ONE, q3, &ldq3, a0, &lda, &ZERO, tmp, &n); |
There was a problem hiding this comment.
Use packed leading dimensions when recomputing outputs
In the new recompute_outputs path, a0/b0/d0 are stored as packed n x n buffers (orig), but the GEMM calls still pass lda/ldb/ldd as the leading dimensions (for example at a0 on this line). When any leading dimension is greater than n, this makes BLAS read the packed buffer with the wrong stride, which can produce incorrect transformed matrices or out-of-bounds reads/crashes for valid inputs (COMPQ1=COMPQ2=COMPQ3='I' with padded leading dimensions).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 316e8eb by copying the original A/B/D inputs into packed buffers with DLACPY using the caller leading dimensions, then using n as the packed buffer leading dimension in the recompute GEMMs. Added a ctypes regression that calls mb04cd directly with padded lda/ldb/ldd; tests/python/test_mb04cd.py passes locally.
Summary
Validation
.venv/bin/pytest tests/python/ -n auto --reruns 2 --only-rerun "worker .* crashed"scripts/check_translation_coverage.py --report-onlyscripts/check_translation_smells.pyorigin/mainwith fresh Meson builds: 13/13 benchmarks passed on both; no clear performance degradation requiring a code change.Notes