Repository navigation
fix(recompiler): defer the TYPE check of a guarded read-only static V# - #6412
fdfawef324 wants to merge 5 commits into
Conversation
|
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.
|
6e89aba to
6b9410b
Compare
|
We rebased this PR onto
Tests, CPU: GPU, on the RTX 3060 (driver 591.86, Windows Server 2025), run on the build of 677b9af (before the label commit, which touches no GPU path): AI-assisted: yes |
|
Coordination question from #5879: the Kena capture is a fragment-stage case with stale, unwritten user-data registers; #6412 is explicitly compute-only and its current eligibility excludes pixel programs. I found no other open PR that defers this path. If #6412 lands, would you prefer a separate follow-up to extend the planner/materializer/fault handling to fragment shaders, or are you already considering that scope? I will avoid opening overlapping work until the boundary is clear. The fragment case would need its own regression for an inactive guarded read and an active read that faults, and must preserve the current failure behavior for stores/atomics and unsupported formats. |
|
Please keep the fragment-stage/Kena case from #5879 as a separate follow-up. #6412 remains compute-only: changed-EXEC plain buffer loads, no LDS/barrier, and the existing BDA fault and dispatch-reuse restrictions. I am keeping that eligibility boundary while rebasing this PR. You can proceed with the fragment work independently. It needs its own active/inactive-lane and completion-fault coverage; the compute tests here do not establish fragment behavior. AI-assisted: yes |
6b9410b to
9585842
Compare
|
Rebased the compute-only change onto Fresh results: all twelve Release targets built, nine CPU/mock CTests passed in 19.35 seconds, and eight real Vulkan registrations passed in 24.39 seconds with Vulkan required and no skips. The latter includes skipped/taken invalid static-TYPE loads and expected fault completion. Root hash-checked the exact executables and runtime dependencies before and after. Conventions pass and the merge into main @MarcZX10, fragment-stage/Kena #5879 remains a separate follow-up as discussed. The PR is a draft while the #6177 shared-area landing order is coordinated; that future fast-path composition must preserve active-lane evaluation of the faulting descriptor. No combined runtime result is claimed. AI-assisted: yes. Co-Authored-By: Codex noreply@openai.com |
|
I opened #7689 as the separate fragment-stage follow-up for #5879, as you suggested. It adds fragment active/inactive EXEC coverage. I listed #6412 as a dependency because the descriptor-fault API and completion path share implementation points; after #6412 merges, I will rebase the fragment change and preserve the cache format version. |
A compute shader can leave a V# whose TYPE (word3[31:30]) is not 0 in a static buffer slot that it reads only behind a lane-dependent guard, for example v_cmpx followed by s_cbranch_execz. The capture refused every such V# with "buffer descriptor uses an unsupported type", so the whole dispatch threw even when no lane runs the read. A static buffer is now eligible for a deferred TYPE check when: - the shader is a compute shader with no LDS and no s_barrier; - it reads the buffer only through plain buffer_load_dword, _dwordx2, _dwordx3 or _dwordx4, under an EXEC the program changed; - no such load is in a loop's continue block; - the target has the BDA fault ABI. The capture plan and the compiled variant both decide eligibility from the IR. Planning-only scalar SRT reads are skipped, as ResourceTracker skips them, and a program that cannot defer skips the walk. For an eligible V# of any nonzero TYPE, capture records an UnsupportedType status instead of throwing. The status is carried into the binding plan, its key and the snapshot hash. On each lane that runs the load with EXEC set, the load records an InvalidDescriptor fault (reason 8) and stops the invocation. Lanes with EXEC clear, and loads that are branched over, do nothing. Scalar, formatted, typed and sub-dword loads, stores, atomics, and loads under the entry EXEC still throw at capture. The RDNA 2 ISA requires TYPE 0 for a buffer (section 8.1.8, Table 37) and does not say what a load through any other TYPE does. None of this is measured on AMD hardware; the shader debt entry records it. CompiledShaderArtifact::deferredTypeFaultsOnly marks a fault buffer that exists only for these checks. The disk cache moves to format 37. Co-Authored-By: Codex <noreply@openai.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…onzero For a guest buffer element whose captured V# has the UnsupportedType status, the driver: - binds the empty placeholder buffer; - requires the compute fault contract; - turns fault checks on, so a lane that ran the load fails the dispatch at completion; - makes the resource object never reusable and refuses to move its buffers; - adds the status to the content key. A shader whose fault buffer exists only for deferred TYPE checks (deferredTypeFaultsOnly) leaves fault checks off while every captured V# has TYPE 0, unless APS5_LOOP_GUARD is set. Those dispatches keep GPU-signalled labels and the dispatch-recipe path. Co-Authored-By: Codex <noreply@openai.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- agc_shader_memory (verifyDeferredStaticBufferTypes): a buffer_load_dword behind v_cmpx/s_cbranch_execz through a V# of TYPE 1, 2 or 3 captures with the UnsupportedType status, a fault buffer and a deferred-only marker. A TYPE 0 capture carries no status. The same read under the entry EXEC, or on a target without the BDA fault ABI, is still refused. - agc_shader_deferred_static_planning_reads: a planning-only scalar read of register quad 3 or 0 neither indexes past the static buffers nor keeps the read-only buffer from deferring. - agc_driver_guarded_null_pointer runs the shader on the GPU. With TYPE 0, the lane that passes the guard reads the buffer. With TYPE 2 or 3 and no lane passing, the dispatch completes without a fault. With one lane passing, completion reports "BDA access failed" with reason 8 at the load's pc. - agc_driver_graphics (BdaResources): a deferred-only fault buffer is polled only when a captured V# has a nonzero TYPE; a shader with other fault sources is still polled. - agc_shader_disk_cache round-trips the binding status, the buffer flag and the artifact and info flags. The shader debt entry cites RDNA 2 ISA sections 8.1.8 and 8.2.6 and records what is not measured on AMD hardware: whether an executed load through such a V# faults, returns zeros or reads through the base, and whether lanes with EXEC clear are unaffected. The driver debt entry records the cost of the fault buffer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ShaderResources::ContentKey writes each binding's bufferDescriptorStatus count and entries between the bufferWritten bits and the sampler count, but LabelShaderKey, which ShaderResources::DiffContentKeys uses to name what changed between two cache keys, did not know that word. It read the status count as the sampler count and the sampler count as the shape count, so every binding after the first was parsed from the wrong offset and a real key came back unlabelled or with the wrong component. LabelShaderKey now labels the status count and its entries as Other, like the neighbouring bufferWritten bits. KeyMiss.cpp builds its hand-built keys with the status word, and a new test checks them word for word against ShaderResources::ContentKey for dispatch, table and status-carrying bindings, so the hand-built layout cannot drift from the real one again. Another new test checks that a changed status entry is labelled Other and that the V#, image and sampler words of the bindings around status entries keep their own labels, for dispatch and draw keys. Without the parser change the first labelled key already fails with "equal keys were not comparable". Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Parse the sampler-coordinate flag vector introduced on main before the deferred descriptor status block. Keep the synthetic key fixture aligned with ContentKey. Co-Authored-By: Codex <noreply@openai.com>
9585842 to
c050c17
Compare
|
Rebased the five-commit stack onto pinned main Nine CPU/mock CTest registrations passed in 14.83 seconds: Current-head managed-slot Vulkan checks passed all eight registrations / seven unique executables in 20.65 seconds without skips: guarded_null_pointer, bda_device, bda_span_reads, buffer_descriptor_runtime, gated_dispatch_capture (ordinary and short-timeout), loop_ending_exits and subvector_loop. ANYPS5_REQUIRE_VULKAN=1 was set; exact runtime hashes were verified before and after. The guarded fixture exercised skipped/taken unsupported static-TYPE reads and the expected InvalidDescriptor reason8 completion faults. A clean merge into later main @MarcZX10, thanks for keeping the fragment follow-up in #7689 separate and recording the dependency. This update remains a draft while the future composition with #6177 is coordinated: its robust-direct path must preserve the active-lane guard or exclude deferred descriptors, with skipped/taken robust-mode fixtures when the branches combine. No combined build or landing-order agreement is claimed. Co-Authored-By: Codex noreply@openai.com |
|
First AMD-on-Linux (RADV) run of this PR (and the first non-NVIDIA one, as far as I can see). Merged onto AMD Radeon RX 7900 XT (RADV NAVI31, gfx1100), Mesa 25.2.8, Linux Mint 22.3 (Ubuntu 24.04 base), kernel 7.0.0-38, GCC 13.3, Release. Full suite ( The new GPUs tested: AMD Radeon RX 7900 XT (RADV NAVI31, gfx1100), Mesa 25.2.8, Linux Mint 22.3 / kernel 7.0.0-38 AI-assisted: yes |
|
First AMD-on-Linux (RADV) run of this PR. Merged onto AMD Radeon RX 7900 XT (RADV NAVI31, gfx1100), Mesa 25.2.8, Linux Mint 22.3 (Ubuntu 24.04 base), kernel 7.0.0-38, GCC 13.3, Release. Full suite ( The PR's new ctest entry, This checks correctness only; I did not measure performance. GPUs tested: AMD Radeon RX 7900 XT (RADV NAVI31, gfx1100), Mesa 25.2.8, Linux Mint 22.3 / kernel 7.0.0-38 AI-assisted: yes |
What
A guarded read-only static V# of nonzero TYPE currently rejects a whole compute dispatch during capture even when no lane runs its load. This change captures that descriptor with a status and faults only an active lane that executes the invalid load, using the existing InvalidDescriptor completion contract.
Eligibility remains limited to compute shaders without LDS/shared accesses or barriers, plain DWORD loads under changed EXEC, no continue-block load, and the BDA fault ABI. Stores, atomics, scalar/formatted/typed reads and entry-EXEC loads retain capture rejection. Valid descriptors keep existing completion and recipe paths; invalid captured descriptors force fault polling and prevent resource reuse/moved-buffer recipes. MarcZX10's fragment/Kena follow-up #7689 addresses #5879 separately and lists this PR as a dependency; it is not included here.
This rebase retains main's SharedValues/table memo, buffer extents, depth-remap plan and serializer fields. It moves the cache to format 41 (pinned main 40), serializes bufferExtent before bufferDescriptorStatus, and preserves all ten shader-key flag vectors including samplerCoordinatesIgnored before the added descriptor-status block. Active invalid-TYPE hardware behavior remains unmeasured and documented as debt.
Nearby #6177 has a source-supported future-composition risk: its robust-direct fast path can evaluate a faulting descriptor outside the active-lane guard. This is not a reproduced defect in #6177 alone. Combining the branches must retain that guard or exclude deferred descriptors from the fast path and rerun skipped/taken fixtures in robust mode. Proposed order is this PR first; coordination is pending, so this update is a draft. No combined build or landing-order agreement is claimed. Other shared cache/reuse changes must retain the descriptor status in identity and the deferred-fault nonreuse guards.
Tested
Exact candidate
c050c17557af9c9dd4275b97c8adcb9586a66c5con pinned main676d423968a1a8fcc79c6247020ce22090a8e0d4, Windows Server 2025 10.0.26100, MinGW-w64 GCC Release-O3.Nine CPU/mock CTest registrations passed in 14.83 seconds:
agc_shader_memory,agc_shader_bindless,agc_shader_deferred_static_planning_reads,agc_shader_disk_cache,agc_shader_control_flow(ordinary, iteration-selection-off and forced-fallback),agc_driver_key_miss, andagc_driver_graphicsincluding BdaResources mocks. No case skipped. All twelve requested targets complete in Release-O3; the final incremental continuation passed in 93.29 seconds after a preserved 900-second capped partial chunk. The integrated disk-cache test also passed separately before the full build, confirming source version9a919d1fcfb2ebe4, format 41 and all 17 plan round trips. CPU checks and the continuation used the independently reviewed and runtime-probed four-core CPU partition with the same 4 GiB job cap; this is not a title speed measurement.The actual-header Windows layout probe reports ShaderInfo 24 fields, BufferResource 13, ImageResource 39, DescriptorBinding 680 bytes, ShaderInvocation 208 bytes and RecompileResult 480 bytes. It is separate from the integrated codec and runtime checks, and is not a Linux execution result. Conventions and diff checks pass against the pinned base.
Independent adversarial source review passed at this exact head after checking all24 source pins and the complete rebase range diff. The prior published head
9585842dbpassed nine CPU/mock and eight managed-slot Vulkan registrations; those are historical after this rebase. Original selective CPU red controls are also historical and were not rerun on this head. Synthetic project fixtures are retained; no game shader payload is attached.Demon's Souls (PPSA01342 #1283) motivated the static-descriptor investigation. The previous public body at
6b9410b92bbd9d74bf3f32ebbbd2b91a0be08f18records a historical offline comparison on mainb3f9f89e62979d7328ff0dec2d14b006f8c2c0ab: main refused capture, while an earlier PR stack materialized successfully in three runs. That report does not record the full integration-stack head, and used a replay-tool option from #5959 that is not part of this PR. It is historical preparation/materialization evidence, not current-head or GPU verification. No request or shader payload is attached.Current-head managed-slot Vulkan checks passed all eight registrations / seven unique executables in 20.65 seconds without skips: guarded_null_pointer, bda_device, bda_span_reads, buffer_descriptor_runtime, gated_dispatch_capture (ordinary and short-timeout), loop_ending_exits and subvector_loop. ANYPS5_REQUIRE_VULKAN=1 was set; exact runtime hashes were verified before and after. The guarded fixture exercised skipped/taken unsupported static-TYPE reads and the expected InvalidDescriptor reason8 completion faults. A clean merge into later main
72a3d9f2a49dwas checked separately; that merged tree was not executed. No current title milestone or performance improvement is claimed.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
Co-Authored-By: Codex noreply@openai.com