Skip to content

fix(shader): flush v_mac_legacy_f32 denormals as measured on gfx1010 - #1290

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
SirDarky:fix/shader-mac-legacy-denormals
Oct 10, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
SirDarky:fix/shader-mac-legacy-denormals

Conversation

@SirDarky

@SirDarky SirDarky commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What

v_mac_legacy_f32 (VOP2 0x06, and its VOP3 form 0x106) measured the same as VOP3 0x140 on gfx1010 (#1281): the product is rounded before the add, f32 denormal sources, product and result are flushed to a zero of the same sign in every f32 denormal mode and both IEEE modes, and a zero factor gives a +0 product. NaN results follow the same rule on the bits. Before this change, main rounded the product but left denormals and NaN bits to the driver, and TechnicalDebt listed it as not covered.

It now shares vFmaLegacyF32 with v_mad_legacy_f32, with the accumulator as the addend. f32DenormalFlushFor returns 3 for VMacLegacyF32 too. vMulLegacyF32 loses its accumulate path, since only v_mul_legacy_f32 uses it now. That instruction is unchanged and still follows the FLOAT_MODE model.

The new MacLegacy.cpp execution test runs four encodings (VOP2, VOP3 -src0, VOP3 |src0| -|src1|, VOP2 with inline 0) on the 32 rows of FmaLegacy.cpp. It pins the measured results and compares NaN bits exactly. Float16Misc.cpp skips its legacy columns for denormal rows, so it did not cover this. TechnicalDebt: the legacy-mad entry, the NaN line and the f32-denormal line now include v_mac_legacy_f32.

Source of the semantics: measured on hardware, an AMD Radeon RX 5700 XT (gfx1010), with tools/hw-oracle/template.s unchanged. It was assembled by the AMD Windows driver's amd_comgr_2.dll and launched through its amdhip64_6.dll instead of oracle.c and ROCr, as in #1281. 32 rows × 4 encodings, wave32 and wave64, IEEE 0 and 1, f32 denormal modes 0 to 3: all 16 combinations agree, except the signaling NaN rows, which are quieted only with IEEE=1. The raw output is the vop2_0x06 part of the attachment in #1281.

Tested

Windows 11, MinGW-w64 GCC 15.2.0 (WinLibs r7), AMD Radeon RX 5700 XT, CI configure flags, full ctest --output-on-failure --timeout 120 on main c963432 plus #1281 and this change: 570 of 574 pass. agc_driver_pixel_interlock, agc_driver_sdwa_float_selectors and agc_driver_sdwa_ldexp_f16 fail on unmodified main on this GPU (see #1281). video_out_flip_controls exited with 0xc0000374 once; rerun six times, it was skipped five times and crashed once, so it is intermittent and unrelated.

  • agc_driver_mac_legacy, agc_driver_fma_legacy, agc_driver_float16_misc, agc_driver_dpp_no_form and vector_mad_legacy pass. With the translation from fix(shader): round the v_mad_legacy_f32 product before the add #1281 alone, agc_driver_mac_legacy fails (lane 26 (0x00000001, 0x3f800000, 0x80000000) v_mac_legacy_f32_e64 -src0 is 0x80000000, expected 0x00000000).
  • Not run on lavapipe or NVIDIA here.

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: fix(shader): round the v_mad_legacy_f32 product before the add #1281
  • AI-assisted: yes

🤖 Generated with Claude Code

@github-actions github-actions Bot added shader core/shader docs agc libSceAgc* labels Oct 8, 2026
@SirDarky
SirDarky force-pushed the fix/shader-mac-legacy-denormals branch from 0f99e17 to bec780a Compare October 8, 2026 03:38
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 8, 2026
@SirDarky
SirDarky force-pushed the fix/shader-mac-legacy-denormals branch from bec780a to 3a053b3 Compare October 10, 2026 04:16
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

#1281 is merged, so this now conflicts in FloatInstructions.cpp, VectorInstructions.cpp and TechnicalDebt.md. Please rebase onto main so only the v_mac_legacy_f32 commit is left.

VOP2 0x06 measured the same as VOP3 0x140 on gfx1010 (RX 5700 XT) in
wave32 and wave64, IEEE 0 and 1 and f32 denormal modes 0 to 3: the
product is rounded before the add, f32 denormal sources, product and
result are flushed in every mode, and a zero factor gives a +0 product.
It now shares the v_mad_legacy_f32 translation with the accumulator as
the addend, and f32DenormalFlushFor returns 3 for it as well. A new
execution test pins the measured results of its VOP2 and VOP3 forms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto main as requested by @oneandonlydean: dropped the already merged #1281 commit, resolved the conflicts in FloatInstructions.cpp and TechnicalDebt.md, only the v_mac_legacy_f32 commit is left.

Checked: build and the Agc/AgcDriver ctest scope on the rebased tree (agc_driver_mac_legacy, agc_driver_fma_legacy, vector_mad_legacy pass), and agc_driver_mac_legacy fails without the translation change. The hardware numbers are the author's gfx1010 measurements (not reproducible on our RDNA2 oracle); merging on green CI.

@adriadam10
adriadam10 force-pushed the fix/shader-mac-legacy-denormals branch from 3a053b3 to 683ea37 Compare October 10, 2026 09:05
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@adriadam10
adriadam10 merged commit cd73e14 into boykopovar:main Oct 10, 2026
14 checks passed
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.

3 participants