Skip to content

build(libkernel): compile Thread on macOS - #11246

Closed
iamthehimansh wants to merge 1 commit into
boykopovar:mainfrom
iamthehimansh:split/macos-thread
Closed

iamthehimansh wants to merge 1 commit into
boykopovar:mainfrom
iamthehimansh:split/macos-thread

Conversation

@iamthehimansh

Copy link
Copy Markdown
Contributor

What

Next part of splitting #8658: libkernel's Pthread/src/Thread.cpp does not compile on macOS. The change is macOS-only; Linux and Windows compile the same code as before.

  • Stack bounds: macOS has no pthread_getattr_np. It takes the stack top from pthread_get_stackaddr_np and the size from pthread_get_stacksize_np.
  • Thread CPU time: macOS has no pthread_getcpuclockid. The thread's Mach port is stored when the thread starts, and its user plus system time is read with thread_info(THREAD_BASIC_INFO). Pthread.hpp holds that port in place of the clockid_t on macOS.

#11201 and #10856 also change Thread.cpp, in other functions; neither touches these lines.

Tested

macOS 26 on an Apple M2 Pro, x86_64 under Rosetta 2, Release with Ninja, on main at b90d009d9: Pthread/src/Thread.cpp.o and Pthread/Posix/Thread.cpp.o compile, where main fails the first. The libkernel tests cannot run on macOS until the rest of libkernel builds (#11240 and follow-ups), so this is a compile check only. On #8658's branch the same code passed guest_thread_cpu_clock 3 of 3 runs. For Linux and Windows, I checked each file's view under unifdef -U__APPLE__, with -D__linux__ and with -D_WIN32: it equals main. python3 tools/check_conventions.py --base origin/main passes.

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 under Silent stubs in their library's technical debt file
  • 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
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes (Cana)

Pthread/src/Thread.cpp reads the stack bounds with pthread_getattr_np and
binds the thread CPU clock with pthread_getcpuclockid, neither of which
macOS has. macOS now takes the stack bounds from pthread_get_stackaddr_np
and pthread_get_stacksize_np, and reads the thread's user and system time
with thread_info on its Mach thread port, which Pthread.hpp now stores
in place of the clock id on macOS.

Linux and Windows compile the same code as before.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macOS branches are consistent with the APIs used: the stack bounds come from the pthread stack address/size, and the cached pthread_mach_thread_np port is queried with THREAD_BASIC_INFO for user plus system CPU time. I did not find a concrete code defect. The checks on this head pass eligibility, conventions, intake, labels and report; product/build jobs are skipped, and the PR's local evidence is compile-only.

Because both changed behaviors are macOS-only, could someone with macOS run the existing guest-thread stack and CPU-clock tests (or add focused coverage if those tests do not exercise these branches) before merge? I have not run the runtime paths on macOS, so I am leaving this as a comment rather than approving on compile evidence alone.

@github-actions

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 3986 core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 4563 core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 5021 core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 6740 core/libs/prx/libkernel/Pthread/include/Pthread.hpp
PR 7149 core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 7201 core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 7804 core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 8592 core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 9684 core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 9938 core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 10061 core/libs/prx/libkernel/Pthread/include/Pthread.hpp

@iamthehimansh

Copy link
Copy Markdown
Contributor Author

Closing this in favour of @mugurc's #7804, as @MarcZX10 pointed out. #7804 is older, already reviewed, and carries the same macOS code for this file: stack bounds from pthread_get_stackaddr_np / pthread_get_stacksize_np, thread CPU time from thread_info(THREAD_BASIC_INFO), and process arguments from _NSGetArgc / _NSGetArgv. In sceKernelGetCurrentCpu it goes further than this PR: it reads the real core with pthread_cpu_number_np, where this PR always reports CPU 0.

I missed it because my overlap check only fetched the first 500 of the ~1,000 open PRs, so it never compared against #7804. That's fixed now. Sorry for the noise, and thanks for the review.

AI-assisted: yes (Cana)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants