Skip to content

perf(shader): pick the f32 NaN result with one quiet and one fallback select - #4287

Open
karanvora2599 wants to merge 1 commit into
boykopovar:mainfrom
karanvora2599:perf/shader-f32-nan-result-once
Open

karanvora2599 wants to merge 1 commit into
boykopovar:mainfrom
karanvora2599:perf/shader-f32-nan-result-once

Conversation

@karanvora2599

@karanvora2599 karanvora2599 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

nanResultF32 now picks the AMD NaN result for an f32 instruction with one quieting step and one final select, instead of quieting and selecting once per source. It also tests sources for NaN with FPIsNan32 instead of an integer and + compare.

Background. These instructions go through nanResultF32 so that a NaN result matches AMD hardware rather than the host GPU:

  • v_add_f32, v_sub_f32, v_subrev_f32 and v_mul_f32;
  • v_fma_f32 and v_fma_mix_f32 with f32 sources;
  • v_mul_legacy_f32 and the legacy mad/mac/fma forms;
  • v_div_fmas_f32.

The rule is:

  • the first NaN source, quieted in IEEE mode;
  • else the default NaN 0xffc00000 when the result is NaN;
  • else the result.

For a fused multiply-add whose product is 0 × ∞, the addend doesn't count as a source.

Before, the function started from the fallback and then, for each source from last to first:

  • tested it for NaN with (bits & 0x7fffffff) > 0x7f800000;
  • quieted it with its own or;
  • selected it over what it had so far.

The addend's test was also combined with the inverse of the invalid-product flag.

How.

  • The chain now carries only source bits:
    • The addend starts it, replaced by 0 when the product is 0 × ∞.
    • Each earlier source that FPIsNan32 finds NaN takes its place.
  • The fallback is isNan(result) ? 0xffc00000 : result.
  • The result is isNan(candidate) ? quiet(candidate) : fallback. So the quieting or and the final select happen once.

Why results don't change. The candidate is NaN exactly when some eligible source is NaN, and it is then the first eligible NaN source, in the same order as before:

  • If no eligible source is NaN, the candidate is the last source or 0. Neither is NaN, so the result is the fallback, as before.
  • The 0 × ∞ case can't pick the addend, because 0 isn't NaN.

agc_driver_f32_arithmetic_nan checks 16 forms and pins both cases:

  • the source order (with the order reversed it fails: v_fmaak_f32 is 0xffc12345, expected 0x7f812345);
  • the 0 × ∞ product with a quiet or signaling NaN addend (row {+inf, 0, 0x7fc00003} expects 0xffc00000 from v_fma_f32).

v_mac_f32 and v_mad_f32 don't go through nanResultF32 and are unchanged.

Before / after on an RTX 4060 Laptop (Ada, driver 591.86). SASS instructions per instruction are derived from the APS5_PIPELINE_STATS binary size over a chain of 16 instructions, with FLOAT_MODE 0xc0, DX10 clamp on and IEEE mode off:

Instruction SASS before SASS after
v_mul_f32 18.0 16.5
v_add_f32 12.0 10.5
v_add_f32 DPP row_shr:1 15.5 14.0
v_subrev_f32 13.0 11.5
v_sub_f32 12.0 11.0
v_fma_f32 23.5 22.5
v_fma_mix_f32 (f32 sources) 23.5 22.5
v_div_fmas_f32 13.0 12.0
v_mac_legacy_f32 46.5 45.0
v_mul_legacy_f32 20.5 20.0
v_mad_legacy_f32 49.5 49.0
  • Of the 363 swept opcodes, 11 got smaller and none got bigger.
  • With IEEE mode on, the old code also had one quieting or per source where the new code has one in total. That case wasn't measured.
  • In the census from perf(shader): compute v_dot4 with packed integer dot products #4061 (11 titles, 837 shaders), these instructions appear 63,869 times: v_mul_f32 42,491, v_add_f32 11,247, v_sub_f32 7,746, v_subrev_f32 1,770, v_fma_f32 615. Weighted by the counts above, that's about 91,600 fewer SASS instructions across the corpus, as a static count.
    • The census also has 79 v_fma_mix_f32, listed as VOP3P v_fma_f32 in the histogram. Only those with f32 sources change, so they're left out of the estimate.
  • The SASS counts are from Ada. The translation change is the same on every GPU.

