Repository navigation
feat(agc): rebuild the hull's relative patch index when it is used as data - #4201
Conversation
5610817 to
f1f83f2
Compare
|
I force-pushed this after a Yōtei run showed the first version was too strict. The terrain draw runs with Checks after the change
|
f1f83f2 to
e81cb5e
Compare
e81cb5e to
4c1e461
Compare
|
One more push here: Yōtei's next terrain domain program (0x8068fbd900) uses the relative patch index as data too, so domain programs now get the same |
|
Data point from Ghost of Yōtei (PPSA26344) on the instancing model here: on that title, the instances of a tessellated draw share hull threadgroups. Restarting the relative patch index with each instance gives the wrong index there. What the title programs
Why the per-instance restart is wrong here With BREAK_WAVE_AT_EOI clear, a threadgroup runs on into the next instance. Patch k of instance i is then linear patch There is also a timing problem for indirect draws. The hull is compiled when the packet is decoded, before the records are read, so it can't see the record's instance count. What we have locally We have this working in #4450, a draft stacked on #4222 (only its top commit is new):
Would you rather fold this into #4201, or keep #4450 as a separate PR on top of #4222? Either works for us. If you fold it in, I'll close #4450. A smaller point from reading the stack: the hull's patch-constant bound compares (AI-assisted: written with Claude.) |
|
Rebase handoff — the chain is complete: Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick up the whole chain: Heads: |
4c1e461 to
6c19457
Compare
|
You're right, and thanks for checking it against the title's registers. I had assumed every instance starts its own hull threadgroups, and with GE_CNTL 0x8040 that assumption doesn't hold, so on Yōtei #4201 alone would draw the terrain with every instance at relative patch 0 and never say so. I'd keep #4450 as its own PR on top of the stack. It's your work and your test, and it carries the in-game result. To make #4201 honest on its own, it now decodes The whole chain had to be rebased anyway. On the smaller point, I agree. #4163 now checks the hull's patch-constant writes against the same bound as the domain's reads: above the control points of all NUM_PATCHES patches, not just one patch's. The descending-record test hull therefore draws at 128 patches per group and is refused at 129. At 129 its lowest record would land inside the last patch's control points. Replayed offline with the title's own GE_CNTL 0x8040, the terrain hull (0x80002fc700) now stops at the new refusal. With the bit set in the request, it passes the tighter bound and recompiles completely (388,903 SPIR-V words). Details
|
1ad64e4 to
cecca8a
Compare
cecca8a to
f823bf9
Compare
|
Quick note: I restacked this on #4172's new head, which now has the block-condition fix from the review there. While rebasing I noticed |
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.
f823bf9 to
2eda9f8
Compare
|
Rebased the chain again after #5234 landed. The only overlap was in the tests and the debt list: #5234's integer-partitioning test and this PR's relative-patch test both sit in |
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
|
…nces With GE_CNTL.BREAK_WAVE_AT_EOI clear, the instances of a tessellated draw share hull threadgroups: the relative patch index counts on through the instances, while the host's PrimitiveID restarts with each one. The local stage now passes its draw-relative instance (InstanceIndex - BaseInstance) to the hull, which numbers its patches instance * patches-per-instance + PrimitiveID before taking (mod PRIM_GRP_SIZE) mod NUM_PATCHES. Instances that start a threadgroup anyway (BREAK_WAVE_AT_EOI, or patches per instance a multiple of the group) keep the per-instance PrimitiveID. This replaces boykopovar#4201's refusal of a hull or domain that uses its relative index as data while BREAK_WAVE_AT_EOI is clear, and builds on its decode of the bit (breakAtInstance, request serialization and draw keys). - Hull and domain cache keys hold the patches per instance instead of the bit; an indirect draw recompiles its hull and domain for each record's instance count. - The device enables shaderDrawParameters when available. Without it, a hull using its relative index as data in such a draw is refused, as is a domain doing so (passing the instance on to the domain is not implemented). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nces With GE_CNTL.BREAK_WAVE_AT_EOI clear, the instances of a tessellated draw share hull threadgroups: the relative patch index counts on through the instances, while the host's PrimitiveID restarts with each one. The local stage now passes its draw-relative instance (InstanceIndex - BaseInstance) to the hull, which numbers its patches instance * patches-per-instance + PrimitiveID before taking (mod PRIM_GRP_SIZE) mod NUM_PATCHES. Instances that start a threadgroup anyway (BREAK_WAVE_AT_EOI, or patches per instance a multiple of the group) keep the per-instance PrimitiveID. This replaces boykopovar#4201's refusal of a hull or domain that uses its relative index as data while BREAK_WAVE_AT_EOI is clear, and builds on its decode of the bit (breakAtInstance, request serialization and draw keys). - Hull and domain cache keys hold the patches per instance instead of the bit; an indirect draw recompiles its hull and domain for each record's instance count. - The device enables shaderDrawParameters when available. Without it, a hull using its relative index as data in such a draw is refused, as is a domain doing so (passing the instance on to the domain is not implemented). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… PRIM_GRP_SIZE) mod NUM_PATCHES)
GE_CNTL.BREAK_WAVE_AT_EOI (bit 18) joins the tessellation configuration as breakAtInstance (draw keys, hull and domain cache keys) and hull or domain programs that use their relative patch index as data throw when it is clear, as boykopovar#4201 does since its 2026-10-13 rebase. On this branch the field is request version 21 (17-19 are the tessellation chain, 20 the 10_11_11 blend); upstream boykopovar#4201 writes 22.
…nces With GE_CNTL.BREAK_WAVE_AT_EOI clear, the instances of a tessellated draw share hull threadgroups: the relative patch index counts on through the instances, while the host's PrimitiveID restarts with each one. The local stage now passes its draw-relative instance (InstanceIndex - BaseInstance) to the hull, which numbers its patches instance * patches-per-instance + PrimitiveID before taking (mod PRIM_GRP_SIZE) mod NUM_PATCHES. Instances that start a threadgroup anyway (BREAK_WAVE_AT_EOI, or patches per instance a multiple of the group) keep the per-instance PrimitiveID. - This replaces boykopovar#4201's refusal of shared threadgroups; boykopovar#4201 already decodes GE_CNTL.BREAK_WAVE_AT_EOI into the tessellation configuration. - Hull and domain cache keys hold the patches per instance; an indirect draw recompiles its hull and domain for each record's instance count. - The device enables shaderDrawParameters when available. Without it, a hull using its relative index as data in such a draw is refused, as is a domain doing so (passing the instance on to the domain is not implemented). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
Stacked on #4172; please review the last commit only.
Tessellation lowering refuses hull and domain programs whose relative patch index (hull
v1[7:0], domainv7) reaches anything but a ring or LDS address, because one Vulkan patch per hull patch slot has no such index of its own.(PrimitiveID mod PRIM_GRP_SIZE) mod NUM_PATCHES(a domain'sgl_PrimitiveIDis its hull patch's, so both stages compute the same number): hull threadgroups takeNUM_PATCHESpatches out of eachGE_CNTL.PRIM_GRP_SIZEprimitive group.GE_CNTL.BREAK_WAVE_AT_EOI(bit 18) set. With it clear the instances share threadgroups and the index counts on through them, so such programs are still refused. fix(shader): number an instanced draw's hull patches across its instances #4450 numbers the patches across instances for that case.PRIM_GRP_SIZE(bits 8:0) andBREAK_WAVE_AT_EOIjoin the tessellation configuration (draw, validation and pipeline keys, the hull and domain cache keys, recompile request version 24). A missing GE_CNTL throws. Local programs still refuse the index: there it is a relative vertex slot whose value depends on how the hardware lays out vertices in a threadgroup.The model is not measured on the console;
debt/shader.mdsays so. PAL says a group size that is not a multiple of the patch count is not allowed, yet PPSA26344's terrain draws run withPRIM_GRP_SIZE64 andNUM_PATCHES63 (GE_CNTL 0x8040, BREAK_WAVE_AT_EOI clear), so on their own they are refused here and need #4450.Tested
Based on
mainf265cd5 plus #4172. Windows 11 (MinGW-w64 GCC 15.2.0) and Linux: configure -DBUILD_TESTING=ON, targeted test builds, ctest. Measured on #4222's head, which contains this PR:agc_driver_tessellation,agc_shader_memory,agc_driver_graphics,agc_tessellation_factor_ringandagc_driver_bda_devicepass (5/5)main: the same 5 tests pass (5/5)log_macros, which fails onmaintoo; Linux (upstream's job in WSL) merged withmain5a81f60: 1089/1091,agc_driver_bc_block_copyandagc_driver_depth_compare_copyfail on thatmainas wellagc_driver_tessellationgainsRequireRelativePatches: the hull writesfloat(rel + 1)as a patch constant, checked at NUM_PATCHES/group 3/3, 2/4, 3/4 (patches 6 and 7, which rule out 0,id mod 3andid mod 4) and 3/2 with BREAK_WAVE_AT_EOI set; a missing GE_CNTL and a clear BREAK_WAVE_AT_EOI are refused. A domain pair (DomainRelativeData) runs the same cases.agc_driver_graphicsdecodes GE_CNTL 0x8040 and 0x5fe3f, andagc_shader_memorycovers version 24 and the hull/domain keys. They fail on #4172's head with the old refusal.Not tested: console behaviour.
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)