Skip to content

perf(graphics): retain vertex descriptor decode plans - #1313

Closed
TibixDev wants to merge 1 commit into
boykopovar:mainfrom
TibixDev:pr/vertex-descriptor-plans
Closed

TibixDev wants to merge 1 commit into
boykopovar:mainfrom
TibixDev:pr/vertex-descriptor-plans

Conversation

@TibixDev

@TibixDev TibixDev commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What

Retain immutable vertex semantic/read plans in the current draw decode. Read live attribute and buffer tables on each use, coalescing only adjacent referenced words. Preserve original semantic ordering, duplicate semantics, format overrides and exact guest-read coverage.

Rebuilt directly on main d146ec1b6, dropping #1311 and the old draw-plan stack. Preserve current raw-header handling, the short user-data header ABI, static preparation, and merged-stage hidden user words.

Tested

Fedora 44, GCC 16.2.1, Release:

  • Vertex-stage, PM4, graphics and prepared-graphics CTests pass in 3.20 seconds. Graphics uses RX 6600 RADV core/synchronization validation.
  • Fixtures cover changed roots/descriptors, immutable header ownership, duplicate/reordered semantics, all 32 descriptors, exact read coverage, guard-page boundaries, concurrent readers, static ABI with no guest reads and merged-stage hidden user words.
  • Same-binary benchmark using process CPU time against the exact decoder extracted from main, with identical live-table mutations and read tracking. Six alternating runs per mode, pinned CPU, 100,000 calls/case. For 1/4/16/32 semantics: main medians 0.152/0.465/1.812/3.162 µs; retained plan 0.113/0.121/0.149/0.195 µs; rebuilding the plan every call 0.149/0.197/0.277/0.346 µs. Checksums match. The one-semantic cold result is within measurement noise; these component figures do not establish title frame-time gains.
  • Conventions and diff checks pass.

No new isolated in-game performance result or Windows test. Full Linux/Windows PR CI requires the collaborator-dispatched workflow.

Checklist

  • Based on main; related work checked.
  • One topic.
  • No new code comments.
  • Unsupported paths still throw.
  • No notes, logs, images or third-party additions in the repository.
  • General behavior.
  • Depends on: none.
  • AI-assisted: yes.

Verified title integration

Verified runtime smoke test: a freshly NID-patched #1313 library (c1eaeb47f2a8cb515aa7a8b818012322a2702928d309c38a5877f55c8f91f6bd) differs from main and contains VertexStagePlan; /proc confirms that the game loaded it. With the identical experimental NGS dependency and APS5_VERIFY_DRAW_RECIPE=1, a 120-second Skyrim run reached the PRESS ANY BUTTON screen. The log reports 3,064 decode comparisons with zero differences and no unexpected exception. It ended at the imposed timeout (124). This is a correctness smoke test, not a title-performance or full-gameplay result.

@TibixDev

TibixDev commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased on c081eb90. Updated #1311 after #1134 merged. Non-indexed mesh draws retain the upstream null index descriptor; retained plans and their invalidation coverage remain intact.

Fedora 44 Release: agc_vertex_stage_tests, agc_driver_draw_plan_tests, agc_driver_tests, agc_driver_graphics_tests, agc_driver_pm4_tests build; the targeted tests and contribution checks pass. Windows CI is pending.

@TibixDev
TibixDev force-pushed the pr/vertex-descriptor-plans branch 2 times, most recently from 6f24deb to a7b3a6c Compare October 8, 2026 07:39
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 8, 2026
@TibixDev
TibixDev force-pushed the pr/vertex-descriptor-plans branch from a7b3a6c to 5afe2d4 Compare October 8, 2026 08:20
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 8, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 8, 2026

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work here. As it stands this can't land:

  • It conflicts with main in 10 files (DrawCache.hpp/.cpp, DrawCacheStatistics.cpp, DrawCapture.cpp, DrawDecode.cpp, Draw.cpp, Driver.hpp, ShaderInputState.hpp/.cpp, CMakeLists.txt), mostly because of #639: compileDrawStage is now materializeDrawStage, built from invocations prepared at registration.
  • It is stacked on #1311 (and through it on #1191/#1193/#1201/#1202/#1312), which oneandonlydean already asked to rebase onto current main (comment on #1311, 2026-10-09). This PR has to follow once the base is redone.
  • The numbers (0.89 → 0.52 µs/call, reads 1,852 → 570) come from a combined fork, not from this change alone against main. Please re-measure with this PR alone on top of current main.

Once the base is rebased, please rebase this one on top of it, rerun agc_vertex_stage_tests / agc_driver_draw_plan_tests and post the before/after for the vertex-plan commit only.

@TibixDev
TibixDev force-pushed the pr/vertex-descriptor-plans branch from 5afe2d4 to 726ad76 Compare October 12, 2026 04:50
@TibixDev

TibixDev commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebuilt this on main d146ec1b6 in 726ad7632, without #1311 or its old draw-plan API. The retained plan now belongs to the existing draw decode. It preserves the current short-header ABI, raw shaders, static prepared invocations and hidden user words for merged stages.

Vertex-stage, PM4, graphics and prepared-graphics tests pass; graphics runs with RADV core/synchronization validation. Added static-ABI/no-read and hidden-user-word coverage to the existing mutation, guard-page, ownership and concurrent-reader cases.

Isolated CPU measurements compare main's exact decoder with the retained plan and with rebuilding a plan on every call. Six alternating runs/mode, live table changes, read tracking and matching checksums: for 1/4/16/32 semantics, main 0.152/0.465/1.812/3.162 µs, retained 0.113/0.121/0.149/0.195 µs, cold 0.149/0.197/0.277/0.346 µs. The single-semantic cold case is inconclusive. These are component results, not a title FPS claim.

