Skip to content

fix(shader): take v_sin_f32 and v_cos_f32 from an exact series where the driver's result is more than 4 ulp off - #3770

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
A-Akhil:fix/shader-sin-cos-series
Oct 11, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
A-Akhil:fix/shader-sin-cos-series

Conversation

@A-Akhil

@A-Akhil A-Akhil commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

What

agc_driver_sin_cos_near_zero fails on Intel UHD Graphics (CML GT2, Mesa 26.2.3) in every run:

AGC graphics: sin cos near zero: lane 24 v_sin_f32 is 0x3f167934, expected 0x3f167919 (tolerance 12 ulp)

Lane 24 is 0.1 of a turn (sin 36°). It is outside the 2^-12 near-zero range that #2380 linearizes, so the result is the driver's GLSLstd450Sin, 27 ulp from the hardware value on this GPU. The same failure was reported in a comment on #2114.

This applies the scheme EmitFPLog2 already uses: keep the driver's value unless it is more than 4 ulp from an exact series, and take the series then.

  • The cycle from EmitTrigCycleF32 is exact, so the reduction to [0, 1/8] of a turn uses only Sterbenz subtractions (0.5 - a, 0.25 - a), which are exact.
  • SinTurnSeries and CosTurnSeries evaluate sin(2πx) and cos(2πx) as Taylor series in turns, with f32 coefficients of (2π)^n/n!, by FMA Horner steps marked NoContraction. Over [0, 1/8] they stay within 2 ulp (sin) and 1 ulp (cos) of the true value; at lane 24 the series gives 0x3f167918, 1 ulp from the hardware.
  • Sine: a or 0.5 - a, then the cosine series of 0.25 - r past 1/8. Cosine: cos(a), sin(0.25 - a) or -cos(0.5 - a) by eighths of a turn.
  • The near-zero linear overlay from fix(shader): make v_sin and v_cos linear within 2^-12 of a turn from a zero #2380 is unchanged. Where the driver is within 4 ulp of the series (NVIDIA here), the driver's bits are kept, so results there do not change.

Tested

Arch Linux (kernel 6.18.54-lts), GCC 16.2.1, Release, BUILD_TESTING=ON, main a11c8f7. One laptop with two GPUs, picked with ANYPS5_GPU: GeForce GTX 1650 Ti (NVIDIA 615.71.09) and Intel UHD (CML GT2, Mesa 26.2.3).

main with this change
agc_driver_sin_cos_near_zero, Intel 3/3 fail (lane 24, 27 ulp) 3/3 pass
agc_driver_sin_cos_near_zero, NVIDIA pass 3/3 pass

Full ctest, 832 tests per GPU, with and without the change:

Windows and AMD: not tested.

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 (AI identified the problem, but I independently developed and implemented the fix, then tested and verified it on my machine.)

@github-actions github-actions Bot added the shader core/shader label Oct 11, 2026
Co-Authored-By: Claude <noreply@anthropic.com>
@adriadam10

Copy link
Copy Markdown
Collaborator

Measured on the HW oracle (gfx1035, IEEE=0, f32 denormals flushed) over 243,092 inputs in [2^-13, 0.5] turns plus ±3000 ulp around 1/8, 1/4, 3/8, 1/2 and 0.1 and negatives: the series is within 2 ulp of the true sine/cosine (also with Fma split into mul + add, which the spec allows), and the hardware itself is up to 9 ulp from it. With the series forced on every input and the tolerance of agc_driver_sin_cos_near_zero cut to 1 ulp, the test passes on RADV and on CI's lavapipe (Mesa 25.2.8); both sin/cos tests pass unchanged on both.

Added on your branch:

  • docs/dev/TechnicalDebt.md: the v_sin/v_cos line said the driver's value is always used; it now describes the series fallback and its bound (within 13 ulp of the hardware on any driver).

Merging once CI is green.

@github-actions

github-actions Bot commented Oct 11, 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
#4030 docs/dev/TechnicalDebt.md L517-518

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants