Repository navigation
fix(libc): pass the exponent to fma as long double in _LCosh and _LSinh - #13952
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — looks good
At head a7e91c26fdca4bbac6cb07817983fc1e1ce7594d, both long-double hyperbolic paths now pass long-double operands to std::fma, addressing the reported narrowing behavior.
What I ran
Reviewed head a7e91c26fdca4bbac6cb07817983fc1e1ce7594d against base 73f4f7e06e4f210a8c6570e335fd654dc0a86134; the merge preview was clean. Local environment: Linux x86_64, Python 3.11.2, GCC 12.2.0. I ran the no-build precheck and inspected both changed call sites and the existing long-double assertions.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 13952
gh pr checks 13952 --repo boykopovar/AnyPS5Results
| Check | Result |
|---|---|
| Diff and conventions | Passed |
| Current-base merge preview | Clean |
| Hosted Windows job | Passed |
| Hosted Ubuntu Debug and Release register-context jobs | Passed |
| Hosted Linux full build | Skipped by workflow |
| Hosted macOS jobs | Pending at review time |
| Author-reported target test | On macOS 27.2 / Apple clang 21 / M4 Pro x86-64 under Rosetta, guest_math reportedly failed at the existing _LCosh assertion 3/3 times without this change and passed 3/3 with it, with #10345 applied. I did not run this test independently. |
| Additional local precheck heuristics | Unavailable in the isolated PR snapshot; no conclusion drawn from those checks |
Unedited output
All conventions checks passed.
windows pass 3m22s
register-context (ubuntu-latest, Release) pass 25s
register-context (ubuntu-latest, Debug) pass 33s
linux skipping 0
Sources
The changed long-double reduction paths are in Math.cpp at this head. The existing long-double comparison helper and _LCosh/_LSinh edge assertions are in GuestMath.cpp at this head.
Not verified
I did not run a local build or test. The target-specific macOS result is author-reported; the macOS CI jobs were still pending, and the hosted Linux full build was skipped.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
The two red AI-assisted: yes |
What
_LCoshand_LSinhreduce the argument withstd::fma(-exponent, Ln2High, magnitude), whereexponentis anintand the other two arelong double. That picks the mixed-typestd::fmatemplate. In libc++ (macOS, and any clang build that uses libc++) that template computes the promoted type,long double, but then calls__builtin_fma, the double version (__math/fma.h, line 48 of the macOS SDK header; the other mixed-type math templates call__math::and are fine). So the reduced argument is rounded to 53 bits beforeexpl, and the last ~10 bits of the 64-bit result are wrong. libstdc++ promotes correctly, so Linux and the MinGW build are not affected.The fix converts the negated exponent to
long doubleonce and passes that to bothfmacalls, so every compiler picks thelong doubleoverload. In the built object the two functions now callfmalfour times andfmano longer.Tested
macOS 27.2 (Apple M4 Pro, x86-64 under Rosetta 2), Apple clang 21, Release,
guest_math_tests, on main plus the support commits needed to build on macOS._LCosh(11357, 0x1p-16000)returns0xc.e2773666cc8cb2dp+380; the test expects0xc.e2773666cc8cc68p+380. The difference, 0x13b in the last place, matches the reduced argument rounded to 53 bits (0x136 predicted).guest_mathfirst stops atfmodf(Apple's libm does not raise invalid forfmod(1, 0)), which fix(libc): set EDOM in fmod and fmodf where titles' libc.prx does #10345 fixes. With fix(libc): set EDOM in fmod and fmodf where titles' libc.prx does #10345 applied: without this changeGuestMath.cppstops at the_LCosh(11357.0L, 0x1p-16000L)check 3 out of 3 runs; with this change the wholeguest_mathpasses 3 out of 3.CheckLongDoubleHyperbolicchecks already catch it on libc++.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)