Skip to content

fix(shader): record BDA faults in hull programs instead of refusing them - #4222

Merged
oneandonlydean merged 3 commits into
boykopovar:mainfrom
Celegans12:fix/agc-tessellation-hull-bda-faults
Oct 14, 2026
Merged

oneandonlydean merged 3 commits into
boykopovar:mainfrom
Celegans12:fix/agc-tessellation-hull-bda-faults

Conversation

@Celegans12

@Celegans12 Celegans12 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

Stacked on #4201; please review the last commit only.

A hull (tessellation-control) program with a BDA read fails to compile with "BDA fault termination requires a barrier-safe tessellation-control execution protocol". The refusal dates from the first BDA commit and is now redundant: BdaInvocationsMayStop already switches any program that contains s_barrier to the non-stopping fault protocol, and a hull's only barriers come from guest s_barrier (EmitBarrier); the LDS-lock barrier runs before any guest code and wave-LDS barriers are compute-only.

  • The refusal is removed. A hull with s_barrier keeps every invocation to its barriers; a hull without one may stop a faulting invocation, since no barrier can be stranded.
  • BdaContracts asserted the refusal and now accepts a hull with s_barrier.

PPSA26344's terrain hull 0x80002fc700 has no s_barrier and reads guest memory through BDA. With this change, its replayed request recompiles completely (388,903 SPIR-V words) once GE_CNTL.BREAK_WAVE_AT_EOI is set in the request; the title clears that bit, which #4201 refuses and #4450 handles.

Tested

Based on main f265cd5 plus #4172 and #4201. Windows 11 (MinGW-w64 GCC 15.2.0) and Linux: configure -DBUILD_TESTING=ON, targeted test builds, ctest.

agc_driver_tessellation gains two hulls that scale their output by a float read through BDA, with and without s_barrier: both render correctly, and an unmapped read reports BDA access failed ... stage=7. Both this and agc_driver_bda_device fail on #4201's head with the refusal. The unmapped-read draws are skipped on llvmpipe, which segfaults on them although the loads are guarded by if (physical != 0) and removing just those guarded loads stops the crash; the mapped-read draws run there.

Not tested: console behaviour.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: feat(agc): rebuild the hull's relative patch index when it is used as data #4201
  • Unblocks: PPSA26344 Ghost of Yōtei (PPSA26344) #675
  • GPUs tested: NVIDIA GeForce RTX 3080 Laptop GPU, driver 610.62, Windows 11; llvmpipe (Mesa 25.2.8), WSL Ubuntu 24.04
  • AI-assisted: yes

@github-actions github-actions Bot added shader core/shader docs agc libSceAgc* labels Oct 12, 2026
@Celegans12
Celegans12 force-pushed the fix/agc-tessellation-hull-bda-faults branch from 47446b9 to 22be94e Compare October 12, 2026 02:06
@Celegans12
Celegans12 force-pushed the fix/agc-tessellation-hull-bda-faults branch from 22be94e to d537af1 Compare October 12, 2026 02:56
@Celegans12
Celegans12 force-pushed the fix/agc-tessellation-hull-bda-faults branch from d537af1 to 699733d Compare October 12, 2026 03:45
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 12, 2026
@blawesom

Copy link
Copy Markdown
Contributor

Rebase handoff — this PR is the 5th link of the hull/tessellation chain (it contains #4172 and #4201 underneath its own commit), so I stacked the rebase on the already-rebased chain: rebase/4114 → … → rebase/4201 → this. Authorship unchanged; applied cleanly.

Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, BUILD_TESTING=ON, AMD RX 9070 XT / RADV):

  • builds; tools/check_conventions.py --base origin/main passes
  • full suite: 952/952 passed

To pick it up (whole chain):

git fetch https://github.com/blawesom/AnyPS5 rebase/4114 rebase/4163 rebase/4172 rebase/4201 rebase/4222

Head is d321aca7b. Worth listing Depends on: #4114, #4163, #4172, #4201 in the description so the chain merges in order.

@Celegans12

Copy link
Copy Markdown
Contributor Author

Quick note: I restacked this on the new #4172 and #4201 heads. The only conflict was the order of the test calls in Tessellation.cpp; this commit's own diff is unchanged. On this head, agc_driver_tessellation, agc_shader_memory, agc_driver_graphics, agc_tessellation_factor_ring and agc_driver_bda_device pass on Windows (NVIDIA) and on Linux (llvmpipe, merged with main).

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.
@Celegans12
Celegans12 force-pushed the fix/agc-tessellation-hull-bda-faults branch from f33872b to c1e4a0b Compare October 14, 2026 03:06
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@oneandonlydean oneandonlydean self-assigned this Oct 14, 2026
@oneandonlydean
oneandonlydean merged commit c698dae into boykopovar:main Oct 14, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants