Skip to content

perf(shader): compile reusable SRT expression plans - #1341

Open
TibixDev wants to merge 3 commits into
boykopovar:mainfrom
TibixDev:pr/compiled-srt-plans
Open

TibixDev wants to merge 3 commits into
boykopovar:mainfrom
TibixDev:pr/compiled-srt-plans

Conversation

@TibixDev

@TibixDev TibixDev commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What

Compile supported immutable SRT expressions into indexed operations, so each draw evaluates current user data and guest memory without rebuilding an IR walk. Preserve descriptor values, flattened slots, active sources, read order and dependency traces.

Rebased on main d146ec1b6. Null user-data roots now return zero words and preserve nullRootReads, without guest reads or dependency records. Scalar-buffer reads outside the descriptor bounds return zero, matching current main. Successful compiled evaluations clear stale poison output. Plans with guarded slots or memory-derived handles use the generic evaluator, preserving inaccessible-read poison and descriptor-zeroing behavior. Conditional flow, specialization memory, cycles and unsupported operations also use the generic evaluator.

APS5_VERIFY_COMPILED_SRT=1 compares null-root and poison outputs in addition to values and traces, and forwards accessibility probes with their original callback context. APS5_NO_COMPILED_SRT=1 disables compiled evaluation for comparisons.

Supersedes the SRT evaluation portion of #1193; its other shader-memory changes remain separate. Depends on: none.

Tested

Fedora 44, GCC 16.2.1, Release:

  • Built agc_srt_execution_tests, agc_shader_memory_tests, agc_driver_tests, agc_driver_graphics_tests and agc_driver_pm4_tests.
  • agc_srt_execution passes normally, with compiled-result verification, and with compiled evaluation disabled. CTest registers it through the existing test loop with a 30-second timeout.
  • Differential rows cover 128 changing roots, null roots with each reporting-pointer combination, out-of-bounds scalar-buffer reads, poisoned nested reads including slot/PC/address, guarded fallback, traces, failures, cycles and unsupported operations.
  • A translated shader is prepared with PrepareShader, instantiated through PreparedShaderInvocation, and captured through that invocation. Its direct root read selects a compiled plan; its null-root capture reports one null read and touches no guest memory. The existing guarded translated-shader test requires generic fallback.
  • Shader-memory, driver, graphics, PM4, frontend-pair on/off and SRT read-patching CTests pass with APS5_VERIFY_COMPILED_SRT=1 (RX 6600 RADV for Vulkan tests).
  • Contribution checks and git diff --check pass.

Fresh measurements on this base, identical Release binary and flags, warm-up excluded:

Workload Generic Compiled
Prepared-invocation capture, synthetic vertex shader, 0 descriptor sources / 1 read; ABBA runs 0.2142–0.2197 µs 0.09493–0.09518 µs
Synthetic scalar-address evaluation, 8 sources / 72 reads; alternating paired runs 5.565 µs median 0.6191 µs median
Synthetic scalar-buffer evaluation, 8 sources / 72 reads; alternating paired runs 5.998 µs median 0.5775 µs median

Prepared-capture checksums match across all four ABBA runs. Reproduce with agc_srt_execution_tests --prepared-benchmark, alternating APS5_NO_COMPILED_SRT=1 and the default; --benchmark runs the paired evaluator fixtures.

These are synthetic evaluator/capture measurements, not title FPS. They replace the stale measurements against #1193 in the previous description. A fresh title-derived capture, title-level performance and Windows runtime validation remain outstanding; guarded nested-read plans receive no compiled-path speedup.

Checklist

  • Based on main at the revision above; competing perf(agc): cheaper shader-memory captures per draw #1193 and its overlap stated.
  • One topic.
  • No new code comments.
  • Unsupported expressions retain generic evaluation and its errors.
  • No notes, logs, or images added to the repository.
  • No new third-party code.
  • General behavior.
  • Depends on: none.
  • AI-assisted: yes.

@TibixDev

TibixDev commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on bc1fbe81. Kept the upstream buffer-device-address tests alongside compiled SRT coverage.

Fedora 44 Release: agc_srt_execution_tests, agc_shader_memory_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/compiled-srt-plans branch from 57b5d0c to 202d5f5 Compare October 8, 2026 07:13
@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

