Dedupe the prefetch hint, drop the streaming one, add the missing benches - #26
Closed
enthropy7 wants to merge 1 commit into
Closed
Dedupe the prefetch hint, drop the streaming one, add the missing benches#26enthropy7 wants to merge 1 commit into
enthropy7 wants to merge 1 commit into
Conversation
…ssing benches Reviewing PR #22 turned up that `prfm pldl1keep` had drifted into three identical copies. Fold them into `ops::prefetch::prefetch_l1_keep`, which also documents the two rules the measured results depend on: hoist the gate out of the K-loop (the per-iteration branch alone costs the A53 more than the hint saves), and only hint operands whose stride actually beats the hardware prefetcher. By that second rule the AVX `binary_same_shape` prefetch does not belong: the access is unit-stride over three streams. Measured on Zen 4 it sits inside run-to-run drift — the A/B flips sign depending on which arm runs first — so it was only adding uops to a bandwidth-bound loop. The transposed-A and `matmul_row_set` prefetch hints from that PR are not here: they measured neutral. On Zen 4 the trans_a kernel at the tracker's real shape is 18.97 vs 19.05 us (p = 0.31) once the A/B is run in both directions, and `FusedTransposeMatMul` — their only caller — is 1.1% of node time on Zen 4 and 1.3% on the A53, so there is nothing there to win. Both kernels were unbenchmarked, which is why that was not visible; they have benchmarks now. Also assert the execution plan carries one action per node. With the sequential fallback gone the runner walks the plan and nothing else, so a short plan would silently skip trailing nodes rather than fail. A53 (Orange Pi, 1T, 60 iters x 3 alternating): 337.6/337.8/337.8 ms vs 338.7/338.9/339.8 on main. Zen 4 tracker unchanged at 8.8 ms.
Owner
Author
|
Closing — landing directly on main instead. |
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
1 participant
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.
Fallout from reviewing #22.
prfm pldl1keephad drifted into three identical copies. They fold intoops::prefetch::prefetch_l1_keep, which documents the two rules the measuredresults depend on: hoist the gate out of the K-loop, and only hint operands
whose stride actually defeats the hardware prefetcher.
By that second rule the AVX
binary_same_shapeprefetch does not belong — theaccess is unit-stride over three streams. On Zen 4 it measures inside
run-to-run drift (the A/B flips sign with run order).
The transposed-A and
matmul_row_sethints from #22 are deliberately not here.Run in both directions the trans_a kernel at the tracker's real shape is
18.97 vs 19.05 us, p = 0.31 — no effect. And
FusedTransposeMatMul, their onlycaller, is 1.1% of node time on Zen 4 and 1.3% on the A53. Both kernels were
unbenchmarked, which is why that was invisible; they have benchmarks now.
Plus two review follow-ups:
use_avx512_mr12claimed "DEFAULT ON" where thegate below it is opt-in on the exact value
1, andrebuild_runtime_indexnowdebug-asserts one plan action per node, since #24 removed the only fallback for
a short plan.
Validation:
cargo test --workspace(all green, incl. doctests), fmt, clippy--all-targets -D warnings, doc-count gate. A53 (Orange Pi, 1T, 60 iters x 3alternating): 337.6/337.8/337.8 ms vs 338.7/338.9/339.8 on main. Zen 4 tracker
unchanged at 8.8 ms.