Tested

Windows 11, MinGW-w64 GCC 15.2.0, Release, this branch on main at d146ec1. Devices:

  • RTX 4060 Laptop (Ada, driver 591.86)
  • Intel UHD Graphics (Raptor Lake, driver 101.6790)
  • lavapipe from Mesa 25.2.8 (llvmpipe, LLVM 21.1.7): the version CI installs; Windows build with GALLIUM_OVERRIDE_CPU_CAPS=avx.

No new test. The existing tests already pin what this change has to keep. As a check, I made the chain keep a later NaN source over an earlier one. agc_driver_f32_arithmetic_nan then failed with lane 0 no float mode v_fmaak_f32 is 0xffc12345, expected 0x7f812345.

f32 and NaN tests. These pass on all three devices, except the division modifier tests, which the Intel UHD skips for a missing feature:

  • agc_driver_f32_arithmetic_nan, agc_driver_nan_quieting, agc_driver_f32_min_max_nan, agc_driver_fma_rounding, agc_driver_fma_legacy;
  • agc_driver_division*, agc_driver_f32_output_modifier, agc_driver_f32_result_clamp, agc_driver_f32_denormal_flush, agc_driver_f32_constant_factor_tininess;
  • vector_fused_multiply_add, vector_mad_legacy, agc_denormal_flush_elimination.

Shader and driver tests (agc_driver_*, agc_shader_*, agc_spirv_*, vector_*, agc_masked_select, agc_validate_program, agc_denormal*, agc_float16_*; 338 tests):

Op sweep. All 363 swept opcodes, compiled from main and from this branch, gave the table above.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

… select

nanResultF32 gives the f32 add, sub, subrev, mul and fma forms, the legacy
mul, mad and fma forms, v_div_fmas_f32 and v_fma_mix_f32 with f32 sources
the AMD NaN result: the first NaN source, quieted in IEEE mode, else the
default NaN 0xffc00000 when the result is NaN, else the result. It started
from the fallback and then, for each source, tested it for NaN with an
integer and and compare, quieted it with its own or, and selected it. The
addend's test was also combined with the inverse of the invalid-product
flag.

The chain now carries only source bits. The addend starts it, replaced by 0
when the product is 0 * inf. Each earlier source that FPIsNan32 finds NaN
takes its place. One quiet and one select then choose between that source,
when it is NaN, and the fallback. The candidate is NaN exactly when some
eligible source is, and it is then the first one, so every result is the
same.

agc_driver_f32_arithmetic_nan already checks the source order and the 0 *
inf product with a NaN addend for every form.
@github-actions github-actions Bot added the shader core/shader label Oct 12, 2026
@Raghav314

Copy link
Copy Markdown
Contributor

Tested on my machine (RTX 4090, Linux), on main d146ec1 + this PR.

Looks good to me, and since it's in the hot path of both titles, I'd like to see it merged.

LLM assistance used.

@karanvora2599

Copy link
Copy Markdown
Contributor Author

Thanks @Raghav314, especially for Demon's Souls: it's the first title in these counts with that much f32 math.

