Repository navigation
Port upstream float and denormal peepholes (U1) - #477
Conversation
… 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
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
…ormal-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
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
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
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
…eeded 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
…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
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
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
Ported from upstream PR boykopovar#5297. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
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
…le 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
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 134d43d2ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| special = &ir.Emit(IrOpcode::FPIsNan32, IrType::U1, {plain}); | ||
| } | ||
| return &ir.BitCastF32(*bits); | ||
| return &ir.Emit(IrOpcode::SelectF32, IrType::F32, {special, fixed, plain}, SelectFlags{true}); |
There was a problem hiding this comment.
Set
rarelyTrue instead of notDenormal
SelectFlags{true} initializes the first field, notDenormal, while leaving rarelyTrue false. When output-denormal flushing is enabled and an add/subtract or canceling FMA produces a subnormal, writeOperand adds a flush, but DenormalFlushEliminator now trusts this flag and removes that required flush, returning the subnormal instead of signed zero; the intended rare-arm branching is also never enabled. Initialize this select with SelectFlags{false, true}.
Useful? React with 👍 / 👎.
Ports the float and denormal peepholes of unit U1 from upstream open PRs. Each PR is its own commit with "Ported from upstream PR #N" in the body. Refs upstream-pr-batch-1.
Ported (11 PRs, 12 commits, in rebase order)
SelectFlagsnow hasnotDenormalandrarelyTrue)OpGroupNonUniformBallot/ FindUMsb, both already handled by the Metal bridge)Metal check: every denormal-free proof only removes a flush where the value is already non-denormal by construction (min/max/med3 select an operand, floor/ceil/trunc/round-even give integers, sqrt/rsqrt cannot reach the denormal range), so it holds with
float16DenormPreservefalse and Metal fast-math flushing. No peephole relies on f16 denormals.Skipped
Interpolatepath before the exact wide-sum code. The PR only reshapes that exact path, so there is nothing to gain here.Cache version
ShaderDiskCache::FormatVersion32 -> 33 in one last commit, since emitted shader code changes. Other units that also bump it should rebase onto the highest value rather than add another bump. The request-serializer version is untouched.Evidence
agc_denormal_flush_elimination_tests; x86_64-DANYPS5_ROSETTA_LIBS=ONforrosetta_libs rosetta_engine rosetta_launcher_tests metal_utility_tests rosetta_direct_graphics_tests, no compile errors.shader_recompiler_agc_drivercompiles all touched backend files.agc_denormal_flush_elimination1/1 (includes the new perf(shader): drop the output flush of f32 products boykopovar/AnyPS5#4539, perf(shader): treat f32 min, max, med3, floor and ceil results as denormal-free boykopovar/AnyPS5#4490, perf(shader): treat f32 fract, sin and cos results as denormal-free boykopovar/AnyPS5#4737, perf(shader): drop flushes of f32 results that cannot be denormal boykopovar/AnyPS5#5086 cases);metal_utility_tests,rosetta_direct_graphics,rosetta_launcher4/4.ShaderMemory,DsPermuteLanes,F32DenormalFlush). Theagc_shader_memory_teststarget does not build on the macOS host config on main either (pthread_getattr_npin GuestMemory.cpp), and the execution tests need a Vulkan device.🤖 Generated with Claude Code
https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj