Repository navigation
fix(agc): retire a multisampled color target the title wrote over - #8223
Conversation
github-actions
Bot
added
the
unblocks title
|
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.
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.
|
|
Native Linux write-watch control for exact head
All nine parent controls stop at After restoration, the seven registered tests matching Ubuntu 24.04, kernel 7.0.0-38, Ryzen 9950X, GCC 13.3, Release; NVIDIA 595.99.02, gfx1036 |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eb9807f to
79b42bd
Compare
|
Rebased onto On the RX 9070 XT, Windows, with the validation layer:
@simonbbby thank you for the Linux control with write watching on three ICDs. I added it to the description. AI-assisted: yes (Claude Code) |
|
@savvaniss thanks for #8223. We hit a second form of the same reuse in PPSA03208, one that the CPU-write evidence can't see: the title reuses the memory through the GPU. From a trace on our bring-up tree (
Nothing wrote over the colour bytes from the CPU, so the write-tracker generation never changes, and as we read #8223 it would keep the guard here. To get past it, we used an unconditional retire-on-read as a probe. The title then runs about 20 s further, to its next, unrelated stop. We don't think that rule is safe for a live target, so we're not proposing it. Could #8223 accept a second piece of evidence? When a native depth/stencil target (or a colour target with a different layout) claims a range that overlaps a guarded multisampled colour target, and the colour target's last use came before that claim, retire the colour guard through the same retirement #8223 adds for overwritten targets. That's the GPU-side counterpart of "the title wrote over it". A colour target that is still drawn after the claim would keep its guard and still refuse. For the test, the existing fixture could:
plus a control where the colour target is drawn again after the depth bind and the read still refuses. We're happy to run the title on the result. |
|
@savvaniss #9186 changes the same function for a different case: a multisampled texture that a shader sampled before any draw, later accessed by the GPU with no CPU write in between (Death Stranding 2 stops there even with #8223). The two conflict in I'm happy for #8223 to land first and then rebase #9186 onto it. If you'd rather go second, I can point you at the merged version. Which order works for you? |
… over A title can reuse the memory of a multisampled color target through the GPU instead of the CPU: it binds a depth or stencil plane, an HTILE or a color target of another layout over it, with no CPU write in between. Each bind now marks the guarded targets it overlaps as claimed, and a draw of a target clears its own mark. The next guarded access of a claimed target retires it the way a CPU write over it does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…itten-msaa # Conflicts: # core/libs/prx/libSceAgcDriver/Graphics/src/DepthSurface.cpp
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean thanks for the trace. The rule is in 93a7629 (with
On One trade-off is in the debt entry: a bind is taken as reuse. So if a title binds another target over a live multisampled target and afterwards only samples that target, without drawing to it, the samples are lost at the next guarded access. |
|
@fma-rc thanks for merging the two. Yes: #8223 first, and #9186 rebases onto it, as you offered. Your merged version needs one more pass, though. Today's push changes the same code again for oneandonlydean's request:
This does not cover your case. DS2's storage access over a texture that was sampled before any draw has no bind over it, so #9186 is still needed after #8223. |
|
@savvaniss a merge-order note: #9533 (a draft) overlaps this PR in |
|
@EagleBearerYS thanks for the merge-order note. Understood: #9533 goes after this pull request and rebases onto it, keeping its claims and retires, and the handling of a retired colour target over the stencil plane is on #9533's side. Nothing changes here. |
|
@savvaniss we'd like to merge this before the MSAA stack (#6973 → #7002), which needs a rebase of its own after #7948, so its author can build your retirement into the stack's publish path in the same pass (I've asked them on #6973). That needs this branch updated first: it now conflicts with
Could you merge |
A dispatch's store to a 2x target after a stencil plane was bound over it keeps the target guarded. A stencil plane bound over a one-fragment target's FMASK leaves its guard refusing, as on main. The FMASK check follows Amoenus's adaptation of this branch (25907aa). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean I merged
Tests on the RX 9070 XT, Windows 11, with
The PR description has the full list and each failure message. AI-assisted: yes (Claude Code) |
|
@Amoenus thanks for the adaptation. I compared today's merge with 25907aa:
@hugodearaujo thanks for the Mirage run. Getting past the texture read at 89 s is what this PR is for. The second stop is not in its scope:
AI-assisted: yes (Claude Code) |
|
@savvaniss I traced that range, and a dispatch wrote it: the refused access is that write. Diagnostic build:
With the diagnostic retiring every refusal, the next refusal in the run is the same case. It is a storage image access of 0x10e2f80000+0xbf0000 over the 4x target 0x10e3a70000+0x480000 (960x540, format 13). The image is 1920x1080, format 36,
So the target's last reader has run when the store overwrites it, and the refusal comes back every frame, so the target is drawn again each frame. This overlap is partial too (0x100000 of 0x480000), so #10049's rule wouldn't fire here either. After that the run reaches 150 s, but with the guard retiring everything that says nothing about the rendering. |
|
Linux validation of exact peer head RX6650XT / RADV Mesa26.2.4, Linux7.2.9, Ryzen7950X, Release. Same named19 focused checks pass on baseline and candidate: the sampled-target, native vertex-input and FMASK cases plus the16 prerequisite/clear/depth checks. Candidate sampling executes PPSA27624 Bendy01.000.003: the baseline Linux fault-recording run reached the office corridor, then a right camera turn hit a CPU-side16MiB read overlapping a32MiB host-only8-sample color target. The runtime error propagated through presentation/submission and aborted. With this private integration, eight freshly converted outputs and the same ordinary doorway save/load, approach, save-before-OPEN and corridor-turn sequence completed420s with native exit0, no guard/driver failure in the log, reviewed post-turn X11 image, and complete save/config/kernel restoration. Limits: one successful candidate run; the baseline fault capture used GDB and the candidate replay did not. This does not establish broad stability or a performance gain, and does not resolve the separate earlier intermittent SEGV. Live-sample readback and the retirement-rule gaps documented here remain outside this result. Source composition, baseline/candidate focused logs, conversion hashes and reviewed scene images, downloaded SHA256 verified: Identical native repeat: the second run on the same candidate |
|
@savvaniss merged main into your branch to resolve the conflict in |
✅ NVIDIA GPU test: passedPR head
Failing on main too (3, not counted against this PR)Validation IDs already present on main (4 IDs in 404 tests)Skipped (39)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
@savvaniss heads-up: #6498 just merged and now conflicts with this PR in |
|
A test on Assassin's Creed Mirage (PPSA13960) on current
So this PR removes the title's first multisampled-target stop on For the refresh oneandonlydean asked for: merging current AI-assisted: yes (Claude Code) |
Keeps both sides of the debt doc: this branch's multisampled color target entry and boykopovar#6498's shader buffer entry. The test file merges with both sides' test blocks. HostOnlySampleRanges from boykopovar#12566 reads the host-only flag from the range's TargetUse. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean this is ready for CI and the game gate. I merged
Tests on the RX 9070 XT, Windows 11, with
AI-assisted: yes (Claude Code) |
|
@hugodearaujo thanks for the Mirage run and for tracing the second stop, and for the The second stop is not in this PR's scope. Your trace settles what it is: the refused access is compute shader 0x1000810a00's own store-only clear over 0x200000 of the 4x target's 0x480000 bytes, and nothing else touches the range in between. That is a GPU write, while this PR's evidence is a CPU write or a bind of a depth or stencil plane, HTILE or color target. So the guard refuses there as this PR's rule says it should, and your second case (0x10b4c89700) has the same shape. A store-only storage image would be a third kind of evidence, so I'd keep it for a PR of its own after this one lands. As you found, #10049's full-coverage rule wouldn't fire on these partial overlaps. Such a PR could call the @JJenkx thanks for the Linux run on the RX 6650 XT, and for repeating the Bendy sequence. Your repeat is the useful part: the 16 MiB CPU read over the 8-sample target still stops there, so I'm not listing this PR as a fix for that stop. This PR only retires a target when there is evidence the title reused its memory. A read of a target with no such evidence still refuses, and publishing live samples is what #6987, #6989 and #6995 work toward. AI-assisted: yes (Claude Code) |
What
A multisampled color target keeps its samples on the host. Its guest memory stays guarded until an unmap or an allocation removal covers it: any access of that memory throws "overlaps multisampled color target … guest access to it is not implemented". A title with its own memory pool never unmaps. It reuses the memory of a render target it no longer needs, and the next access of the new data throws, although the target is dead. This PR takes two kinds of reuse as evidence that a target is dead:
CollectWritesover its color memory each time it is used. When a guarded access overlaps a target in write-watched memory, and the CPU wrote the overlapped bytes since that generation (UnchangedSinceCollected), the title has put other data there.DepthSurfaceView, before any import), a single-sample color target (whereDrawalready retires the depth surfaces it overlaps), and a draw of a multisampled color target with another address, extent, sample count or format. A draw through the target's sRGB twin format is served by the same target, so it is a draw of that target, not a claim.A target's use clears its own mark and takes a new write generation. A use is a draw, and, as #9186 already holds a target host-only there, a dispatch's storage view of it or a color fill it absorbs. A target used again after the claim keeps its guard.
Since #9186, a guarded access also retires a target whose samples are not only on the host, that is a texture sampled before any draw, until a draw, a storage write or an absorbed fill. The three conditions sit together in
RequireNoMultisampleColorTarget, in #9186's order: when every guarded range that an access overlaps can retire, they are all retired the way an unmap retires them, and the access goes on; when any of them cannot, the access refuses and nothing is retired. A later draw to the same address creates and guards a new target. A target is only retired at an access thatmainrefuses with an exception, so nothing that runs onmainbehaves differently.Each target keeps this state in one
TargetUse(hostOnly,writes,claimed), and every guarded range of the target points to it, the DCC, CMASK and 8-sample FMASK ranges included. The one-fragment FMASK range ofGuardOneFragmentFmaskkeeps itsfmaskflag and gets aTargetUseof its own, but claims skip it, so it refuses every guest access as onmain.guard()returns void as onmain; the color footprint it computes isguardedColorBytes(), which the use stamp also calls.The debt entry for multisampled color targets records the rule and its gap: a target that is only sampled after another target was bound over it loses its samples. #6987, #6989 and #6995 work toward reading live samples from guest memory, which this does not do. Amoenus's adaptation of this branch onto
main5a2744c (25907aa1fb) was the reference for the one-fragment FMASK check in the test.Tested
Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, AMD Radeon RX 9070 XT, on
mainb7488e1 (merged into this branch on 30 Oct).agc_driver_sample_multisample_targetrunsCheckOverwrittenTargetandCheckClaimedTargetsaftermain's sampling test. The first is the CPU-write case: a write over a drawn 4x target in watched memory retires it at the next guarded read, and a later draw guards the new target. The second has the bind cases, in plain (unwatched) memory:holdHostOnlystamps no use, case 5: "a 2x target that a dispatch stored to after a D32S8 target's stencil plane was bound over it is not guarded";CheckOverwrittenTarget: "a read of target memory the title wrote over … overlaps multisampled color target".agc_driver_sample_multisample_targetonmain's production code stops 3 of 3 at the CPU-write case ("a read of target memory the title wrote over … overlaps multisampled color target").mainand on this head, on the same machine, withANYPS5_GPU=AMD,ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, 31 tests:agc_driver_sample_multisample_target,agc_driver_native_msaa_vertex_input,agc_driver_native_fmask,agc_multisample_guest_access(withmain's one-fragment FMASK guard tests),agc_driver_msaa_dcc_state,agc_driver_msaa_dcc_clear_2,_4and_8,agc_driver,agc_driver_cached_image_multisample_guard,agc_driver_multisample_fill_host_only,agc_driver_multisample_retirement,agc_driver_multisampled_pass_order,agc_driver_multisample_readbackand its eight cases,agc_driver_multisample_srgb_twin,agc_driver_multisample_centered_lines,agc_driver_msaa_layout,agc_driver_depth_copy_draw, and the native depth testsagc_driver_native_multisample_depth,agc_driver_native_depth_retirement,_clears,_clear_refusalsandagc_driver_native_depth_bda_clear. On this head, onmainb7488e1, all 31 pass, andagc_driver_sample_multisample_targetpasses 3 of 3. Onmain07da04f (25 Oct, not rerun for the latest merge), all 31 pass, and itsagc_driver_sample_multisample_target(without this PR's cases) passes 3 of 3. Neither log has aVUID-line orwas not found but was requested by env var VK_INSTANCE_LAYERS, and everySelected GPUandVulkan device:line names the RX 9070 XT.main's newagc_driver_packed_unorm4444_multisample_partial_writedoes not build in this tree on either side: its object's dependency file path passes Windows' 260-character limit.maincecb540 (21 Oct): the only conflict was thedepthWrittenparameter thatmainadded toDepthSurfaceView.mainhad moved the guard loop intoRequireNoMultisampleColorTarget, and the retirement moved with it.main1da03c6 (22 Oct, after feat(agc): expose expanded FMASK for tracked native owners #7948): feat(agc): expose expanded FMASK for tracked native owners #7948's FMASKguarded()range takes the target'suse, like the DCC and CMASK ranges. In the test'sBlock, both sides had added the samewatchedparameter, andmain's version is kept.main07da04f (25 Oct, after fix(agc): retire an undrawn filled multisampled color target that a guest access overlaps #9186 and the one-fragment FMASK guard): conflicts inMultisampleTarget.hpp,Draw.cpp,MultisampleTarget.cpp, the test and the debt file, resolved as described above. InDraw.cppand the header, the claim comes beforeGuardOneFragmentFmask.mainb7488e1 (30 Oct, after fix(agc): refuse buffer aliases of native MSAA targets #6498 and fix(agc): write host-only multisample memory as zeros in a capture snapshot #12566): the debt doc keeps this PR's entry and fix(agc): refuse buffer aliases of native MSAA targets #6498's shader buffer entry, and the test file keeps both sides' test blocks.HostOnlySampleRangesfrom fix(agc): write host-only multisample memory as zeros in a capture snapshot #12566 readsrange.use->hostOnly(found by hugodearaujo).main, a native multisample stencil plane is not imported, so the test reads the overlapped bytes throughRequireNoMultisampleTargetinstead of through that import. The claim happens before the import.mainbb04a4f it aborts at 86 s on a CPU read over a 4x target, and with this PR that stop is gone (comment). It then aborts at 79 s on a cached image access over another 4x target. Their trace shows that access is a dispatch's store-only clear over part of the target, a GPU write that is neither a CPU write nor a bind, so it still refuses and is a follow-up.main'sMultisampleTarget.cppfails 3 of 3, comment) also covered the CPU-write case only.tools/check_conventions.py --base origin/mainpasses, andtools/pr_overlap.pyfinds no shared exports.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)🤖 Generated with Claude Code