One correction to the share. This PR changes only the instructions that go through nanResultF32: v_add/sub/subrev/mul_f32, v_fma_f32 and its other encodings, the legacy forms, v_div_fmas_f32 and v_fma_mix_f32 with f32 sources. v_mac_f32 and v_mad_f32 become FPMad32 without it, so they don't change. Taking them out of your counts leaves about 11% of Hades II's instructions and about 26% of Demon's Souls', still with v_mul_f32 the largest (1,206 and 180,261).

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Could you add a frame-time comparison on a title, main against main plus this PR alone, same machine and scene? The SASS and SPIR-V sizes show less code, but a perf( PR needs the run-time numbers from the Performance report form. Going by Raghav314's counts, Demon's Souls (PPSA01341) is the title where it should show.

@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.

Checked the new chain against the old one: same result (first NaN source, quieted in IEEE mode; 0×∞ drops the addend; fallback 0xffc00000). agc_driver_f32_arithmetic_nan, agc_driver_nan_quieting, agc_driver_fma_*, agc_driver_division* and agc_driver_f32_min_max_nan pass here on CI's lavapipe.

One thing is missing, and @oneandonlydean already asked for it: a perf( PR needs run-time numbers. The SASS counts show less code but not that a frame gets faster. Please add main vs main + this PR on a title (Demon's Souls, per Raghav314's counts), same machine and scene, with the Performance report form. Once that is in, this is ready to merge.

@karanvora2599

Copy link
Copy Markdown
Contributor Author

@Raghav314, the review needs one more thing before this can merge: a run-time comparison on a title. Your counts showed the most f32 math in Demon's Souls, so that's the title to measure. Could you capture it?

  • Build main (d146ec1, this PR's base) and this PR (7aea3fe).
  • Run the scene once and restart the title, so the caches are warm.
  • Capture each build in the same scene with APS5_TRACE_FRAMES=<start>:<count>. Set no other APS5_TRACE_* or APS5_PROFILE_* switch, as the Performance report form describes.
  • Post the two aps5-summary.jsonl lines and attach the two aps5-trace-*.json.gz files.

RINNECODER pushed a commit to RINNECODER/AnyPS5 that referenced this pull request Oct 13, 2026
… select

nanResultF32 gives the f32 add, sub, subrev, mul and fma forms, the legacy
mul, mad and fma forms, v_div_fmas_f32 and v_fma_mix_f32 with f32 sources
the AMD NaN result: the first NaN source, quieted in IEEE mode, else the
default NaN 0xffc00000 when the result is NaN, else the result. It started
from the fallback and then, for each source, tested it for NaN with an
integer and and compare, quieted it with its own or, and selected it. The
addend's test was also combined with the inverse of the invalid-product
flag.

The chain now carries only source bits. The addend starts it, replaced by 0
when the product is 0 * inf. Each earlier source that FPIsNan32 finds NaN
takes its place. One quiet and one select then choose between that source,
when it is NaN, and the fallback. The candidate is NaN exactly when some
eligible source is, and it is then the first one, so every result is the
same.

agc_driver_f32_arithmetic_nan already checks the source order and the 0 *
inf product with a NaN addend for every form.

Ported from upstream PR boykopovar#4287.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
RINNECODER added a commit to RINNECODER/AnyPS5 that referenced this pull request Oct 13, 2026
* perf(shader): pick the f32 NaN result with one quiet and one fallback select

nanResultF32 gives the f32 add, sub, subrev, mul and fma forms, the legacy
mul, mad and fma forms, v_div_fmas_f32 and v_fma_mix_f32 with f32 sources
the AMD NaN result: the first NaN source, quieted in IEEE mode, else the
default NaN 0xffc00000 when the result is NaN, else the result. It started
from the fallback and then, for each source, tested it for NaN with an
integer and and compare, quieted it with its own or, and selected it. The
addend's test was also combined with the inverse of the invalid-product
flag.

The chain now carries only source bits. The addend starts it, replaced by 0
when the product is 0 * inf. Each earlier source that FPIsNan32 finds NaN
takes its place. One quiet and one select then choose between that source,
when it is NaN, and the fallback. The candidate is NaN exactly when some
eligible source is, and it is then the first one, so every result is the
same.

agc_driver_f32_arithmetic_nan already checks the source order and the 0 *
inf product with a NaN addend for every form.

Ported from upstream PR boykopovar#4287.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* perf(shader): drop the output flush of f32 products

In a FLOAT_MODE that flushes f32 denormals, writeOperand flushes every
f32 result, and DenormalFlushEliminator removes the flush only when the
value can't be a denormal. It had no rule for products, so every
v_mul_f32 kept its output flush, although the tiny-product flush before
it already leaves no denormal.

The eliminator now proves a product denormal-free from three facts:

- The tiny-product flush of a multiply (round to nearest even, no
  addend) returns a signed zero or a product at least 2^-126 in
  magnitude, so its select is flagged notDenormal. An fma isn't
  flagged, since its addend can cancel the result into a denormal.
- The NaN select picks its first arm only when that arm is a NaN, so it
  is as denormal-free as its fallback.
- A product by a constant that is ±0 or at least 1 in magnitude, of a
  denormal-free factor, isn't a denormal.

A dependent chain of 2048 v_mul_f32 on an RTX 4060 runs 9% faster.
v_mul_legacy_f32 loses the same flush. Results are unchanged.

The f32 denormal flush test gets rows where a nonzero addend cancels a
normal product into an exact denormal, which the fma output flush must
turn into a signed zero. No row covered that before.

Ported from upstream PR boykopovar#4539.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* perf(shader): treat f32 min, max, med3, floor and ceil results as denormal-free

In the float modes that flush f32 denormals, writeOperand flushes the result
of v_min/max/min3/max3/med3_f32 and v_floor/ceil_f32. DenormalFlushEliminator
drops a flush only when it can show the value isn't a denormal, and it had
no rule for these opcodes, so their result flush stayed even when every
operand had already been flushed.

FPMin32, FPMax32, FPMinTri32, FPMaxTri32 and FPMedTri32 return one of their
operands' bits, or the and or or of two zeros, so the result is denormal-free
when every operand is. FPFloor32 and FPCeil32 return an integer, a zero, an
infinity or a NaN whatever their operand. In IEEE mode the min and max
results and the floor and ceil results go through selects with a quieted
operand, which the eliminator can't follow, so their flush stays there.

agc_denormal_flush_elimination covers the five min and max forms with
flushed and with unknown operands, floor and ceil of an unknown value, and a
max carried around a loop.

Ported from upstream PR boykopovar#4490.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* perf(shader): treat f32 fract, sin and cos results as denormal-free

In the float modes that flush f32 denormals, writeOperand flushes the result
of v_fract_f32, v_sin_f32 and v_cos_f32 after flushing their input.
DenormalFlushEliminator had no rule for FPFract32, FPSin or FPCos, so the
result flush stayed.

None of the three returns a denormal for an input that isn't one.
EmitFPFract32 takes fract(|x|), which is |x| below 1 and otherwise a multiple
of the input's ulp, and 1 - fract, which is at least 2^-24. EmitFPSin and
EmitFPCos reduce the input exactly with x - roundEven(x), so the cycle and
its distance from a zero of the function are 0 or at least 2^-126. Near a
zero, TrigLinearNearZero returns the distance times about 6.28. Elsewhere
the result is within 4 ulps of a series value of at least about 1.5e-3. In
IEEE mode floatUnary quiets a NaN input with an or, which the eliminator
can't follow, so the flush stays there.

agc_denormal_flush_elimination covers the three opcodes with a flushed and
with an unknown input.

Ported from upstream PR boykopovar#4737.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* perf(shader): drop flushes of f32 results that cannot be denormal

DenormalFlushEliminator removes f32 denormal flushes on values it can prove are not denormal. Recognises FPTrunc32, FPRoundEven32, square roots (FPSqrt), reciprocal square roots (FPRecipSqrt32), and bitwise operations that set exponent bits (BitwiseOr32) or mask to signed zeros (BitwiseAnd32). Adds unit tests in DenormalFlushElimination.cpp.

Ported from upstream PR boykopovar#5086.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* fix(shader): take the fused result when an f16 fma's sum is exact

fmaF16RoundedToOdd adds the f32 product and the addend separately and uses
TwoSum to round to odd when the sum is inexact. When the error is zero it
returned the separate sum. On MoltenVK (Apple M3 Pro) the separate multiply and
add produce +0 where the IEEE result is -0: agc_driver_dpp16_operands got 0x0000
instead of 0x8000 in the high half of lane 1's row_shr v_pk_fmac_f16.

The exact case now returns an f32 fma of the same operands. The product of two
f16 values is exact in f32 (11-bit significands, exponents well inside the f32
range), so when TwoSum's error is zero the separate sum and the fused result are
the same IEEE value, including the sign of zero; the change only matters on
devices whose separate multiply and add do not keep it. The inexact path is
unchanged.

Ported from upstream PR boykopovar#4293.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* perf(shader): take the f32 mul, fma and add result fixups only when needed

v_mul_f32, v_fma_f32, v_add_f32 and v_sub_f32 compute two fixups on
every result: the AMD NaN result (the first NaN operand, quieted, or the
default NaN) and, for products, the flush of results that are tiny after
rounding to 24 bits. Both change the result only when it is a NaN or its
magnitude is at most 2^-126. They were straight-line selects, so a
v_mul_f32 took 16.5 SASS instructions on an RTX 4060 where the multiply
is one.

The translator now wraps the fixed result in a select that takes it only
when the host result is NaN (add and sub) or NaN or at most 2^-126 in
magnitude (mul and fma, round to nearest even). That select is flagged
as rarely true, and the SPIR-V backend moves the instructions used only
by its rare arm into a branch. The rare arm is the same IR as before, so
results are unchanged. Loop headers, continue targets and values shared
between the two halves of a wave64 program keep plain selects.

A dependent chain of 2048 v_mul_f32 runs 2.2 times as fast, v_fma_f32
1.85 times and v_add_f32 1.13 times; on all-NaN inputs v_mul_f32 is 6%
slower. Static SASS grows, since both paths are compiled.

Ported from upstream PR boykopovar#5400.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* fix(shader): read lane 63 of half-wave reductions whose keys exclude helper lanes

On a 32-lane host, a wave64 pixel program's v_readlane_b32 of lane 63 is emitted only when its half-wave scan's keys are proven zero for lanes that did not enter. The proof walk accepted the not-helper test (IEqual32 of HelperInvocation and 0) only inside ballots, not as a lane bit, and it searched conjunctions depth first: a key mask like exec & ~vcc inside a loop spent the 512-step budget on the loop's exec phis before it reached the helper test. Accept the helper test as a lane bit, and look for a false or helper conjunct before walking an operand. Resident Evil Village's wave-wide OR reductions (DPP row shifts, v_permlanex16, v_readlane of lanes 31 and 63) failed this way.

Ported from upstream PR boykopovar#5189.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* test(shader): cover the search order of the half-wave key proof

The proof that a half-wave scan's keys are zero for lanes that did not
enter walks a conjunction depth first under a 512-step budget. Narrowing
exec with 600 compares before the scan puts the helper-lane test at the
bottom of a 600-deep LogicalAnd chain in the keys: walking the first
operand first spends the whole budget before reaching it, and the
v_readlane_b32 of lane 63 is refused. Looking for a false or helper
conjunct before walking an operand finds it, so lane 31 becomes a
subgroup OR on a 32-lane host. Resident Evil Village's wave64 pixel
programs key these scans by a loop's exec ahead of the helper test.

Ported from upstream PR boykopovar#5189.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* perf(shader): find the ds_permute_b32 source lane with ballots

ds_permute_b32 sends each active lane's data to lane
((addr + offset) >> 2) & 31 of its 32-lane group; the highest sending
lane wins and a lane nobody writes to gets 0. It was emitted as one
shuffle of the data, the address and the EXEC bit from each of the 32
source lanes: 96 shuffles and 144.5 SASS instructions per instruction
on an RTX 4060 when the address changes between instructions.

Each receiving lane now finds its writers directly: the ballot of the
active lanes, intersected with the ballot of the lanes whose target has
bit b set, or its complement, for each of the 5 target bits. The
highest set bit is the winning lane, and one shuffle fetches its data.
Results are unchanged. A chain of 256 dependent ds_permute_b32 runs
4.7 times faster.

New test agc_driver_ds_permute_lanes compares wave32 and wave64
programs with random targets, heavy collisions and run-time EXEC masks
against a model of the rule.

Ported from upstream PR boykopovar#5334.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* perf(shader): mark runtime metadata buffers read-only

Ported from upstream PR boykopovar#5297.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* perf(shader): fence a CAS-loop LDS atomic once

SharedAtomicUpdate emitted a workgroup acquire-release fence after AtomicUpdate,
which already ends its compare-and-swap loop with the same fence for LDS, so
ds_rsub, ds_add_f32, ds_cmpst, ds_mskor, ds_wrap and the LDS float min/max
fenced twice. The second fence is now emitted only for GDS, where the loop's
own fence is device-scoped on uniform memory and differs from it.

Ported from upstream PR boykopovar#5399.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

* chore(shader): bump the shader disk cache format for the float peephole ports

The ported float, denormal and wave-lane changes alter the emitted shader
code, so cached entries from before them must not be reused.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

---------

Co-authored-by: Karan Vora <52519664+karanvora2599@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-authored-by: Friesmaniac <181011464+Friesmaniac@users.noreply.github.com>
Co-authored-by: Utku Halis <personal@utkuhalis.com.tr>
Co-authored-by: Yunus Shaikh <75296019+EagleBearerYS@users.noreply.github.com>
Co-authored-by: Sanxchep Sharma <sanxchep@gmail.com>
@adriadam10 adriadam10 self-assigned this Oct 14, 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.

Rechecked the head (7aea3fe): the new chain gives the same result as the old one (first NaN source, quieted in IEEE mode; 0×∞ drops the addend; fallback 0xffc00000), and the f32/NaN/fma/division tests pass here. The code is fine.

One thing is still missing and it is the only blocker: a perf( PR needs run-time numbers, as @oneandonlydean asked. The SASS counts show less code, not a faster frame. Please add main vs main + this PR on a title (Demon's Souls, per Raghav314's counts), same machine and scene, using the Performance report form. We can't run titles on our side, so this has to come from someone who can. Once it is in, this is ready to merge.

@Raghav314

Raghav314 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

I can't test this in Demon's Souls yet, so I ran the comparison on my machine in Hades II (PPSA36082) instead. Why Demon's Souls doesn't work for this right now:

  • d146ec1 can't run Demon's Souls here: it aborts after about 13 s on "DB_RENDER_CONTROL ... resummarize unsupported", which fix(agc): handle Demon's Souls read-only depth resummarize #4049 fixed later. So both builds are current main (84b01f9), one plain and one with this PR merged in (203bb7730, clean merge).
  • On current main, Demon's Souls was still on a black screen after 4 minutes of cold shader compiles (about 1,160 shaders compiled, no presents yet), so I had no scene to capture.

Hades II is in the counts too (about 11% of its instructions after your correction). Scene: the hub, standing still at the same spot, 1920x1080. Each build ran once to the hub and was restarted, then captured with APS5_TRACE_SECONDS=200:15 and no other trace or profile switch. Same machine, same session, one capture each:

{"commit":"84b01f9cd042","dirty":false,"env":{"APS5_HOST_IMPORT_MIB":"6784","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"200:15"},"gpu":"NVIDIA GeForce RTX 4090","driver":"NVIDIA 615.78.08","os":"Linux 7.2.9-2-cachyos","resolution":"1920x1080","window":{"spec":"seconds 200:15","first_present":8318,"ms":15011.593},"presents":586,"frame_ms":{"p50":25.428,"p90":27.641,"p99":29.832,"max":35.781},"gpu_busy_ms_per_present":5.689,"worker_busy_pct":{"queue worker 0x0":98.846},"top_phases_ms_per_frame":{"Driver.Packet":25.314,"Driver.Packet.draw":22.919,"Driver.Draw":22.774,"Driver.Draw.shader_compile_and_link":10.298,"Driver.Draw.draw_and_resource_release":9.393,"Graphics.Draw":9.228,"Shader.DrawStage":8.260,"Graphics.Draw.draw_and_resource_release":7.506},"trace_dropped":true,"trace":"aps5-trace-84b01f9cd042-378348.json.gz"}
  • main + this PR (203bb7730):
{"commit":"203bb7730f97","dirty":false,"env":{"APS5_HOST_IMPORT_MIB":"6784","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"200:15"},"gpu":"NVIDIA GeForce RTX 4090","driver":"NVIDIA 615.78.08","os":"Linux 7.2.9-2-cachyos","resolution":"1920x1080","window":{"spec":"seconds 200:15","first_present":8257,"ms":15007.671},"presents":580,"frame_ms":{"p50":25.619,"p90":27.970,"p99":29.458,"max":30.311},"gpu_busy_ms_per_present":5.590,"worker_busy_pct":{"queue worker 0x0":98.876},"top_phases_ms_per_frame":{"Driver.Packet":25.577,"Driver.Packet.draw":23.159,"Driver.Draw":23.008,"Driver.Draw.shader_compile_and_link":10.393,"Driver.Draw.draw_and_resource_release":9.515,"Graphics.Draw":9.341,"Shader.DrawStage":8.356,"Graphics.Draw.draw_and_resource_release":7.592},"trace_dropped":true,"trace":"aps5-trace-203bb7730f97-379144.json.gz"}

GPU busy per present goes from 5.689 to 5.590 ms (-1.7%). Frame time doesn't move (p50 25.4 vs 25.6 ms) because the hub on main is bound by the queue worker (99% busy), not the GPU, so a shader-side change can't show up there. For scale: two earlier captures of mine in the same session with builds that don't touch shader code gave GPU busy 8.630 and 8.631 ms, so 0.1 ms is more than the run-to-run spread I've seen, but this is one capture each. A GPU-bound scene would be the better test, which is what Demon's Souls would have been.

I will attach the two traces below.

aps5-trace-84b01f9cd042-378348.json.gz
aps5-trace-203bb7730f97-379144.json.gz

Traces: aps5-trace-84b01f9cd042-378348.json.gz = main 84b01f9, aps5-trace-203bb7730f97-379144.json.gz = main + this PR (203bb7730).

LLM assistance used.

@fma-rc

fma-rc commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

I ran Demon's Souls (PPSA01341) on Linux with an RTX 5080 (NVIDIA 615.78.08), on main a453108 and on the same commit with this PR (7aea3fe) merged.

With the PR the title stops on the language select screen. It did so in both runs that lasted long enough, at 379 s and at 589 s after start:

compute shader 0x248946900: bindless image table: 17 distinct keys exceed the 16 slots

main without the PR reaches the same screen and does not print that line in any of six runs, two of them 600 s long. On the language select main has a frame time p50 of 29.9 ms (trace window 280:3 from the first present, trace_digest.py, warm shader cache). I don't have a number for the PR yet, because both of its long runs threw before or inside that window.

I haven't looked into why the f32 NaN change would change the image keys of that compute shader.

@milescoviello

Copy link
Copy Markdown
Contributor

A compile-time data point for this change. It's not the frame-time comparison the review asked for.

Linux (Ubuntu 24.04, GCC 13.3, Release), RTX 4070 Ti, NVIDIA 595.99.02. Base is main 66702f1; the PR is 7aea3fe merged onto it cleanly.

The shader is the largest fragment shader of Afterglow (PPSA47770, homebrew built on the PS5 OpenGL stack). It has 19,373 guest instructions, about 8.2k of them v_mul_f32, 6.2k v_add_f32 and 1.4k v_sub_f32, at the default float mode (f32 denormals flushed, IEEE off). I recompiled its dumped request with agc_shader_replay and timed NVIDIA building its fragment-shader library part in the title. The timer is local, around that vkCreateGraphicsPipelines call. Cache was cold, two runs per build:

Build SPIR-V words Part build
main 2,167,462 24.9 s, 25.9 s
main + this PR 2,143,013 22.8 s, 22.9 s

Words drop 1.1%, but the compile drops about 10%.

To show where this shader's compile time goes, I also ran local switches that skip one piece of the f32 emulation each. They change results, so they're for measurement only, not a proposal. One run each, base 24.8 s in that series:

Skipped SPIR-V words Part build
nothing 2,167,462 24.8 s
NaN result selection (nanResultF32) 1,585,252 13.8 s
f32 denormal flushes (flushF32Denormal) 1,687,775 16.5 s
tiny-product check (flushTinyProduct) 1,784,863 20.0 s
all three 715,161 3.9 s

In this shader the NaN selection is the largest single part of the compile time.

The title exits on a cold cache either way: its own watchdog stops waiting on that submission after about 2 s.

AI-assisted: yes (Claude Code)

@karanvora2599

Copy link
Copy Markdown
Contributor Author

Thanks @milescoviello. A 10% shorter build of Afterglow's (PPSA47770) largest fragment shader is a title measurement I couldn't produce here. The breakdown showing NaN selection as the largest single part of that compile is useful too.

@fma-rc, thanks for the Demon's Souls run. Your base, a453108, predates @Celegans12's d9e47d1, which replaced the 16-slot bindless image table with runtime-indexed image and sampler tables. Since then, main has no "distinct keys exceed the 16 slots" stop. I still don't see how this change, which only rewrites how nanResultF32 picks the NaN result, could change that shader's image keys. But a stop main doesn't have needs explaining before this merges.

Could you rerun Demon's Souls on current main with this PR? If it still stops or misrenders there, I'd need that compute shader's dumped request (APS5_DUMP_SHADERS=1, the request for 0x248946900) and the APS5_TRACE_IMAGE_TABLE=1 lines before the stop. I can't run Demon's Souls here, so with those I would replay the shader with agc_shader_replay on main and with this PR and compare.

@fma-rc

fma-rc commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

I reran Demon's Souls (PPSA01341) on main b39d90b and on the same commit with this PR merged, RTX 5080 on Linux, 700 s per run. Neither of the two PR runs stopped, and the 17 distinct keys exceed the 16 slots line is gone; main ran three runs without a stop as well. So the stop I reported came from the old base, as you thought. With APS5_TRACE_IMAGE_TABLE=1, shader 0x248946900 resolves its 256-record table thousands of times per run on both builds. I didn't capture frames in these runs, so this says nothing about rendering.

@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
@jotitadev

Copy link
Copy Markdown
Contributor

Apple GPU / MoltenVK data point on the switch to FPIsNan32. The perf question is still open; this is about correctness on Metal.

Machine: Apple M2 Max, macOS 27.2, x86-64 build under Rosetta. Head 7aea3fe. Method: macos-port @ b7eb6b6 + ImportWatched + PR diff, not main (main doesn't build on macOS yet). The PR applies cleanly.

The port tests NaN with the bits in EmitFPIsNan32 ((bits & 0x7fffffff) > 0x7f800000, from mugurc's macOS commit 47bf367), where main emits OpFUnordNotEqual(x, x). So I also ran with main's EmitFPIsNan32 put back into the port, which is what this PR gets on main. Tests: agc_driver_f32_arithmetic_nan, agc_driver_nan_quieting, agc_driver_f32_min_max_nan, agc_driver_fma_legacy, agc_driver_fma_rounding, agc_driver_division and the other agc_driver_f32_*. "pass" below means: same as the unchanged port. Four f32 denormal/tininess tests (f32_constant_factor_tininess, f32_denormal_flush, f32_dot2_rounding, f32_output_modifier) fail there in every row, because MoltenVK doesn't preserve f32 denormals.

Tree MoltenVK 1.4.2 default MVK_CONFIG_FAST_MATH_ENABLED=1
port + main's EmitFPIsNan32 pass f32_min_max_nan fails
same + this PR pass f32_arithmetic_nan, division, fma_legacy and f32_min_max_nan fail

With fast math forced, the failures with the PR are all a canonical NaN where an AMD NaN was expected:

  • v_add_f32 is 0x7fc00000, expected 0x7f812345

  • v_div_fmas_f32 is 0x7fc00000, expected 0xffc00001

  • v_mad_legacy_f32 |src0| -src2 is 0x7fc00000, expected 0xffc00000

  • Default configuration: no change. MoltenVK compiles the test shaders with MathMode Safe (its log), since the module declares SignedZeroInfNanPreserve. MoltenVK main + SPIRV-Cross Ratchet & Clank: Rift Apart: SIGSEGV since #861, an address-less 64 GiB reservation now takes the addresses the title hints next #2704 also passes.

  • The difference: the current integer test in nanResultF32 gives the same results with Metal fast math on. After this PR, the NaN source selection depends on Metal keeping x != x true for a NaN, which fast math doesn't guarantee. The port's commit also reports x != x as false through MoltenVK for a NaN converted from f16, even with fast math off.

  • With the port's bit test (the PR on the port as is), all of these pass in both modes, and so do agc_driver_f32_arithmetic_nan's order and 0 × ∞ rows.

So on Apple this is fine as long as safe math stays on. A bit-based FPIsNan32 on main would remove the dependency, but that's a separate change.

GPUs tested: Apple M2 Max (MoltenVK 1.4.2; MoltenVK main + SPIRV-Cross #2704 for the default case)

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

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

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.

7 participants