Repository navigation
fix(recompiler): take a paired wave64 zero branch when some invocations are inactive - #12211
Conversation
…ns are inactive
A wave64 program held as 32 invocations x 2 lanes decided s_cbranch_execz,
s_cbranch_vccz and s_cbranch_scc0 by balloting the per-lane "zero"
condition and requiring all 64 bits set. OpGroupNonUniformBallot sets the
bit of an inactive invocation to 0 (SPIR-V specification,
OpGroupNonUniformBallot: "The bit is set to one if the corresponding
invocation is active and the Predicate for that invocation evaluated to
true; otherwise, it is set to zero"), and an invocation that returned
from the entry point is no longer active. When one invocation stopped,
e.g. after a BDA fault, the zero branch was never taken for the others:
a loop leaving through s_cbranch_execz never ended.
The RDNA2 ISA branches when the whole register is zero ("if (EXEC == 0)",
"if (VCC == 0)", "if (SCC == 0)", SOPP S_CBRANCH_EXECZ / VCCZ / SCC0), so
the branch now asks whether any active invocation still has a lane with
the bit set, as the wave32 path does: not any(own lane set || other
lane set). Inactive invocations then count as lanes with the bit clear.
The non-zero kinds keep any(own || other).
The execution test runs a wave64 program in which lane 0 faults and
stops its invocation, then takes each zero and non-zero branch; before
this change the other lanes fell through all three zero branches.
AI-assisted: yes
|
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.
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The paired condition logic is consistent with the SPIR-V ballot rule cited in the PR, and the focused regression plus same-GPU main comparison are useful evidence. I found no independent defect in the change or test by static inspection. This branch is 56 commits behind current main, however, and the contribution rules require an up-to-date branch. Please rebase on current main, rerun the paired-wave test and the same focused validation run on main and the rebased change, then I can reassess.
|
Correction to my review: I re-counted against the refreshed main tip; this branch is 60 commits behind, not 56 as stated in the review. The requested rebase and focused rerun still apply. |
|
@XairRrR The fix looks right to me. Thanks for the clear write-up and the spec citation. One optional request on the test. |
What
A wave64 program held as 32 invocations x 2 lanes decides
s_cbranch_execz,s_cbranch_vcczands_cbranch_scc0inEmitBranchConditionby balloting the per-lane "zero" condition and requiring all 64 bits set.OpGroupNonUniformBallotsets the bit of an inactive invocation to 0 (SPIR-V specification, OpGroupNonUniformBallot: "The bit is set to one if the corresponding invocation is active and the Predicate for that invocation evaluated to true; otherwise, it is set to zero"), and an invocation that has returned from the entry point is no longer active. Once one invocation stopped, for example after a BDA fault, the other invocations never took the zero branch, so a loop that leaves throughs_cbranch_execznever ended.The RDNA2 ISA takes these branches when the whole register is zero (SOPP
S_CBRANCH_EXECZ/VCCZ/SCC0: "if (EXEC == 0)", "if (VCC == 0)", "if (SCC == 0)"). The branch now asks whether any active invocation still has a lane with the bit set, as the wave32 path does:not any(not own || not other)over the lane pair's zero conditions, so inactive invocations count as lanes whose bit is clear. The non-zero kinds keepany(own || other).Found while hunting a GPU hang in PPSA05684 (Uncharted: Legacy of Thieves Collection): its 320x180 light-loop compute program is wave64 and leaves its loops through
s_cbranch_execz. The hang itself turned out to be the mip-chain issue that #6280 (open) fixes; this branch bug is real on its own and has a test.Tested
Arch Linux 7.2.11-zen, GCC 16.2.1, Ninja, Release, on
main3ca715e:agc_driver_paired_wave_zero_branches: a wave64 program in which lane 0 faults and stops its invocation, then takes each zero and non-zero branch and records which way each lane went. WithSpirvFlowEmitter.cppfrommainit fails:lane 1 fell through s_cbranch_execz after another invocation of its wave stopped.ctest -j8(this branch together with my five other branches of today, on the samemain): 2619 of 2620 pass;agc_driver_mesh_barycentric_emulationfails the same way onmainon this GPU (the pixel shader did not emulate barycentrics).tools/check_conventions.py --base origin/mainpasses.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)