Fix two reward-hacking gaps in the bench harness - #851
Open
robobryce wants to merge 1 commit into
Open
Conversation
(1) Disable the persisting L2 carveout for benchmark_kernel — without
this, a kernel that sets a persisting access policy keeps its
working set L2-resident across iterations (cudaMemset of the
flush buffer doesn't evict persisting lines), so the bandwidth
formula reports L2 throughput under the HBM label.
(2) Re-validate kernel output against the CPU reference after the
benchmark loop, not just before. A kernel that drifts state
across iters would otherwise report fast bandwidth on broken
outputs.
layernorm_forward7 is a regression test for (2): a wrapper that runs
the existing kernel 5 correctly for the first 10 calls (passes the
pre-bench check) then corrupts out[0] (caught by the post-bench
check).
|
I, a human, am responsible for this agent; I've vetted this PR. |
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
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.
Summary
Two fixes to
dev/cuda's benchmark harness that close gaps an autonomous-agent optimizer can exploit to "improve" the reported metric without actually doing more work.(1) Cap the persisting L2 carveout to zero during
benchmark_kernelA kernel that sets a stream-level
cudaStreamAttributeAccessPolicyWindow(hitProp=Persisting, hitRatio=1.0)covering its working set can keep that data L2-resident across the entire 2000-iter benchmark loop. The existingcudaMemset(flush_buffer, …)between iterations doesn't evict lines marked persisting — that's documented CUDA L2-persistence behaviour. The bandwidth formula2*B*T*C*4 / timethen silently reports L2-to-SM throughput under the HBM label.Concretely, on Ada (L40S, 864 GB/s HBM peak), a modified
layernorm_forward5that wraps its kernel launch withreports 3530 GB/s — over 4× HBM peak, physically impossible if the formula's premise held. After this fix it reports 714 GB/s, under HBM peak, with no correctness impact. The fix:
cudaLimitPersistingL2CacheSize, sets it to 0 (which also resets any currently-persisting lines to normal status), runs the benchmark, restores the original limit on exit;flush_bufferto 2× L2 so a singlecudaMemsetis guaranteed to evict the entire L2 via LRU pressure;(2) Re-validate kernel output against the CPU reference after the benchmark loop
The pre-bench
validate_resultconfirms the kernel is correct on one invocation. The 2000-iter bench loop runs against unverified results. A kernel that's correct on iter 1 but drifts state on subsequent iters — e.g.— passes pre-bench, runs the loop, reports a fast bandwidth on broken outputs.
After this fix, the post-bench
validate_resultruns against the final iteration'sd_out/d_mean/d_rstdand exits 1 on mismatch.A new
layernorm_forward7is included as a regression test for (2): it wrapslayernorm_forward5so the LayerNorm result is correct for the first 10 invocations (passing the six pre-bench correctness checks at the six block sizes) and then corruptsout[0]on every subsequent call. The post-bench check catches it and exits 1.Why this matters
We hit both gaps running an autoresearch-style autonomous-agent optimization loop against
dev/cuda/layernorm_forward.cu. The agent's best trial reported 5.67× over the baseline (622 → 3534 GB/s) — but ~4.5× of that came from the L2-persisting trick alone, not from kernel improvements. The reported speedup wouldn't transfer to production (a real training step doesn't normalize the same tensor 2000 times in a row against a flush that the persisting policy specifically defeats), the metric exceeded HBM peak, and the optimization landscape was being explored against a mislabeled signal.The (2)-class drift hack we didn't observe in practice on this run, but the harness today admits it: any kernel that uses a device-global counter to make its 2000th call's output different from its 1st passes the existing check.
Test plan
On Ada (L40S, 864 GB/s HBM peak):
./layernorm_forward Nfor N in 1..6 → all pre-bench and post-bench checks pass; all reported bandwidths ≤ HBM peak (kernel 5 drops from 3530 → 714 GB/s when the persisting-window variant is present; the upstream kernel 5 reports ~637 GB/s, unchanged)../layernorm_forward 7→ pre-bench passes, post-bench fails, exit 1 with a clearMismatch of out (post-bench) at 0: CPU_ref: … vs GPU: 1234.500000message.Scope
Only
dev/cuda/common.h(thebenchmark_kernelhelper, shared by everydev/cuda/*_forward.cu) anddev/cuda/layernorm_forward.cu(main + kernel 7) are touched. The otherdev/cuda/*_forward.cuharnesses would benefit from the same post-bench re-check; happy to extend in a follow-up if that's wanted, or fold the post-bench check into a shared helper incommon.h.