Skip to content

feat(shader): follow IEEE_MODE in f16 and f64 min and max - #2036

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
CouchRiv:fix-shader-minmax-f16-f64-ieee
Oct 10, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
CouchRiv:fix-shader-minmax-f16-f64-ieee

Conversation

@CouchRiv

@CouchRiv CouchRiv commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

The f16 and f64 min/max family ignored the shader's IEEE_MODE, so a NaN operand gave a different result than on the console:

v_min_f16(0x3c00, 0x7c01), IEEE=1:              hardware 0x7e01, main 0x3c00
v_min_f64(+0, 0x7ff0000000000001), IEEE=0:      hardware +0,     main 0x7ff8000000000001

#1826 and #1867 made the f32 forms and the unary instructions follow IEEE_MODE; TechnicalDebt still listed "the f16 and f64 min/max/med3" as following a single mode. This PR covers those forms: v_min_f16, v_max_f16, v_min3_f16, v_max3_f16, v_med3_f16, v_min_f64 and v_max_f64. RDNA2 has no f64 min3, max3 or med3.

The f16 forms now choose the NaN result on the f16 bits, like the packed f16 forms already do. The f64 forms keep their current lowering at IEEE=1 (it already matched) and get the IEEE=0 rule. The f32 path and ieeeMinMaxF32 are unchanged. Shaders with no known float mode follow IEEE=0, as elsewhere.

Measured with the hardware oracle on RDNA2. I used gfx1036 (Radeon 610M), not the gfx1035 of #1826; both are RDNA2. Every float control was set explicitly, at IEEE=0 and IEEE=1, with f32 denormals kept and flushed:

IEEE=0 IEEE=1
f16 a NaN operand returns the other operand unchanged, src0 when both are NaN as IEEE=0, but a signaling NaN operand returns that NaN quieted, src0 checked first; min3/max3 are two nested pairs, med3 is min3 when any operand is NaN (same rule as f32)
f64 src1 NaN: src0 unchanged, even a signaling NaN; otherwise src0 NaN: src1 a signaling NaN operand returns that NaN quieted (already the case on main)

Measured rows (gfx1036):

instruction operands IEEE hardware main
v_min_f16 0x0000, 0xfd23 1 0xff23 0x0000
v_med3_f16 0x0000, 0x0000, 0xfd23 1 0xff23 0x0000
v_min_f16 0x7c01, 0xfe55 0 0x7c01 0x7e01 RADV, 0x7fff NVIDIA
v_min_f64 0x7ff0000000000001, +0 0 +0 0x7ff8000000000001
v_max_f64 0x7ff0000000000001, 0x7ff8000000000000 0 0x7ff0000000000001 0x7ff8000000000001

The TechnicalDebt float mode line now lists these forms with their rule. It also records a separate gap: when f16/f64 denormals are flushed, the hardware flushes denormal inputs of these instructions (v_min_f16(0x0001, 0x8001) gives 0x8000), and this lowering does not, before or after this PR.

Which title needs it

No title yet. This completes the IEEE_MODE series (#1826, #1867) for the f16 and f64 min/max forms that TechnicalDebt still listed as following a single mode. Most titles use FLOAT_MODE 0xc0 with IEEE=0, where v_min_f64/v_max_f64 now return the non-NaN operand as the console does instead of a NaN, and f16 min/max keep src0's NaN bits when both operands are NaN. No title is known to hit either case.

Tested

CachyOS Linux, RTX 4070 Ti (NVIDIA) and Radeon 610M (RADV), Release build.

  • New agc_driver_f16_min_max_nan, laid out like agc_driver_f32_min_max_nan: 128 rows of ±0, ±1, ±inf, quiet and signaling NaNs with payloads and denormals, with garbage in the high halves, through the five f16 ops and -/|x| modifiers. Each row runs with no float mode and with IEEE=1, against tables captured on the oracle. It passes on both GPUs. On main it fails at row 94 v_min_f16 v10, v4, v5 is 0x00007e01, expected 0x00007c01.
  • agc_driver_f64_arithmetic held the IEEE=1 results (its table matches gfx1036 exactly at IEEE=1) but ran without a float mode. It now runs both ways, and its min/max columns at IEEE=0 come from the oracle.
  • Every row of the f16/f64 min/max cases from our oracle sweep (424 to 2,221 rows per op, with clamp and neg/abs variants) matches the hardware on both drivers in the four mode combinations (IEEE 0/1, DX10 clamp 0/1, f32 denormals kept/flushed).
  • The SDWA forms follow too: v_min/max_f16_sdwa with WORD_0/WORD_1 selects, clamp, mul:2 and neg/abs, 400 rows each in two IEEE=1 modes (DX10 clamp on and off), match the hardware on both drivers with this branch (main: 64 of 400 differ).
  • Full ctest on both GPUs, check_conventions.py, git diff --check.
  • With f16/f64 denormals flushed, these forms still differ from the hardware (measured, recorded in TechnicalDebt, see above).
  • Not tested: lavapipe, gfx1013 hardware, rounding other than to nearest even.

Checklist

🤖 Generated with Claude Code

@github-actions github-actions Bot added shader core/shader docs agc libSceAgc* labels Oct 9, 2026
@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

I reviewed the full diff and the hardware evidence in the PR description. The current Build, Conventions, and Progress workflows for this head are all marked action_required, so those upstream checks have not run. CONTRIBUTING says a maintainer must approve held fork workflows; please have that done before merge. I am holding approval until the required checks are visible.

@popcoder565

Copy link
Copy Markdown
Contributor

Native Windows run, since no Windows results were posted. Setup: Windows 10 (19045), RTX 3060 Laptop (NVIDIA 616.92), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. Head b05a8048, merged onto main d70b899 in one build with 38 other PRs that had no Windows run.

No title was run.

🤖 Addressed by Claude Code

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
The f16 min/max/min3/max3/med3 now pick NaN results on the bits and
quiet a signaling NaN only with IEEE=1. With IEEE=0, v_min_f64 and
v_max_f64 return the other operand for a NaN, not a quieted NaN. This
completes boykopovar#1826 for these forms, as measured on RDNA2 (gfx1036).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main (only conflict: the shader float mode line in TechnicalDebt, merged with the lines that landed since; no code changes). agc_driver_f16_min_max_nan and agc_driver_f64_arithmetic pass on the rebased tree, and fail without the change. The earlier red linux/windows jobs were the agc_shader_fusion_tests missing spirv.hpp break on main, fixed by 37bc6f2. Merging once CI is green on the new head.

@adriadam10
adriadam10 force-pushed the fix-shader-minmax-f16-f64-ieee branch from b05a804 to 6d1fe2b Compare October 10, 2026 07:19
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

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.

Pull request Conflicting files (lines on main) Same exports Same new files
#2025 docs/dev/TechnicalDebt.md L338

@adriadam10
adriadam10 merged commit f198c8f into boykopovar:main Oct 10, 2026
14 checks passed
SleglJan pushed a commit to SleglJan/AnyPS5 that referenced this pull request Oct 10, 2026
…6-f64-ieee

feat(shader): follow IEEE_MODE in f16 and f64 min and max
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants