Repository navigation
fix(shader): lower hulls with one input or one output control point (fork CI) - #2
Closed
EagleBearerYS wants to merge 5 commits into
Closed
EagleBearerYS wants to merge 5 commits into
EagleBearerYS wants to merge 5 commits into
Conversation
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 only recognised a ballot word directly and refused the rest with "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: a ballot gives its predicate, bitwise and/or/xor/not become logical ones, selects and phis become boolean selects and phis, all-zero or all-ones words become constants. Both halves must have the same shape and ballot the same predicate. Hull values that only feed one another, such as a dead 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 rewrite runs in hull programs only; local and domain programs keep their exec words, which the half-wave reduction matcher needs.
… 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 23). 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.
github-actions
Bot
added
the
open PR limit
|
Thank you for your contribution! Authors with no pull requests merged into |
Owner
Author
|
CI done (run 38455894201); the upstream PR is boykopovar#5457. The branch stays: it is that PR's head. |
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
2 participants
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.
Fork-internal PR to run the full Build workflow before the upstream PR. Based on boykopovar#4222's head; the last two commits are the change.