Repository navigation
perf(shader): emit both registration layouts of a pixel shader from one program - #4884
Conversation
|
I reviewed #4884 against #4757. Your retained interface program changes variant preparation; #4757 fixes a reproduced CPU collision in materialized-result memo reuse. The direct overlap is the shared test insertion points and Windows CMake test-property list. I see no duplicated implementation of the memo identity fix. I propose landing #4757 first, then rebasing #4884 while retaining the owned snapshot and exact Matches checks at both lookups. Both layout-pair CTests and agc_shader_snapshot_memo should remain registered, and the collision case should be checked with layout reuse enabled and with APS5_NO_PERF_LAYOUT_PAIR=1. I have not tested the combined branches. Would you agree to this order? AI-assisted: yes. — Codex |
|
Agreed: #4757 first. Once it merges I'll rebase #4884 onto it, keep the owned snapshot and the exact AI-assisted: yes. |
|
Checked on Linux (PR merged onto current main, scoped build + ctest of libSceAgc/libSceAgcDriver): |
…474) * fix(shader): verify materialized snapshot identity before memo reuse Ported from upstream PR boykopovar#4757. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): emit both registration layouts of a pixel shader from one program Ported from upstream PR boykopovar#4884. The libSceAgcDriver test hunks are not ported: those tests are not built by any macOS configuration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(recompiler): keep the front-end resource plan in the shader disk cache Ported from upstream PR boykopovar#4883. A warm launch loads the stored resource plan and skips the shader front end. ShaderDiskCache FormatVersion 32 -> 33. The libSceAgcDriver test hunks are not ported: those tests are not built by any macOS configuration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): compile reusable SRT expression plans Ported from upstream PR boykopovar#1341. The disk-cache plan decoder from boykopovar#4883 rebuilds the execution plan after a stored plan is decoded, since it is derived from the value graph and not stored. The libSceAgcDriver test hunks are not ported: those tests are not built by any macOS configuration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): keep only evaluable branch conditions in resource plans Ported from upstream PR boykopovar#5181. The plan values now live in an IrValueArena, so the disk-cache plan codec from boykopovar#4883 walks and fills the arena instead of a vector of owned values. The libSceAgcDriver test hunks and the docs hunk are not ported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): ignore unused descriptor fields for scalar buffer loads Ported from upstream PR boykopovar#5049. The test and docs hunks are not ported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(agc): cheaper shader-memory captures per draw (ShaderMemory half) Ported from upstream PR boykopovar#1193, ShaderMemory.hpp/.cpp of its last commit only: word-mask page marks, pooled pages with a last-page shortcut, and lazily copied 64-byte blocks for pages no recorded GPU work writes. Adapted for native guest memory: a lazy block is read through GuestMemory::Read instead of the guest address as a host pointer, and NativeGuestMemory gains a no-op FlushGpuWrites (it keeps no deferred GPU writes). The SRT evaluator half is superseded by boykopovar#1341; the draw cache commits are stacked on upstream #494/boykopovar#1191 and not ported. The capture profile line keeps its current fields. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(graphics): retain vertex descriptor decode plans (ShaderInputState half) Ported from upstream PR boykopovar#1313, Graphics/ShaderInputState only. The Metal draw path calls DecodeVertexStageInfo per draw, which now builds a VertexStagePlan and reads only the referenced, coalesced table words. The Vulkan DrawDecode/DrawCache hooks that keep the plan across draws are not ported (that driver is not built on macOS). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(graphics): retain vertex input layouts (Metal draw) Ported from upstream PR boykopovar#1316 and adapted to Metal. VertexInputCache (LRU per shader variant, shape-checked on every hit, a variant seen with two shapes is bypassed) now takes a layout builder, so MetalDraw.mm keeps the layout it built with the Metal format predicate instead of calling BuildVertexInputLayout on every draw. Vertex buffer addresses are still validated per draw. The Vulkan Context/VulkanDevice/Draw.cpp hooks and the libSceAgcDriver test are not ported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(shader): build and run the shader disk cache codec tests on macOS Registers anyps5_metal_shader_disk_cache with ctest in the Metal guest-replay configuration and adds a plan round trip: value arena, rebuilt SRT execution plan (evaluated against the original), phi blocks, indirect image tables, and the 9-field SamplerResource / 5-field SampledResourcePair encodings. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…on steps compileVariant ran the layout-independent interface passes (ApplyStaticInterface, identity and dead-code removal, ShaderInfoCollector) and the layout-dependent emission (binding allocation, SPIR-V emission, the registration word-limit fallback, validation) as one step on a program it consumed. Split it into prepareInterface and emitVariant, which take the program by reference, and keep compileVariant as their composition with the same emission-failure recording, so a later change can emit another layout from a program whose interface is already prepared. No behaviour change: compileVariant still runs both steps once per variant and still moves the compiled information out of the program. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ne program PrepareRegistered prepares every registered pixel shader twice: with the plan's layout, then with pushConstantSizeBytes = 0. The layout is not part of the source key, so both requests share one SourceEntry, but the first variant consumes the entry's front-end program. On a cold cache the second layout therefore ran PrepareResourceProgram again (translation, SSA, the fold and eliminator passes, SrtWalker, ResourceTracker) and the interface passes after it, although none of them reads the layout. Only decode, CFG and structurization were shared through the context (boykopovar#2526). A ShaderPreparationContext now keeps the program it last emitted from, after the interface step, together with its source entry. Another layout of the same source at the same code address resets the binding allocation and emits from that program, as the registration word-limit fallback already does within one variant, and copies the compiled information out instead of moving it. A different source or code address builds a program as before. Building another program through the context releases the kept one first, and the program dies with the context, so no program outlives the registration that built it (the rule of boykopovar#471). ResourcePrograms() counts the programs built through a context. Disk-cache loads and stores, the emission-failure memo and the variant order are unchanged. APS5_NO_PERF_LAYOUT_PAIR=1 restores the previous path; it is a neutral switch, so it does not change disk-cache keys. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add a --layout-pair mode to agc_shader_memory_tests, registered as agc_shader_layout_pair and agc_shader_layout_pair_off (the latter with APS5_NO_PERF_LAYOUT_PAIR=1), next to the frontend-pair tests. A pixel shader with a buffer load is prepared with a 128-byte push range and with none through one context, in both orders: without registration modules, with them at push offset 0 and at the fragment slot offset 128 that devices with graphics pipeline libraries use, and with a registration word limit that forces the fallback to plain modules. Each pair must build one program with reuse and two without, and each layout must match a compile of the same code made on its own source without a context: SPIR-V, binding layout, push range and push-data use, artifact fields and compiled shader information. The materialized result must also match an uncached Recompile. The context must not reuse a program translated at another code address or one built for another source at the same address, and it must release the kept program when it builds another one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs/dev/CONVENTIONS.md asks for C#-style template parameter names (TKey, TValue). recordEmissionFailure, added with the split of variant compilation, named its callable parameter Step; rename it to TStep and write the template header the way the neighbouring recompiler sources do. No behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The layout-pair check compared a subset of the prepared artifact: the fragment parameters only by count, and not the barycentric emulation, the runtime ABI version or the vertex interface fields. Its pixel fixture also had no interpolated inputs, so the fragment parameter list was empty on both sides. Compare every artifact field except the variant id, the fragment parameters element by element, and add a fifth configuration: registration modules at the fragment slot offset with two interpolated inputs, which must declare two fragment parameters in both layouts. With this, flipping the barycentric emulation flag, a fragment parameter's flat flag or the runtime ABI version on the reused path fails agc_shader_layout_pair, where the previous check passed all three. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reused program hands out its compiled information through IrProgram::CompiledInfo() const&, while compileVariant still moves it out with TakeCompiledInfo(). The layout-pair fixtures are pixel shaders whose user-data base, scratch size and parameter export mask are all zero, so the comparison with isolated compiles could not tell whether the copy kept those fields: a copy that zeroed them still passed. Translate a compute program with a user-data base of 2 and 64 scratch dwords and require the copied and the moved information to agree on every field. Zeroing those fields in the copy now fails agc_shader_layout_pair and agc_shader_layout_pair_off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
7028eda to
49de394
Compare
|
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.
|
What
On a cold cache, registering a pixel shader builds its front end twice.
PrepareRegisteredprepares every pixel shader through oneShaderPreparationContext, first with the plan's layout and then again withpushConstantSizeBytes = 0(ShaderRegistry.cpp:754). The layout is not part of the source key, so both requests share oneSourceEntry. But the first variant consumes the entry's front-end program (findOrCompileVariant, Recompiler.cpp:952-953). The second layout therefore runsPrepareResourceProgramagain (translation, SSA, folding and the eliminator passes,SrtWalker,ResourceTracker) and then the interface passes, although none of them reads the layout. #2526 shares only decode, CFG and structurization between the two calls.With this change, the context keeps the program it last emitted from, after the interface passes, and emits the second layout from it:
compileVariantis split intoprepareInterface(ApplyStaticInterface, identity and dead-code removal,ShaderInfoCollector) andemitVariant(binding allocation, SPIR-V emission, the registration word-limit fallback, validation).compileVariantis now the composition of the two and behaves as before. This is the first commit.bindingLayoutCompleteand emits from that program, which the word-limit fallback already does within one variant (Recompiler.cpp:488-494). The compiled information is copied with a newIrProgram::CompiledInfo() const&instead of being moved out.shaderHash, which loop-guard fault records carry.PrepareRegisteredcall, so no program outlives its registration (the rule of perf(shader): build a shader's front end once on first use #471). A compute pair (full, then partial threads, two sources) never holds two programs at once.entries.front(). Calls without a context (Recompile,CaptureResources,PrepareShaderwithout one) take the old path.APS5_NO_PERF_LAYOUT_PAIR=1restores the old path. It is listed inNeutralSwitches, so it does not change disk-cache keys.ResourcePrograms()counts the programs built through a context, for the tests.This does not fix #4399. There, simonbbby's RTX 3090 profile logs both registration variants of one ps5-opengl pixel shader (5,413 code dwords) at 1,273.6 ms and 1,207.2 ms. That shows the duplicate exists on a real shader and costs about as much as the first variant. But the title's ~2 s wait is lost to pipeline creation (~2.2 s), as MarcZX100 concluded in that issue. By default
RegisterShaderhands the preparation to the backgroundRegistrationPreparePool. The saving reaches frame time only where something waits for it: a draw or dispatch that needs a shader whose preparation is still pending (AwaitRegisteredPreparation),Driver::ResolveShaderAbiwhen it prepares an ABI that was not registered (inline, on the calling thread), andAPS5_SYNC_SHADER_PREPARE=1.Open PRs #4339 (wave64 layout choice inside
compileVariant), #4048 and #4475 (lines of the emission step) edit nearby lines. The conflicts are textual; whichever merges second needs a rebase. #4271 (IR reclamation aroundResourceProgram) and my #4883 (front-end resource plan in the disk cache) are merged, and this branch is rebased onto both. After a plan hit the entry has no program, so the first layout builds it through the context and this change lets the second layout reuse it.Tested
WSL2 Ubuntu (kernel 6.18), GCC, i9-14900KF, lavapipe, configured like CI (Release, SPIRV-Tools ON):
APS5_NO_PERF_LAYOUT_PAIR=1set for the whole run, made before the test-only follow-up commits, also had 0 failures and the same skips. The last commit only adds a check toagc_shader_memory_tests; its seven ctests pass on the head.tools/check_test_registration.pypasses.tools/check_skips.pyflagsguest_video_out_high_frame_rate("current display does not provide 120 Hz"); unmodifiedmain43e3a70 skips it the same way in the same tree.agc_shader_layout_pairandagc_shader_layout_pair_off(0.3-1.3 s each under load; also run insideagc_shader_memory). A pixel shader with a buffer load is prepared through one context with a 128-byte push range and with none, in both orders and in five configurations: plain modules, registration modules at push offset 0, registration modules at offset 128 (the fragment slot on devices with graphics pipeline libraries), a word limit that forces the fallback to plain modules, and registration modules at offset 128 with two interpolated inputs. Each pair must build one program with reuse and two without.UsesPushData, every artifact field except the variant id (fragment parameters element by element), and the compiled shader information. The materialized result must also match an uncachedRecompile. The test also checks that the context does not reuse a program across code addresses or across sources at one address, that it releases the kept program when it builds another one, and thatCompiledInfo()andTakeCompiledInfo()agree on a compute program with a nonzero user-data base and scratch size.agc_shader_layout_pairfails and_offpasses. Never reusing the program fails with "the second layout ignored the reuse switch", because it builds 2 programs asmaindoes. The other controls remove the source check (the SPIR-V differs from the isolated compile), the code-address check, the release before another build, or thebindingLayoutCompletereset ("binding layout already allocated"), empty the bindings of the copied compiled information, or change the barycentric flag, a fragment parameter or the runtime ABI version of the reused artifact. Zeroing the user-data base, scratch size and export mask of the copy fails both tests, since that check does not depend on the switch.agc_shader_memory_testsbuilds and passes on the refactor commit and on the perf commit alone.Performance: I have no PS5 and no game files, so there is no trace from the Performance report form. The closest equivalent without a title is a synthetic measurement:
PrepareRegistereddoes: one context,PrepareShaderwith a 128-byte push range (A), then with none (B). The program is a chain of buffer loads, f32 ALU and exec-masked branches.ANYPS5_NO_SHADER_CACHE=1, so the in-memory and disk caches are cold. The build is Release with SPIRV-Tools OFF, asrelease.ymlbuilds.maineachPrepareShaderfrees its own program inside A and B. On the branch the one program is freed with the context, so pair = A + B + teardown is the comparable total.main43e3a70 and this branch were built in the same tree and run alternately, 10 rounds (5 in each order), on the same machine in one session. Other builds moved the load average between 2 and 23 (median 6), so absolute times are noisy.Medians of each column, ms:
A is a few percent lower on the branch only because
mainfrees the first program inside A, while the branch frees it in the teardown column. B onmainalso includes freeing its program and B on the branch does not, so the pair column is the fair comparison.In every run, the SPIR-V and binding layout of both layouts hash the same on
mainand on the branch. With registration modules these synthetic modules exceed the 200,000-word registration limit, so each variant is emitted twice (module, then fallback), and that emission is most of what remains of B. How much a title gains depends on how much of its cold pixel-shader registration is front end; I have not measured a title. I can attach the harness source, the scripts that build and run it, and the raw JSONL (one line per process, with timings and output hashes). An earlier batch stopped the timer before the context was destroyed and reported 2-3 points more pair saving; this batch supersedes it.Native Windows 11 (WinLibs GCC 15.2.0, Release with tests) on an RTX 5080 (NVIDIA 616.92), same head: full
ctestwithANYPS5_REQUIRE_VULKAN=1, 1021 tests, 0 failed, 7 skipped astools/check_skips.py --job windowsexpects;agc_shader_layout_pairandagc_shader_layout_pair_offpass. Not tested with a title.main0c3dc9c for this pull request: it builds in the CI configuration and the 34 shader, pipeline-library, structurizer and control-flow tests (ctest -R 'agc_shader|agc_driver_prepared|agc_pipeline_library|agc_structurizer|control_flow') pass on lavapipe.main0b6d033 (head 7028eda is now 49de394). One conflict, inRecompiler.cpp: perf(recompiler): keep the front-end resource plan in the shader disk cache #4883 addedFrontEndBuilds()where this change addsShaderPreparationContext::ResourcePrograms(), and both are kept. The rest applied cleanly, and the diff againstmainhas the same added and removed lines as before (7 files, +239/-18).Since 0c3dc9c, perf(shader): reclaim unreachable cached source IR values #4271 makes
ResourceProgramclear block register state and reclaim unused values (PrepareShaderpassesreclaim = false), and perf(recompiler): keep the front-end resource plan in the shader disk cache #4883 loads a stored resource plan ingetSourcebefore the front end runs. Neither changes which program the context keeps or when it releases it. After a stored-plan hit the entry has no program, so the first layout builds one through the context and the second reuses it. I checked this with a cache directory that held only the stored plans of the--layout-pairfixtures: each pair built one program, or two withAPS5_NO_PERF_LAYOUT_PAIR=1, and both layouts matched their isolated compiles. The fixture's later checks assume no disk cache, as ctest runs it. The stored formats are unchanged, soFormatVersionstays 30. fix(shader): verify materialized snapshot identity before memo reuse #4757 was closed unmerged, so there is nothing of it to keep.Tests on 49de394, WSL2 lavapipe configured like CI: the full ctest had 0 failures out of 1112 tests, with 43 skipped;
tools/check_skips.pyflags onlyguest_video_out_high_frame_rate(no 120 Hz display), as before. All 29 tests ofctest -R 'agc_shader|shader|recompiler'pass, including the two layout-pair tests,agc_shader_memory,agc_shader_disk_cacheand both front-end pair tests.agc_shader_memory_testsalso builds and passes on the refactor commit and on the perf commit alone. The branch also merges cleanly with the newermain107190d, and that merge passes the full ctest on the same setup (0 failures out of 1114 tests, 43 skipped, the samecheck_skips.pyresult).Native Windows 11 (WinLibs GCC 15.2.0, Release with tests) on an RTX 5080 (NVIDIA 616.92), with
ANYPS5_REQUIRE_VULKAN=1: all 29 tests of the same filter pass. The full ctest had 1 failure out of 1135 tests, with 7 skipped ascheck_skips.py --job windowsexpects. The failure isguest_avplayer("enable video failed"), which fails intermittently in this tree (10 of 24 runs). Its executable is built only from libSceAvPlayer (which compiles in libkernel'sTimedWait.cpp), libc and the prebuilt FFmpeg, none of which this branch changes, andmain's Windows CI passes it.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