@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. This conflicts with main and the conflict is not only mechanical:

  • core/shader/recompiler/Optimization/src/SrtWalker/SrtDescriptorEvaluation.cpp:51 (main): EvaluateRuntimeSourcesImpl now takes std::vector<SrtReadPoison>* poison and std::uint32_t* nullRootReads (#2989, reads through a null user-data pointer return zero words). SrtExecutionPlan::Evaluate produces neither, so the compiled path would diverge from the generic one on those cases.
  • ResourceMaterializer.cpp (main) now computes plan.guardedSrtSlots next to where this PR sets plan.executionPlan; the compiled plan has to respect guarded slots too, or fall back when any is set.
  • core/libs/prx/libSceAgcDriver/CMakeLists.txt: adjacency with agc_shared_write_tests and the test foreach list; just a rebase.

What would fix it: rebase on current main, port the plan to the new signature (poison, null-root reads, guarded slots) or make CompileSrtExecutionPlan return null when they apply, extend tests/SrtExecution.cpp with a null-root and a poisoned-read row compared against the generic evaluator, and re-run the paired benchmark on top of main (capture now runs against the prepared invocation since #639, so the 18 sources / 72 reads graph may have changed). Also register agc_srt_execution_tests via the same loop as the other tests, as it is now, but check it still lands in ctest after the rebase.

@NFSS10 NFSS10 mentioned this pull request Oct 11, 2026
7 tasks done
Compile supported immutable descriptor expressions into a compact indexed operation list. Evaluate changing user data and guest memory without walking the IR or rebuilding hash and cycle tables on every draw. Preserve read ordering and dependency traces; retain the generic evaluator for specialization, conditional flow, cycles and unsupported operations.

Add differential fixtures for values, arithmetic boundaries, read failures, nested pointers, inactive sources and generic fallback, plus opt-in captured-input verification. Six suites pass. An alternating warm fixture improves descriptor evaluation from about 5.8-6.2 us to 0.6-0.7 us. Skyrim menu and saved Helgen gameplay complete over 1.8 million compiled evaluations against the generic walk with zero mismatches and no Vulkan validation errors.

Written with AI assistance.
Use the IR operand schema when validating memory operations. Scalar address loads carry four operands; the two-operand restriction unnecessarily sent their descriptor plans through generic evaluation.

Validate fixture operand shapes and require a translated scalar-address shader to compile its SRT plan. Differential tests, captured-graph replay and verified Helgen gameplay pass.

Written with AI assistance.
@TibixDev

Copy link
Copy Markdown
Contributor Author

@adriadam10 Updated to 5380bb932, rebased on main d146ec1b6.

  • Ported compiled evaluation to the poison/null-root signature. Null roots return zero without memory reads or dependency records; successful evaluation resets the reporting outputs. Out-of-bounds scalar-buffer reads also return zero, matching main.
  • Guarded slots and memory-derived handles now select the generic evaluator, preserving poisoned-read and descriptor-zeroing behavior. Guard metadata is computed before compiling the plan.
  • Added differential null-root, poisoned-read and bounds coverage, plus a translated shader captured through PreparedShaderInvocation. Verification compares poison/null-root outputs too. Confirmed agc_srt_execution is registered through the existing CTest loop with a 30-second timeout.

Fedora 44 Release: SRT tests pass normally, with verification, and with compiled evaluation disabled. Shader-memory, driver, graphics, PM4, frontend-pair and read-patching checks pass with compiled-result verification; contribution checks pass.

Replaced the stale benchmark in the description with fresh paired synthetic measurements. Prepared-invocation capture (0 descriptor sources / 1 read) measured 0.214–0.220 µs generic versus 0.0949–0.0952 µs compiled across ABBA runs with matching checksums. The 8-source/72-read evaluator fixtures and reproduction commands are also in the description.

Ready for another correctness review. A fresh title-derived capture, title-level performance and Windows runtime validation remain outstanding; these synthetic timings do not establish an FPS improvement.

This was referenced Oct 15, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

adriadam10's review of 10-13 is still open on this head: libSceAgcDriver/CMakeLists.txt conflicts with main (keep both the foreach test list and the timeout block), and after that agc_srt_execution fails. Could you rebase and look at that failure? #6095 overlaps the compiled evaluation, so please also say which of the two this should build on.

@github-actions github-actions Bot added perf Performance change perf numbers missing perf pull request without a before and after summary line labels Oct 18, 2026

This branch has not been deployed

No deployments
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 perf numbers missing perf pull request without a before and after summary line perf Performance change shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants