Repository navigation
build(libs): support Emscripten memory64 compilation - #11341
Conversation
Progress report (91a6995 → 9824860)📉 System libraries: 43.22% (-0.02%, +2 declared, -1 reverted functions) 🆕 2 declared as stubs
|
| Library | Name |
|---|---|
| libkernel | sceKernelGetModuleInfoForUnwind |
Changed libraries (implemented exports)
| Library | Before | After | Change |
|---|---|---|---|
| libkernel | 700/746 (93.83%) | 699/748 (93.45%) | -0.38% |
MarcZX10
left a comment
There was a problem hiding this comment.
Blocking Emscripten runtime path: this new __EMSCRIPTEN__ branch delegates to sceKernelGetModuleInfoForUnwind. In core/libs/prx/libkernel/Module/src/Module.cpp, Emscripten falls through to the generic non-Apple implementation, which calls HostMapping::Find; HostMapping.hpp reads /proc/self/maps and throws if it cannot open it. The Emscripten portability workflow compiles libSceSysmodule/Export.cpp but omits libkernel/Module/src/Module.cpp, and its test only exercises long-double conversion, so this route is neither compiled nor runtime-tested by the new check. Please add an explicit Emscripten implementation (or make this operation fail as an explicitly unsupported path) and include its translation unit and a focused test in the portability workflow. Please also change the checklist dependency from #10017 to none: #10017 is already merged, so it is no longer a PR that must land first.
MarcZX10
left a comment
There was a problem hiding this comment.
The prior blocking issue is resolved at this head. Under Emscripten, the generic HostMapping implementation is excluded, the kernel unwind query reports the unsupported path explicitly, and the sysmodule entry point forwards to that same path. The new Node regression links both production translation units, checks unsupported queries leave output unchanged across representative addresses and flags, and preserves the kernel null-output EFAULT result. The focused library-portability check and conventions check pass at fcb7255. The checklist now correctly lists no dependency on already-merged #10017. I did not independently run the full native library build; the automatic workflow reports the full native library jobs as skipped.
|
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.
Exports introduced here that are already present in
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.
|
MarcZX10
left a comment
There was a problem hiding this comment.
The previous module-unwind blocker is resolved, and the current hosted Emscripten portability, relinker, register-context and write-watch checks pass. The native full-library Linux and Windows jobs are still skipped by the automatic gate, as the report correctly says.
The reviewed head fcb72551ab9ad72d21290ab8d4f1b818abb3fbad is now 4 commits ahead and 52 behind current main (14c99b52ac), with a clean merge preview. The reported native/memory64 builds and Node regressions were run against 9b197b6ce4583e64bf2427e6beb7270bcfa1528e. CONTRIBUTING.md asks branches to stay current; please rebase onto current main, resolve the debt-file overlaps, and rerun the library-portability job at the refreshed head. I found no changed production file overlap on the intervening commits, but this broad portability change needs exact-head validation after the refresh.
|
Follow-up to my existing review: current upstream |
|
Current-main follow-up ( |
|
My draft #11332 rewrites the narrow formatter and conflicts with your |
fcb7255 to
4a18853
Compare
|
Published the current-main refresh at @MarcZX10 The fresh library-portability run is running on this head. The earlier stale-head results have been replaced in the description; merge-order replies and the remaining unconfirmed overlaps are recorded separately. Please take another look after this run completes. @adriadam10 Could you dispatch the full native Build on main for
Thanks for the review and CI help. |
Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.
|
Thanks for confirming #11341 before #11332. I have updated my draft's description/checklist to record the agreement. I will preserve GuestLongDoubleLoad and host-layout independence when resolving the formatter after its prerequisites land; the existing test results remain scoped to their published heads. |
|
@SteelCrab an overlap note from #12514 ( AI-assisted: yes (Claude Code). |
|
@SteelCrab a merge-order question, following my earlier note about the shared Proposed order: this PR lands first, and #12514 rebases afterwards (#12514 will declare it in Depends on). Your PR needs no changes. Could you confirm that order works for you, or say if you'd rather #12514 went first? AI-assisted: yes (Claude Code). |
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — changes requested
The author has now published current-main validation on head bb065fccc0b2a5ee3fc3b8deeceb94b2245de901: the report includes native and memory64 compilation of 18 sources, long-double and Node regressions, and the VideoOut policy test. The hosted portability, relinker, register-context, write-watch, Windows, conventions, and policy checks pass; Linux product CI is skipped.
The tested base is d7fd41699238337a84cc61de848b5523a938031d. Current main is 2960e9a6e7e1fc99c4ed180c71495b5eb83464c7, and this branch is 831 commits behind. Nine touched paths changed on both sides since the tested base, including libc/libkernel source files compiled by the memory64 validation. The merge preview is clean, but those source changes are part of the feature’s compile surface. Please refresh the branch and rerun the native/memory64 source compilation and focused regressions against that integrated head before I can approve.
I did not run Emscripten or Node tests locally.
bb065fc to
d5ba05c
Compare
MarcZX10
left a comment
There was a problem hiding this comment.
Re-check — changes requested\n\nThe refreshed validation on exact head d5ba05c is substantial: the author reports 18 native and memory64 translation units compiled, native long-double 1/1, Node regressions 2/2, and the CPU-only VideoOut policy comparison passing on both main and the change. The branch is based on current main aaccd0b, and the report clearly distinguishes the maintainer-dispatched full native suites and GPU rendering that were not run.\n\nOne repository coordination blocker remains, and the PR body confirms it is unresolved: #12514 changes the same %Lf store in ScanningWide.cpp and reports a 10-byte store with preserved padding/NaN normalization, while this PR's GuestLongDoubleStore writes 16 bytes. The proposed order is #11341 first, but #12514's author has not confirmed it and the checklist still says Depends on: none. Please confirm the order with that author, record Depends on #12514 only if this PR must follow it (otherwise record the agreed order on both PRs), and resolve the 10-byte interaction before landing. No combined behavior has been tested. I did not independently run the author-reported builds or Node tests.
d5ba05c to
5f2d455
Compare
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — new head, changes requested
The branch has advanced, but its test report still names the older head and the required landing order with the overlapping #12514 remains unconfirmed.
What I ran
- Reviewed head
5f2d45501d2acb9b7407df37f7842ac39d8f427bagainst currentmaindfcc0e17cb254d08fec9ec120e362620795d59a0; the merge preview is clean. - Checked the branch distance: it is 37 commits behind and 6 ahead of current
main. - Compared current-main changes since the PR's integrated main
7aabd03d0a32bb928311ef1323b95dffb4618f7e;Process.cppanddocs/dev/debt/libkernel.mdoverlap this PR. - Ran
git diff --check upstream/main...refs/remotes/review/pr-11341; it passed with no output. - Read the current Tested section and exact-head hosted checks. I did not run a native or Emscripten build locally.
git merge-tree --write-tree upstream/main refs/remotes/review/pr-11341
git rev-list --left-right --count upstream/main...refs/remotes/review/pr-11341
git diff --name-only 7aabd03d0a32bb928311ef1323b95dffb4618f7e upstream/main
git diff --check upstream/main...refs/remotes/review/pr-11341
gh pr checks 11341 -R boykopovar/AnyPS5Results
| Check | Result |
|---|---|
Merge preview against current main |
Pass; clean tree d4014a7a4ed1d5220dca5debd46ef0089663d896 |
| Branch distance | 37 behind / 6 ahead |
| Diff check | Pass; no output |
| Current-main changes overlapping this PR | core/libs/prx/libkernel/System/src/Process.cpp; docs/dev/debt/libkernel.md |
| Tested-section head | Still says d5ba05c0f4035d306b5a839c9c506393e03bd53a, not the reviewed 5f2d45501d2acb9b7407df37f7842ac39d8f427b |
| Hosted conventions, eligibility, labels, portability, and report | Pending |
Unedited command output
d4014a7a4ed1d5220dca5debd46ef0089663d896
37 6
core/libs/prx/libkernel/System/src/Process.cpp
docs/dev/debt/libkernel.md
conventions pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/40882774691/job/123478414631
eligibility pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/40882780480/job/123478432352
labels pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/40882774743/job/123478414774
library-portability pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/40882780502/job/123478432302
report pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/40882780516/job/123478431998
Negative control
| Assertion | Change made to code under test | New test failed? |
|---|---|---|
| The native and Emscripten portability regressions distinguish this head from main | None | Could not exercise; the reported runs target d5ba05c0f4035d306b5a839c9c506393e03bd53a |
Findings
Blocking
- Current-main integration and test evidence: the PR is 37 commits behind current
main; two touched paths changed since the integrated base, while the Tested section names an older head and the portability CI for this head is pending. Please integrate currentmainand rerun the native and memory64 portability tests on the exact updated head. - Unconfirmed overlap order: the PR changes the same
%Lfstore inScanningWide.cppas #12514, and its own Tested section says the proposed order is not confirmed. The current contribution rules require the authors to agree an order and declare it withDepends on. Please confirm the order with #12514's author and update the checklist.
Non-blocking
The merge preview is clean. The body accurately distinguishes the earlier test results from validation still pending on this head.
Sources
- Current contribution rules at main
dfcc0e17 - The overlapping
%Lfimplementation at reviewed head - PR's current tested section and stated #12514 order
Not verified
I did not build native or Emscripten targets, run Node tests, or run host, browser, title, or console execution on this head.
To get this to a good state
Rebase onto current main, rerun native and memory64 portability tests on the resulting head, and record the author-agreed #12514 order in Depends on; I will repeat the same checks on that head.
|
@MarcZX10 @SteelCrab from the author of #12514, to settle the order on this side: #11341 lands first. #12514 already declares AI-assisted: yes (Claude Code). |
✅ NVIDIA GPU test: passedPR head
Failing on main too (6, not counted against this PR)Validation IDs already present on main (5 IDs in 686 tests)Skipped (40)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Emscripten targets wasm64 without x86 assembly, Linux-only headers or the x87 long double. These changes add __EMSCRIPTEN__ paths beside the host ones; native builds compile the same code as before. - libc: long doubles in guest varargs and scanf go through GuestLongDouble, a plain copy where long double is the x87 format; APS5_EXPORT emits no assembler alias; the guest arena is one aligned allocation that zeroes pages when they are committed again; getifaddrs keeps its link-level entries on Linux; /proc lookups and the host stderr duplicate are left out. - libkernel: file-backed mmap, EVFILT_VNODE watches, getcontext and setcontext throw; getdents replaces the raw getdents64 syscall; getdtablesize takes the resource limit alone. - VideoOut leaves SDL, the display window and the HDR probe out, Sysmodule asks libkernel for unwind information and Net includes sys/time.h.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
5f2d455 to
9824860
Compare
✅ NVIDIA GPU test: passedPR head
Intermittent validation messages (1, seen once in the parallel run, not reproduced when the test was re-run alone 2 times; not counted against this PR)Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (5 IDs in 689 tests)Skipped (41)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
Checked the x87/binary128 long-double load and store (zeros, normals, denormals, inf/NaN, sign), the Emscripten guest arena including the new host-region release throw, the unsupported paths (context switching, module unwind, vnode watches, file/shared mappings) and the new workflow and tests; the native |
What
Compile shared system-library sources with Emscripten memory64: adapt guest memory, module/file/process interfaces and ABI declarations, and load/store guest x87 long doubles on a binary128 host. Unsupported context switching, module unwind queries, vnode watches, mapping operations and the new host-region release path throw; the remaining limitations are recorded in library debt files.
Replaces closed #9092 and follows merged #10017. The module-unwind review is addressed by an explicit unsupported Emscripten path and a linked Node regression. #8864 and #9506 are already in main; their native Bessel and DNS changes are retained.
Tested
Ubuntu 26.04.1 x86-64, GCC 15.2, Emscripten 6.0.12, Node 26.12.0. Refreshed on 2026-10-30 onto main
91a69952c10099966378a6a9006b8ef616ae22da; tested head98248606d4ef786f04329ec815334b6539e88035.guest_long_double,emscripten_module_unwind). The latter links both production module sources, checks unsupported queries preserve output across addresses and flags, and preserves the kernel null-outputEFAULTresult.video_out_output_mode_policyon the same machine: main 1/1; change 1/1 passed. Both verbose runs usedANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1, andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation. Neither log contained the missing-layer message or aVUID-error. This CPU-only test does not exercise Vulkan or GPU rendering.git diff --checkpassed.CI coverage:
library-portabilitylinuxandwindowsconventionsRemote validation for the new head is pending. The prior head's hosted portability and Conventions jobs passed; its automatic Build skipped Linux and its Windows configure/link job did not establish full-library CTest coverage. A maintainer dispatch of the full native Build remains needed for the refreshed head.
Overlap coordination:
GuestLongDoubleLoadand avoiding host long-double layout assumptions when rebased.ScanningWide.cppat the%Lfstore:GuestLongDoubleStorewrites 16 bytes, while fix(libc): read swscanf floating fields as titles' libc.prx does #12514 requires a 10-byte write preserving padding and canonicalizing NaNs. A scratch merge of fix(libc): read swscanf floating fields as titles' libc.prx does #12514 headccdd878bd9184e5031ddf1108c9d7044821ebeb9with the preceding rebased heade212645586b8db1cd67beaf8cd66fc990040b5c9encoded into a temporary 16-byte buffer and copied 10 bytes. A focused scanner harness passed 4 cases (finite value, negative zero, infinity and canonical NaN, with padding and canaries checked) on native and memory64/Node. This is a candidate integration test, not a test of the future rebased fix(libc): read swscanf floating fields as titles' libc.prx does #12514 or full suite.91a69952, the scanner reports conflicts with perf(libc): skip the known occupied guest arena prefix #7938 (guest arena), fix(libkernel): map unbacked AMM PRT pages to one read-only zero page on Windows #10025 (direct memory), and fix(linux): enter the guest entry on a main stack at the console's address #7532 / fix(libc): set EDOM in fmod and fmodf where titles' libc.prx does #10345 (debt entries). Some branches could not be compared because they already conflict with main, including fix(libc): bound narrow formatting fields #11332 and fix(libc): read swscanf floating fields as titles' libc.prx does #12514, so no blanket overlap-free claim is made. The scanner also listsgetcontext,setcontextandswapcontextas already in main: this PR adds their mutually exclusive Emscripten unsupported branches, while retaining the existing native definitions. Its build(libs): support Emscripten memory64 compilation #11341 entry compares this PR against its previous remote head.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)