Repository navigation
Conversation
|
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.
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.
|
|
Linux run at this PR's exact head Arch Linux, kernel 7.2.9, Ryzen 9 7950X, g++ (GCC) 16.2.1 20260810, glibc 2.44, x86-64. Release,
This is the focused test only; no title, console or floating-point exception flags were checked. AI-assisted: yes (Claude Code). |
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main 11d6efa and FreeBSD 11.4 msun e_fmod.c/e_fmodf.c:
- It still merges cleanly. On main,
fmodhas moved toMath.cpp:648andfmodfis at:292, and both still just save and restore errno. - The checks in both functions match the msun condition for the
(x*y)/(x*y)return (zero divisor, non-finite dividend, NaN divisor), and finite pairs still go tostd::fmodwith errno preserved. - This matches main's
fmodl(MathExact.cpp, around lines 91-98), which already setsEDOMand returns(x*y)/(x*y)for the same classes, so all three widths now agree. - The debt entry replaces the old
fmodfone under Functional, next to the other measured msun entries.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
Additive conflicts with main's newer libc changes: the debt list keeps both sides' entries (per entry) and GuestMath.cpp keeps both sides' test functions and calls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged current
AI-assisted: yes (Claude Code) |
|
@JJenkx I checked 3132cbc. The fmod source and regression cases are unchanged, and both the fmod and expm1 test functions and calls are retained. One debt conflict needs a correction: please remove the old standalone fmodf bullet at docs/dev/debt/libc.md:67, as the original patch did. It still says the wrapper preserves errno, which contradicts this PR setting EDOM. Keep the new combined fmod/fmodf entry and the other main entries; the combined entry already records that exception flags remain unverified. |
|
@oneandonlydean thanks, you're right. The merge kept the old standalone I re-checked the other branches merged the same way for base entries their PR deleted but the merge kept. This was the only one. AI-assisted: yes (Claude Code) |
|
Linux run at this PR's exact head Arch Linux, kernel 7.2.9, Ryzen 9 7950X, g++ (GCC) 16.2.1 20260810, glibc 2.44, x86-64. Release,
This is the focused test only; no title, console or floating-point exception flags were checked. AI-assisted: yes (Claude Code). |
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head 8bee26992d127a7fa5b2c3df543188f6fa7883b2. The new binary64 and binary32 paths follow the stated FreeBSD msun exceptional-input ordering, set EDOM for zero divisors and invalid operands, preserve incoming errno for finite valid inputs, and leave finite result calculation to std::fmod. The added bit-pattern cases cover signed zero divisors, infinities, quiet/signaling NaNs, payload preference, valid finite results and subnormals. The stale standalone fmodf debt claim is removed; the combined entry records the unverified exception flags.
The author reports guest_math passing three times on this exact head. Hosted eligibility, conventions, Ubuntu/Windows/macOS relinker, ASan, register-context and write-watch checks pass; product Linux/Windows jobs are skipped by policy. I independently ran the conventions check and git diff --check successfully. A local full CMake configure could not complete because the SDL2 submodule has no CMakeLists.txt and the environment lacks SDL XRandR support, so I did not run guest_math locally.
Current #10742 also edits docs/dev/debt/libc.md; combining the heads conflicts only in that document. The code and tests merge cleanly, and the combined debt file should retain both entries. I found no implementation blocker.
MarcZX10
left a comment
There was a problem hiding this comment.
Follow-up to my earlier approval: I refreshed upstream/main and found this branch's merge base is fe204e1a8d3d, with 6,693 current-main commits since it. CONTRIBUTING.md asks contributors to keep branches up to date with main. The local full configure was also blocked, and the available guest_math runs were reported on the older base; the current hosted product jobs are skipped. Please update the branch to current main and rerun guest_math on the resulting tree so the changed fmod implementation and regression cases are validated against the supported base. I will revisit approval after that evidence is available.
8bee269 to
025a0c2
Compare
|
Refreshed as one commit Fresh Linux libc/math build passes; guest_math and guest_dinkumware_internals each pass three runs. The production-only main control aborts guest_math; restoration returns both groups to passing and reproduces the positive libc library hash. Conventions/whitespace pass. Description now distinguishes these exact-head checks from historical Windows/bundled-library measurements; no fresh Windows claim. Logs, JUnit, source/rebase proof and control receipts, SHA-256 |
|
A macOS data point. On Apple M4 Pro, macOS 27.0.1, x86-64 under Rosetta 2, This PR's AI-assisted: yes |
025a0c2 to
7385e2d
Compare
|
Correction to my refresh note above: that merge of AI-assisted: yes (Claude Code) |
|
Linux run at this PR's exact head Arch Linux, kernel 7.2.9, Ryzen 9 7950X, g++ (GCC) 16.2.1 20260810, glibc 2.44, x86-64. Release, Passed in each of 3 runs, exit code 0 each time, with no failed, skipped or not-run tests:
These are the focused tests only; no title or console was used. AI-assisted: yes (Claude Code). |
|
Re-checked at the current head A scratch loader runs the decrypted AI-assisted: yes (Claude Code). |
|
New head The titles raise One existing assertion changed: Checks (Windows 11 Enterprise 10.0.22621, WinLibs GCC 15.2.0 UCRT r7, Release; tested at the branch's own head, since it conflicts with
AI-assisted: yes (Claude Code). |
|
Linux run at this PR's exact head Arch Linux, kernel 7.2.9, Ryzen 9 7950X, g++ (GCC) 16.2.1 20260810, glibc 2.44, x86-64. Release,
The branch currently shows as conflicting with |
53966d1 to
760f4f9
Compare
|
Rebased onto
Exact source pins, preservation proofs, dependency checks, build logs and JUnit, SHA-256 AI-assisted: yes. |
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewing exact head 760f4f9386287b626560c16bcbe7363936741872.
What I ran
- Inspected both commits, the final fmod/fmodf paths and tests, current review discussion, and hosted checks.
- Confirmed the branch's last tested main was
2227f13661. Current main has added adjacent math and test changes in the same files, but the affected hunks are separate and GitHub reports a clean merge. - Hosted conventions, eligibility, labels, register-context, relinker, ASan, write-watch, Windows, and report checks pass. The product Linux job is skipped.
- I did not run a local build or tests.
Results
| Check | Result | Evidence |
|---|---|---|
| Domain/NaN ordering, EDOM and FE_INVALID behavior | Reviewed | Exact current diff |
| Linux focused math tests | Pass, author-reported | guest_math and guest_math_exact, three runs on the tested sync |
| Windows tests and title-libc comparisons | Pass, author-reported | Multiple titles and broad input samples; older measured heads |
| Hosted Windows and supporting platform/policy checks | Pass | GitHub Actions |
| Hosted Linux product build | Skipped | GitHub Actions |
| Local build and tests | Not run | This review |
Findings
Blocking
None.
Non-blocking
- The fast bit checks select the same domain cases as the referenced FreeBSD implementation. NaN divisor precedence, NaN dividend handling, zero divisors, and infinite dividends use the measured quiet-NaN/EDOM path; finite results continue through host
std::fmodwith the prior errno restored. - The head documents that finite-result exception flags still come from the host and were not compared with the console. The PR's eleven-title comparisons cover result bits and errno, not console floating-point flags.
- Main's later adjacent math/test edits remain in separate hunks. Keep them when resolving the resulting textual overlap.
Sources
Not verified
- I did not locally build or run CTest.
- Windows title comparisons and Linux focused runs are author-reported; hosted Linux product tests did not run on this PR.
To get this to a good state
No blocking issue remains on the reviewed head. Preserve current main's adjacent math/test changes when merging.
760f4f9 to
5b596a5
Compare
|
Refreshed against main Linux qualification at Evidence: immutable Linux qualification bundle. Its SHA-256 is AI-assisted: yes. |
|
Static review plus an independent Linux run (CachyOS, kernel 7.2.10, GCC 16.2.1, Ninja, Release, What I checked (static)
Focused test run
Not an AGC/shader/VideoOut change — no paired Vulkan-validation run required. The old debt entry is correctly replaced (the "exception flags and NaN payloads remain unverified" note deleted and rewritten to record the 4.0M-sample console-bit comparison that the note now cites — measured, not assumed). |
fmod and fmodf preserved errno, following FreeBSD msun, but the libc.prx of eleven titles sets EDOM exactly where msun returns (x*y)/(x*y): a zero divisor, an infinite or NaN dividend, or a NaN divisor. There a NaN divisor is returned quieted, then a NaN dividend, then the default NaN. Both functions now do the same; finite results still come from the host. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Titles' libc.prx raises FE_INVALID wherever fmod and fmodf store EDOM, a quiet NaN argument included, where returning x + x or y + y raised nothing. The domain paths now raise through _Feraise, which stores EDOM and raises the flag in both units. Results and errno are unchanged. CheckFmodEdgeFlags covers both functions. CheckFmodfErrno expected no FE_INVALID for its three quiet NaN rows, as C specifies; PPSA07923's and PPSA13934's libc.prx raise it there, so those rows now expect it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5b596a5 to
6215d19
Compare
|
Additional main-conflict refresh — 2026-10-30 Refreshed to The conflict was in debt documentation. Main's newer WebAssembly and license notes are preserved alongside this PR's measured fmod qualification. The production Conventions, whitespace and source preservation checks pass. Latest main Immutable source, build and test receipts, SHA-256 Packaging note: the archive's nested |
What
fmodandfmodfsave and restoreerrnoaround the host call. #3600 did that forfmodand #9643 forfmodf, both following FreeBSD msun, which never storeserrno. #9643's debt entry records that the console'serrnowas not measured. Measured now, thelibc.prxthat titles ship setsEDOMexactly where msun'se_fmod.cande_fmodf.creturn(x*y)/(x*y):For example,
fmod(1, 0)returns0xfff8000000000000withEDOMthere,fmod(NaN, 2)returns that NaN withEDOM, andfmod(1, inf)returns 1 withouterrno.The NaN on that path also differs from the host's:
(x*y)/(x*y).UCRT returns the dividend's NaN when both are NaNs, so the host's result differs on 19 of the 4,000,256 double pairs tested below and on 344 of the float ones.
Both functions now take that path themselves and set
EDOM; every other argument still goes to the hoststd::fmodwitherrnopreserved, whose finite results are exact.Tested
Current-main conflict refresh, 2026-10-27 UTC: exact head
8b4b0ecdfc2fbdb9ae3f10ec929d627cb942fed9on tested main74acfad65b3f0fd4eab653f3a3989d298fba1157. Resolved overlapping test/debt context while preserving this branch's authored production/test edits and main's nextafter environment implementation and coverage. Existing prerequisite commits remain in their original series order.Linux focused rebuild of
libcand selected test executables passesguest_math,guest_math_exact,guest_dinkumware_internals, andpublic_headers, each three times: 12 passes, zero skips. Conventions and whitespace checks pass. Final fetched maincb45ae2ddf6e113a4619e8fed91fe7b9333c5c0fmerges cleanly; its later change is an unrelated Windows crash-test pattern, and the executed qualification remains pinned to the stated base. No new Windows, console, GPU, title, full-suite or negative-control run is claimed. Earlier reference/Windows evidence keeps its original pins.Exact pins, branch patches, preservation proofs, build/test logs, JUnit and lease-push receipts for this refresh batch. SHA-256:
e33f98cc467f038af662cc5fa491c3acc1806bb47171cf77b748f0d4d052c1f2; downloaded copy verified.Latest conflict refresh (2026-10-26): exact head
11d52c9a9f5cdce272b71217031fac5f397bf997on mainbf77a9e9cba4b02c358779b35295176dd9cae164. The conflict was in adjacent debt notes; the production implementations and regression checks are preserved. Main's unrelated_LCosh/_LSinhdebt is retained.Rebuilt
libcand the focused test executables on Linux.guest_math,guest_math_exact,guest_dinkumware_internals, andpublic_headerspass: four groups × three repetitions (12 passes), zero skips. Conventions, whitespace and final main merge checks pass; the source tree is clean. This is a mechanical refresh with a debt-note resolution, so no new console, Windows, title, GPU or negative-control run is claimed. Earlier measurements retain their original exact pins.Patch/preservation checks, exact pins, build/test logs, JUnit and push receipts. SHA-256:
196fbf4f07b96515f278558ca31d139759c24195adfd35ddebc93f03c186c92b; downloaded copy verified.Current Linux rebase:
7385e2d9b96eac4c193f74c48cfa5a9a021f0540, one topic commit on main16ff208dc32cf3927ba1bae492eee363a5ae7ae8, without merge commits. Production and regression function bodies are unchanged from025a0c21203773de7ae94365aedaffd53beeceec; main's merged log1p implementation, regressions and calls remain intact. Additive regression insertions were retained on both sides.Fresh Linux x86-64 Release:
guest_math,guest_math_exact,guest_dinkumware_internalsandpublic_headerseach pass three repeats before and after the production-only main-Math negative control. The negative fails as expected while exact-math/Dinkumware controls pass; restored libc is byte-identical. Conventions and whitespace pass. The Release negative aborts without a named assertion; no more specific failure attribution is claimed.Fresh exact-head logs, negative/restoration evidence and preservation proof (SHA-256
fba4546fd2ffc5ea33459b5cc498adc8d2844b835d01d33e301b09d530c93087). No new Windows/macOS, title/module matrix or performance result is claimed. Earlier independent reports below retain their original measured revisions.Previous qualification (historical source pins)
Historical Linux refresh: head
025a0c21203773de7ae94365aedaffd53beeceec, one commit on main50a4f824fc3ee23177d8e1effbe306e50150187f(no merge commits). Original fmod/fmodf function bodies and CheckFmodConsoleBits are byte-identical; main's expm1 test/function call and other debt entries are retained. The obsolete standalone fmodf debt entry remains removed as reviewed.Linux 7.2.9 / Ryzen 9 7950X / GCC 16.2.1 / Release: fresh independent libc/math build;
guest_mathandguest_dinkumware_internalseach pass three runs. With only production Math.cpp reverted to this main and the fixture retained,guest_mathaborts; restoring the fix gives six passing invocations and the same libc library SHA-256413501dad82cc3f36177cc0b5e0eb8727289232d67f778a33aef3293acaeae32. No named assertion is emitted by this Release negative control. Conventions and whitespace pass. Windows remains user-paused: the comparison below is historical and has not been rerun on this head.Exact-head Linux build/test/negative-control receipts, SHA-256
81e2804b8c970b7a9605a8951dd685aeb1525c2976e4e9ac9713cf0ee166301c.Hosted Build 40151994198 completed successfully on this exact head, including Linux, Windows and both macOS relinker builds. These relinker jobs do not run the libc math regressions; full platform math claims remain scoped to the evidence above/below.
Historical Windows evidence (original implementation):
Windows 11 Enterprise 10.0.22621, Ryzen 9 5950X, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release,
BUILD_TESTING=ON,main65ac1d3.Comparison: a scratch loader runs the decrypted
libc.prxof eleven titles natively and supplies__error: PPSA04158, PPSA05916, PPSA07923, PPSA09614, PPSA13934, PPSA20423, PPSA25636, PPSA26528, PPSA33655, PPSA34008 and PPSA35169. It compared theirfmodandfmodfwith this PR's code on 4,000,256 pairs per title:DBL_MAX, ±inf, signalling and quiet NaNs of both signs);All eleven give identical bits and
errnofor both functions on all of them.EDOMis set on 3,118 of the double pairs and 23,617 of the float pairs. A separate 76-row edge probe (remainder,fmod,remquo,logb,ilogbandnextafter, double and float) prints byte-identical output on all eleven.guest_math: the newCheckFmodConsoleBitshas 15 double and 15 float cases with the console's results anderrno. They cover each error class with NaN payloads in both positions, a signalling NaN in each position, an infinite divisor, ±0 over ±0, subnormals, and ordinary remainders.Changed tests:
CheckFmodErrno(fix(libc): fmod leaves errno unchanged #3600) now expectsEDOMfromfmod(1, 0).fmodftable now expectsEDOMfor its NaN-result rows and the untouched sentinel for the others. Its environment check now expectsEDOMafterfmodf(1, 0).Their result,
FE_INVALIDand environment expectations are unchanged and pass. cc @CouchRiv @Noxy-HNRguest_math,guest_floating_pointandguest_dinkumware_internalspass in three runs out of three on the exact head.Negative control: with
main'sMath.cppand these tests,guest_mathaborts, while the other two still pass.A standalone
-O0and-O3build of the new code with the new vectors, fix(libc): preserve errno across fmodf domain errors #9643'sfmodftable and its environment check passes.python tools/check_conventions.py --base origin/mainandgit diff --checkpass.Not tested: Linux and macOS (the error path no longer depends on the host; finite results come from the host's exact
fmod), a console, the exception flags against the console, andfmodl.Independent macOS integration evidence
imellag reports Apple M4 Pro, macOS27.0.1, x86-64 under Rosetta2, using main
61fc572ceeplus open macOS build changes. Without this PR, four fmodf domain cases return NaN withoutFE_INVALID, andguest_mathfails at the reported(1,0)case. With this PR's domain expression, plus #9668 and the denormal-operand test adjustment #10907, the reporter gets three passingguest_mathruns; omitting this PR restores the failure. The comment does not give this PR's exact tested head, so this is attributed integration evidence, not a new exact-head or title run by us.Current Linux qualification (2026-10-28)
Rebased onto
main2227f1366188cda5699dd20e2e756a18006a5d4b. Linux CPU validation on 2026-10-28 at760f4f9386287b626560c16bcbe7363936741872(Arch Linux 7.2.9, AMD Ryzen 9 7950X, GCC 16.2.1, Ninja Release, BUILD_TESTING=ON).libc,guest_math_testsandguest_math_exact_testsrebuilt from this exact source.guest_mathandguest_math_exacteach passed three consecutive runs (6/6, no skips); the same two suites each passed three runs on the pinned main (6/6). Conventions and whitespace checks pass.Exact source pins, preservation proofs, dependency checks, build logs and JUnit, SHA-256
1176269c9e6948b5252a5db9e660978d7f978fb2d9f4ed6c68bc050b10b5ae68. This is focused Linux CPU qualification; earlier Windows/title/reference measurements retain their original pins. No new title or performance result is claimed.Current Linux qualification — 2026-10-29
Refreshed against main
5454cd2e455994d992aca39335ce60507eeda1eb.Linux qualification at
5b596a5d104db3f81d0241afa562fb8726f0c944(Arch Linux 7.2.9, Ryzen 9 7950X, GCC 16.2.1, Ninja Release, BUILD_TESTING=ON):guest_math,guest_math_exact,guest_dinkumware_internalseach pass three consecutive runs (9/9, no skips). Thepublic_headersaudit passes at this exact head. All tracked public-header files match the pinned main; the affected test targets rebuild against those headers. Conventions and whitespace checks pass. The same focused groups passed three times on the pinned main.Evidence: immutable Linux qualification bundle. Its SHA-256 is
53b095da2f67f55b8aa56ba273681e0330fb39739be844458038dde410fb8cc4. No fresh Windows runtime execution is claimed; older measurements retain their recorded source pins.AI-assisted: yes (Codex).
Additional main-conflict refresh — 2026-10-30
Refreshed to
6215d19689c83c8bd0d33239a341e7394cac215eagainst tested mainde2ba4cecb30ffb1c04d7e7dd693b03eace96b1b. Original authored changes, authorship and main gitlinks are preserved; publication used an exact old-head lease.The conflict was in debt documentation. Main's newer WebAssembly and license notes are preserved alongside this PR's measured fmod qualification. The production
fmod/fmodfbodies andCheckFmodConsoleBitsmatch the previous public head byte for byte; main's other math changes and tests remain. Fresh libc and test builds passguest_math,guest_math_exact,guest_dinkumware_internalsandpublic_headersthree times: 12 executions, zero skips. Earlier reference-library and platform measurements retain their original pins.Conventions, whitespace and source preservation checks pass. Latest main
42d009f5f37c8f388b01fbfe38d2e704c792c46fhas a clean merge preview; that preview tree was not executed. This refresh adds Linux CPU qualification; it adds no Windows/macOS, SDK, GPU, title or performance execution.Immutable source, build and test receipts, SHA-256
098ecc93f56b576370b295af428157ef9e4b8ae2dabfdcb9816869be02270985(download verified). AI-assisted: yes (Codex).Packaging note: the archive's nested
result.jsonretains an earlier packaging digest. The download SHA-256 above is authoritative; all 81 source/test proof files were checked unchanged, including the raw JUnit counts and published-head patches.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)AI-assisted (Claude Code).
🤖 Generated with Claude Code
Contributor-reported Windows follow-up: @ycbuzi tested earlier head
11d52c9a9fmerged onto mainf616a1397f, Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0 UCRT r7, Release. Four groups (guest_math,guest_math_exact,guest_dinkumware_internals,public_headers) each passed three times. Their main-productionMath.cppcontrol abortedguest_math; the other three groups passed. This is an attributed report on that earlier tree, without independently inspected raw logs or an assertion stack; it is not a Windows rerun of subsequent rebased heads. Original report.