The seven-file branch is ready for another code review; title benefit and full platform CI remain separate outstanding validation. AI-assisted update.

Verified runtime smoke test: a freshly NID-patched #1313 library (c1eaeb47f2a8cb515aa7a8b818012322a2702928d309c38a5877f55c8f91f6bd) differs from main and contains VertexStagePlan; /proc confirms that the game loaded it. With the identical experimental NGS dependency and APS5_VERIFY_DRAW_RECIPE=1, a 120-second Skyrim run reached the PRESS ANY BUTTON screen. The log reports 3,064 decode comparisons with zero differences and no unexpected exception. It ended at the imposed timeout (124). This is a correctness smoke test, not a title-performance or full-gameplay result.

Separate immutable shader semantics from live vertex tables. Coalesce only adjacent referenced words and buffer descriptors, preserving exact read coverage and original semantic ordering.

Written with AI assistance.
@adriadam10
adriadam10 force-pushed the pr/vertex-descriptor-plans branch from 726ad76 to bff970a Compare October 13, 2026 09:08

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased on current main (one adjacent-line conflict in Draw.cpp with the paClVsOutCntl line, kept both). Checked on the rebased tree: build and the libSceAgcDriver/libSceAgc ctest scope pass, including agc_vertex_stage. The new test does not compile without the change, and swapping the buffer index in VertexStagePlan::Read makes it fail ("changed descriptor or root was not read"). The only failing test here, agc_driver_tessellation, crashes inside RADV's pipeline link on this host and does so on unrelated PRs too. Merging once the full Build is green.

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 13, 2026

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The full Build (windows job) fails on this head: agc_vertex_stage exits with 0xc0000135 (DLL not found) because the test links libc but its ctest entry does not put libc's directory on PATH. Please add, inside the if(BUILD_TESTING) block of agc_vertex_stage_tests in core/libs/prx/libSceAgcDriver/CMakeLists.txt, the same thing the neighbouring tests do:

if(WIN32)
    set_tests_properties(agc_vertex_stage PROPERTIES ENVIRONMENT_MODIFICATION
        "PATH=path_list_prepend:$<TARGET_FILE_DIR:libc>")
endif()

main also moved: Draw.cpp now conflicts on the paClVsOutCntl line (keep vertexInputs->Read(...) and main's | graphics.layerExports). Please rebase and push; the rest of the diff looked fine on review (new test fails without the change).

@gunerky

gunerky commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Additional Windows integration and boundary-test evidence for PR head bff970af554a1e928d476e55f217562c36262702, combined locally with main 0aa4cb1c2f2f8936f3985f905697b0070f1f7305. No commit or branch history was rewritten.

The single Draw.cpp conflict was resolved retaining both program.vertexInputs->Read(...) and main's | graphics.layerExports. I applied the requested Windows CTest PATH property and extended the existing Linux-only guardBoundary() fixture to Windows with VirtualAlloc, VirtualProtect(PAGE_NOACCESS) and VirtualFree. This tests a descriptor at the last 16 bytes and an attribute at the last 4 bytes of a readable page, followed by an inaccessible page.

Windows 11 Pro 10.0.26300, WinLibs GCC 15.2.0, Python 3.12.14, Ninja Release, BUILD_TESTING=ON, Windows-default SPIRV-Tools OFF, offscreen VideoOut. Graphics tests ran on NVIDIA GeForce RTX 3060, WMI driver 32.0.16.1742.

  • Full Windows build and libs: passed. The runtime libraries were built afresh; the guard probe records libc.prx and libSceAgcDriver.prx loaded from this new build directory.
  • Actual agc_vertex_stage CTest: 3/3, test times 0.16, 0.03, 0.03 seconds; its 30-second timeout is unchanged.
  • A guard-only probe passes. Separate temporary extra-byte attribute and descriptor read mutants are both rejected. Decoder bytes were restored exactly and the original runtime rebuilt; the clean probe passes again. No mutant remains.
  • Conventions against the stated main, test registration (787 executables, none unregistered), and whitespace checks pass.

Full CTest used --parallel 4 --timeout 120, preserving the existing per-test timeouts. The first integration at main dacd78525e537a779682032c0ace7f7af120af05 passed 1091/1098 with seven skips. After incorporating the unrelated NGS2/PosixForWebKit main updates, one full run passed 1091/1099 with seven skips and failed savedata_memory_metadata: std::filesystem::remove_all reported Access denied on its temporary cleanup directory and the process aborted (0xc0000409). That test then passed three serial retries. Full-suite recheck without source changes: 1092/1099 passed, 7 skipped, 0 failed. The failed run is preserved; the cleanup cause is not established or fixed here. The test's imports and changed-file scope show no AGC runtime/code dependency.

The follow-up delta is only the CTest environment property and Windows test-platform branch; the retained-plan implementation remains the PR author's work. The GitHub branch still requires the author's rebase/update. Linux/macOS, PS5 and games: not tested. No title-performance claim is made. AI-assisted: yes, local integration, test-platform adaptation and validation assisted by Codex.

RINNECODER added a commit to RINNECODER/AnyPS5 that referenced this pull request Oct 14, 2026
…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>
@oneandonlydean oneandonlydean self-assigned this Oct 14, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing for now: adriadam10's review from 10-13 (Windows PATH fix, the Draw.cpp rebase) is unanswered, it conflicts with main and the last full build is red on Windows. Reopen with those addressed and before/after numbers from a title scene if you pick it up again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* build needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants