Repository navigation
fix(shader): keep pixel invocations running after a BDA fault in a program that reads lanes by index - #14866
Merged
oneandonlydean merged 1 commit intoNov 3, 2026
Conversation
…ogram that reads lanes by index A pixel program without barriers stops an invocation whose BDA read faults: it records the fault and returns from the guest code. The other lanes keep the masks they took before, with the stopped lane's bit still set. A waterfall over such a mask (s_ff1_i32_b64, v_readlane_b32, v_cmp_eq, s_andn2_b64, s_cbranch_scc1) picks the stopped lane, reads nothing from it and never clears its bit, so it spins until the device is lost. Pixel programs that read or write a lane by index now keep faulting invocations running with the read returning zero, as programs with barriers already do. Other stages keep stopping. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
unblocks title
Contributor
✅ AMD GPU test: passedPR head
Unstable on main (4, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (9, not counted against this PR)Passed on retry (1, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validation IDs already present on main (5 IDs in 484 tests)Skipped (55)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (4, not counted against this PR)Validation IDs already present on main (5 IDs in 484 tests)Skipped (46)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
BunnyFl
approved these changes
Nov 2, 2026
BunnyFl
left a comment
BunnyFl
left a comment
Contributor
There was a problem hiding this comment.
Build and test on Windows 11, AMD Radeon RX 9070 XT (RDNA 4), WinLibs GCC, BUILD_TESTING=ON, Khronos validation layer on, PR merged into current main:
agc_driver_host_import_failure_handles, which runsRunBdaContractTests, andagc_driver_bda_device_selection_cpupass.- With
main'sSpirvBdaRead.cpprestored,agc_driver_host_import_failure_handlesfails on the new check ("a pixel program with v_readlane must keep its invocations running").
The reasoning holds: a waterfall's v_readlane can't drain a lane whose bit stays in the copied mask after the lane stopped, so a pixel program with cross-lane reads must keep faulting lanes running, as one with a barrier already does.
One question, not blocking: why only pixel? A compute or vertex program that stops a lane the same way (it returns from the guest code while the other lanes keep a mask copied from exec) can run the same waterfall, and the new test pins BdaInvocationsMayStop as true for a compute program with v_readlane. If stopping works differently outside pixel shaders, so the stale bit can't occur there, a sentence in the PR or debt entry saying so would help the next reader.
Contributor
Author
|
@BunnyFl thank you for the RDNA 4 run and for checking that the new case fails without the change. I added both to Tested. On why only pixel: stopping works the same way in every stage. On The compute AI-assisted: yes (Claude Code) |
XairRrR
approved these changes
Nov 3, 2026
XairRrR
left a comment
XairRrR
left a comment
Contributor
There was a problem hiding this comment.
Reviewed — approved
BdaInvocationsMayStop now returns false for pixel programs that contain ReadLane or WriteLane. The new contract checks pass on the merged tree and fail with main's implementation restored. I found nothing blocking.
What I ran
Head 9172e6c2a5cfb2d7a331f663be58d3010aa7555a merged with --no-ff into main d1ae628bc2f62459cce1e9872ba5632c938c71f2. The merged tree is 020afaf4ebc7c2a24a5ddcf3e7ab1ac9afa94615. I ran everything in a clean worktree on Debian 13 (VM, 32 vCPU Xeon Gold 6136), with GCC 14.2.0 and lavapipe (Mesa 26.1.6).
git diff --check d1ae628bc2f62459cce1e9872ba5632c938c71f2...9172e6c2a5cfb2d7a331f663be58d3010aa7555a
python3 tools/check_conventions.py --base d1ae628bc2f62459cce1e9872ba5632c938c71f2
ctest --test-dir build -j8 --test-load 32 --output-on-failure -R "^agc_driver_(host_import_failure_handles|bda_device|bda_device_selection_cpu)$"
git show d1ae628bc2f62459cce1e9872ba5632c938c71f2:core/shader/recompiler/SpirvBackend/src/SpirvBdaRead.cpp > core/shader/recompiler/SpirvBackend/src/SpirvBdaRead.cpp
ninja -C build -j32 -l32 agc_driver_bda_device_tests
ctest ... (same three tests)
git show 020afaf4ebc7c2a24a5ddcf3e7ab1ac9afa94615:core/shader/recompiler/SpirvBackend/src/SpirvBdaRead.cpp > core/shader/recompiler/SpirvBackend/src/SpirvBdaRead.cpp
ninja -C build -j32 -l32 agc_driver_bda_device_tests
git status --porcelain
ctest ... (same three tests)
Results
| Check | Result |
|---|---|
git diff --check |
clean |
check_conventions.py |
all checks passed |
Configure / build / libs (script) |
ok |
| Selected tests (script) | 2131 run, 2011 passed, 113 expected skips. No new failures and no timeouts. The 7 failures (agc_driver_division, the denormal-mode tests, vector_constant_factor_tininess, an unwrap_self case) also fail on main and are not this PR's. |
Vulkan validation vs main (script) |
767 tests, layer loaded in 766, none missing, 0 tests with new VUID- lines |
agc_driver_bda_device, agc_driver_host_import_failure_handles, agc_driver_bda_device_selection_cpu |
pass, each under 1 s |
Negative control
I restored only core/shader/recompiler/SpirvBackend/src/SpirvBdaRead.cpp from main and rebuilt. agc_driver_bda_device and agc_driver_host_import_failure_handles then fail on the first new check:
output
BDA device selection CPU tests passed
AGC graphics: a pixel program with v_readlane must keep its invocations running: a waterfall over a mask that holds a stopped lane never clears its bit
After I restored the merged file and rebuilt, all three tests pass again and git status --porcelain is empty. This confirms @BunnyFl's negative control on RDNA 4 with a second, independent run.
Findings
Blocking
None.
Non-blocking
- Field comment no longer covers every case.
core/shader/recompiler/SpirvBackend/include/SpirvBackend/SpirvEmitterState.hpp:198saysbdaStopsInvocationsis "False for programs with workgroup barriers". After this change it is also false for pixel programs withReadLane/WriteLane. The description says the head deliberately dropped an edit to that comment, because comment changes need a technical-debt change. A maintainer may want to update it in a follow-up so that it matchesBdaInvocationsMayStop(core/shader/recompiler/SpirvBackend/src/SpirvBdaRead.cpp:416-426). - The rule matches opcodes, not the loop shape. Any pixel program with a
ReadLane/WriteLanenow loses the early stop, including programs with no waterfall. In those programs a faulting invocation keeps running on zeros and its later writes are not masked;#6416'sbdaFaultMasksWritescovers only non-pixel stages. This matches what barrier programs already do, and the fault is still recorded and reported, so I see no correctness regression. Other cross-lane ops (DppMoveU32,Permlane16U32,BpermuteU32,SwizzleU32) keep the stop. I agree that these cannot spin on a stale mask bit the way thes_ff1/v_readlanewaterfall does. - Missing bring-up discussion on the Unblocks line. The line names PPSA02225 but no Game bring-up discussion number. CONTRIBUTING asks for both (
Unblocks: PPSAxxxxx #N).
On @BunnyFl's question about compute and other stages: the author's answer is consistent with the code. SpirvEmitter.cpp:332 combines BdaInvocationsMayStop with the stage-independent conditions, and the new compute check pins the current behaviour. The stale-bit hazard in non-pixel stages is still open on main until #6416 lands. Neither the PR nor the debt docs record this, so it depends on that PR being merged.
Not verified
- The Ghost of Tsushima (PPSA02225) device-loss result. I did not launch the title, and I had no NVIDIA or AMD hardware; GPU coverage here is lavapipe only.
- The offline-replay claim (201 returns to 2,
OpTerminateInvocationremoved). I did not have the dumped requests. - Any behaviour on hardware where a faulting pixel invocation keeps running with zero reads.
Contributor
Author
|
@XairRrR thanks for the full check with the negative control. Restoring only AI-assisted: yes (Claude Code) |
github-actions
Bot
added
the
review queue
github-actions
Bot
removed
the
review queue
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.
What
A pixel program without workgroup barriers stops an invocation whose BDA read faults. The read records the fault, sets the stop flag and returns from the guest code (
StopBdaInvocationIf), and the invocation is terminated after the guest code returns. The other lanes keep running with the masks they took before, and those still hold the stopped lane's bit.A waterfall over such a mask is a loop that walks the lanes of a mask copied from
exec:s_ff1_i32_b64picks the lowest remaining lane;v_readlane_b32reads that lane's value;v_cmp_eqfinds the lanes with the same value;s_andn2_b64removes them from the mask;s_cbranch_scc1repeats while lanes remain.When the lowest remaining lane is the stopped one,
v_readlane_b32reads nothing from it, no running lane matches, and its bit is never removed. The loop spins until the GPU's context-switch timeout loses the device.Ghost of Tsushima Director's Cut (PPSA02225) is where this showed up. These runs used my development stack (
mainplus local GoT work), with and without this change, on one machine (Linux, RTX 4070 Ti, NVIDIA 615.71.09):dmesg, it showedXid 109 CTX SWITCH TIMEOUTfor the title. In the three runs where I traced the batch that never finished, it held the same two pixel programs, and each of them has three such waterfalls and about 200 stop points.The two runs with it whose logs I checked (the first two) never logged a BDA fault report, so this shows the device loss going away, not a stop caught in the act.
How
BdaInvocationsMayStopalready returns false for programs with workgroup barriers, where a stopped invocation would leave a barrier incomplete. It now also returns false for pixel programs withReadLaneorWriteLane. Those programs keep faulting invocations running: the read returns zero, and the fault is still recorded and reported.ReadFirstLanealone keeps the stop, since it picks among the lanes that are still running.Other stages keep stopping. #6416 covers non-pixel programs with wave operations: it keeps their faulting invocations voting and masks their later writes. It leaves pixel programs out, so the two cover different stages, and this change leaves compute and the other stages as they are on
main. Stopping works the same way in every stage, so onmaina compute or vertex program with a waterfall can hit the same stale bit. This PR leaves those stages to #6416, which setsbdaFaultMasksWritesonly for stages other than pixel and keepsBdaInvocationsMayStopunchanged. That is why the new test still expectsBdaInvocationsMayStopto be true for a compute program withReadLane.Tested
Linux (Ubuntu 26.04.1, GCC 15.2, Release, RTX 4070 Ti, NVIDIA 615.71.09) unless noted. The Linux and validation-layer runs are at
ba404360ad. This head9172e6c2a5only drops that commit's edit to a comment, so the code is the same.agc_driver_bda_device(BdaContracts.cpp, next to the existing barrier case): a pixel program withReadLane, and one withWriteLane, must keep faulting invocations running. A pixel program with onlyReadFirstLane, and a compute program withReadLaneand no barrier, must still stop them.9172e6c2a5(and onba404360ad);mainf2d874c1fdpasses 3 of 3 without the new cases.8b668f002b(mainplus only the new cases, branchtest/bda-stop-red2on my fork) fails 3 of 3 on the VM:a pixel program with v_readlane must keep its invocations running: a waterfall over a mask that holds a stopped lane never clears its bit.ba404360ad; the red control8b668f002bfails.main(@BunnyFl):agc_driver_host_import_failure_handlesandagc_driver_bda_device_selection_cpupass; withmain'sSpirvBdaRead.cpprestored, the new check fails.mainf2d874c1fdagainstba404360ad, over the BDA, lane-read and wave GPU tests (^agc_driver_(bda_|read_lane|write_lane|read_first_lane|lds_mask_waterfall|pixel_helper_lanes|pixel_wave_reduction|permlane16|wave_uniform$)|^agc_shader_specialized_bda|^agc_bda_staging, 18 tests): 18 of 18 pass on both, 11 Vulkan devices created on each side, no validation IDs on either. The twoagc_driver_bda_reads_shadowed_results*tests skip on both sides.agc_shader_replay --prepareon dumped requests): their prepared modules go from 201 returns and anOpTerminateInvocationto 2 returns and none. Both passspirv-val.Checklist
main(branched atf2d874c1fd, merges cleanly with5bae1a4bc2); no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt.mdfiles or images added to the repository (attach them to this PR)4414d7880e)🤖 Generated with Claude Code