Repository navigation
fix(shader): lower hulls with one input or one output control point - #5457
Conversation
|
Intel run for this PR: Intel UHD Graphics 770 (Raptor Lake iGPU of an i9-13900K), driver 32.0.101.6129, Windows 11, MinGW-w64 GCC 15.2.0 Release, device picked with
AI-assisted (Claude Code). |
391f1b3 to
5589f92
Compare
A wave64 hull program narrows EXEC with s_and_saveexec_b64 and restores it later, so its exec words reach the lane-bit test as ANDs with the host ballot, as loop or join phis, and through separate ballots for the two halves. Tessellation lowering refused them as "uses a lane mask other than its own lane's bit". The own-lane bit of a mask pair is now rebuilt from the words' operations (ballots, bitwise ops, selects, phis, constants); both halves must have the same shape and ballot the same predicate. Dead hull cycles such as a saved-exec loop are removed before the remaining-ballot check, so a mask whose other lanes are read (s_bcnt1_i32_b64 exec) is still refused. The words are replaced through identities and block conditions are resolved, so a branch on a compare copied to VCC keeps its condition. Local and domain programs keep their exec words for the half-wave matcher. Block conditions fix and HullCompareBranch test from EagleBearerYS.
… data Hull programs whose relative patch index (v1 bits 7:0) reached anything but a tessellation ring or LDS address threw, because the lowering runs every Vulkan patch as relative patch 0. The quad terrain hull 0x80002fc700 in Ghost of Yotei is one, and so is the domain 0x8068fbd900 (v7). Such hulls and domains now take (PrimitiveID mod GE_CNTL.PRIM_GRP_SIZE) mod VGT_LS_HS_CONFIG.NUM_PATCHES, as if hull threadgroups took NUM_PATCHES patches at a time out of each primitive group; both stages see the same PrimitiveID for a patch. This holds only when every instance starts a new threadgroup, so it needs GE_CNTL.BREAK_WAVE_AT_EOI (bit 18); with the bit clear the instances share threadgroups and such programs still throw. PRIM_GRP_SIZE and BREAK_WAVE_AT_EOI join the tessellation configuration (draw keys, hull and domain cache keys, request version 24). A missing GE_CNTL throws, and so do local programs, whose relative vertex index depends on vertex reuse that Vulkan does not expose.
Every hull program with a BDA access was refused with "BDA fault termination requires a barrier-safe tessellation-control execution protocol". The refusal predates BdaInvocationsMayStop: a program with s_barrier no longer stops a faulting invocation; the fault is recorded, the read returns zero and every invocation reaches the barrier. In a hull, s_barrier is the only control barrier the backend emits after the guest code starts, so that rule already covers it, and a hull without one can stop an invocation without stranding a barrier. The tessellation test draws hulls that scale their levels by a BDA read, with and without s_barrier, and requires an unmapped read to report a tessellation-control fault (not on llvmpipe, which runs hull loads that the unmapped-address check skips). The contract test now accepts a hull with s_barrier.
…eads one control point
derive() learned the LS-to-HS stride from the control-point term of hull LDS reads, or from the patch term of reads at a computed control-point index. A hull with one input control point reads it at constant offsets, so neither applies and the stride stays unknown ('the hull shader reads no LDS per control point, so the local stride is unknown'). Every hull read already has a patch term of exactly inputControlPoints * localStride, so take the stride from it for reads without a control-point term too. Resident Evil Village's quad patches have one input control point.
A hull with one output control point writes its control point at the
patch record offset alone, so the write has no control-point term and
was taken for a patch constant with no control stride ('the hull shader
writes off-chip memory outside its control points and patch constants').
Take the control stride from the patch term of such writes when the
write fits in one record (its offset is at least 0 and its end within
the patch term), the rule domain programs already apply to control-point
reads; patch-constant records lie outside that, below the top of the
off-chip block or above every control point. With one invocation per
patch, per-patch outputs also need no control-point-0 guard. Resident
Evil Village's quad patches have one output control point.
5589f92 to
1243f06
Compare
* fix(agc): accept integer tessellation partitioning Ported from upstream PR boykopovar#5234, adapted to our triangle/fractional-odd only tessellation path: VGT_TF_PARAM.PARTITIONING 0 decodes, emits equal spacing and bounds factors at 64. Metal still refuses guest tessellation draws in MetalRenderState. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): accept fractional-even tessellation partitioning Ported from upstream PR boykopovar#5402 (its integer-partitioning base commit is the previous commit), adapted to our triangle-only tessellation path: VGT_TF_PARAM.PARTITIONING 3 decodes, emits fractional-even spacing and bounds factors at 64. Metal still refuses guest tessellation draws. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(shader): place hull input components by their offset in the control-point slot Ported from upstream PR boykopovar#4723 (shader hunk; its Vulkan execution test is not ported). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(shader): lower hulls with one input or one output control point (partial) Ported from upstream PR boykopovar#5457, commits 1, 4 and 5 of 5 (hull exec masks restored through phis, input stride from the patch term, one output control point per patch). Vulkan execution-test hunks are not ported. Skipped from boykopovar#5457, with reasons: - relative patch index rebuilt as data: needs GE_CNTL.PRIM_GRP_SIZE and BREAK_WAVE_AT_EOI in TessellationConfiguration, VGT_LS_HS_CONFIG patchesPerGroup and the request key/serializer additions. None of these exist on main, which only lowers triangle fractional-odd hulls. - recording BDA faults in hull programs: drops a refusal whose safety rests on upstream hull work we lack; the refusal stays. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(shader): honour SPI_BARYC_CNTL.FRONT_FACE_ALL_BITS for the front-face input Ported from upstream PR boykopovar#5314. Tracks SPI_BARYC_CNTL in the draw key and shader request key, and bumps the request serialization to version 24 (shared with the following slice-offset port, which adds no second bump). Not ported: the ShaderMemory serializer-version assertions of the PR, which build on upstream test scaffolding absent from main. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(shader): propagate color view slice to pixel ancillary input Ported from upstream PR boykopovar#5500, adapted to main: the draw decode sets the color view slice as layerBase (the layer-export decode this PR also touches does not exist on main, so its diagnostic and layers hunks are skipped). layerBase is added to the pixel info, request key, shader info, disk cache and pipeline specialization constants. The request serialization stays at version 24, bumped once by the front-face commit. The shader disk cache format is bumped to 36. Not ported: unrelated hunks the upstream commit carried in Recompiler.cpp (dot-product capability option) and its ShaderMemory serializer tests. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(shader): cover tessellation partitioning, hull slot placement, layerBase and serializer v24 Fix the stale serialization version expectation in the shader memory test, add pixel ancillary rows with a non-zero layerBase, extract PixelLayerBase for a derivation test, and point the FrontFace note at TechnicalDebt.md. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> Co-authored-by: XairRrR <dev910382@gmail.com>
|
Title evidence for this PR: PPSA02530 (PRAGMATA v01.200.000) reaches the same first error as the Resident Evil Village one in the description. Windows 10 (19045), MinGW-w64 GCC 15.2.0, Release, all
Not tested: this PR's own tests (conflict in the test file as above), Linux, a console; I did not check whether the second error is covered by another open pull request. AI-assisted (Claude Code). |
|
Thanks for running PRAGMATA on it. That's a second title past the same hull error. Your next stop, |
|
Correction to my comment above: I named this PR's head as AI-assisted (Claude Code). |
|
Tested on head PPSA02530 (PRAGMATA v01.200.000), Windows 10 (19045), MinGW-w64 GCC 15.2.0, Release, 90 s run ( Result: with this head the hull-shader stop (line 209; a different shader and a different error). I did not check whether another open pull request covers that error. Test conditions
Not tested: this pull request's own tests (conflict in the test file), Linux, a console. AI-assisted (Claude Code). |
|
I ran this against Ghost of Yōtei's hull and domain programs to check it doesn't change anything for a title that uses 3 and 4 control points, and it doesn't: every request gives the same result and byte-identical SPIR-V with and without your two commits. Yōtei's terrain hull (0x80002fc700, quads, 4 in / 4 out) still lowers to the same 386,580-word module, and the 14 triangle hull requests (3 in / 3 out) produce the same modules too. None of them has a single control point, so the new paths in
One heads-up: after #5234 landed, I rebased the chain onto DetailsHeads tested (Windows 11, WinLibs GCC 15.2.0, Release, NVIDIA driver 610.62):
I replayed the requests with
The domain programs (including the terrain domain The captures predate request version 24: they're v16, v18 and v19 written by our game branch, whose local fields differ from upstream's. So I re-serialized them to upstream v24 with a small script. It parses each capture to the last byte, keeps every captured field and adds defaults for the fields upstream gained since. The terrain hull uses its relative patch index as data. Older captures don't record |
What
Resident Evil Village (PPSA01556, content 01.210.000) tessellates quad patches with one input and one output control point. While it creates its PSO cache, its hull programs stop in tessellation lowering, first with
and, once that is fixed, with
#5234 reports the first message for Resident Evil 2 (PPSA04288) too, after integer partitioning, with the same
VGT_TF_PARAM0x40042 (quad domain, integer). I haven't run that title.This builds on Celegans12's #4201 and #4222; #4172, the first PR of that chain, has merged. Only the last two commits are this pull request:
derive()learned the LS-to-HS stride from the control-point term of hull LDS reads, or from the patch term of reads at a computed control-point index. A hull with one input control point reads it at constant offsets, so neither applies. Every hull read already has a patch term of exactlyinputControlPoints * localStride, which the check after it requires, so the stride is now also taken from the patch term of reads without a control-point term.requireFirstInvocation.Tested
agc_driver_tessellation, with the newRequireSingleControlPointHulls.HullSingleInputreads the one input control point of its patch. Without the first commit it fails with the title's message (... the local stride is unknown).HullSingleOutputhas one output control point and patch outputs without a guard. With only the first commit it fails with the title's second message (... outside its control points and patch constants).agc_driver_tessellationandagc_driver_graphicspass.maindacd785 (https://github.com/EagleBearerYS/AnyPS5/actions/runs/38455894201): Windows 1097 of 1097 pass, withagc_driver_tessellationskipped because the runner has no Vulkan device. Linux 1079 of 1079 pass, withagc_driver_tessellationrun on lavapipe.agc_shader_control_flow_tests, built on fix(shader): record BDA faults in hull programs instead of refusing them #4222's head.lowerOwnLaneBitsfix requested in fix(agc): lower hull exec masks restored through phis to own lane bits #4172's review (now inmainwith fix(agc): lower hull exec masks restored through phis to own lane bits #4172), 19 compile. The other 2 stop at fix(agc): bound tessellation patch records by the draw's patch count #4114's patch bound only because the dumped requests carry noNUM_PATCHES; with 128 or 170 patches per group, all 21 compile.python3 tools/check_conventions.py --base pr/4222: passed.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)