[Based on #2090] runtime: introduce G/M/P pthread backend#2166
Conversation
There was a problem hiding this comment.
Review: runtime G/M/P pthread backend
Overall this is a solid, well-structured change. The runtime side is clean and, if anything, a small improvement: go lowering now emits a single runtime.NewProc call, and newproc1 does one combined AllocRoot for the G/M/P context. The runtimeContext lifecycle (alloc → mexit → FreeRoot), the mutually-exclusive arg-free paths, and the EnterLocalContext/LeaveLocalContext pairing are internally consistent. The switch of the FuncForPC/symtab caches to atomic pointer publication is the correct call now that goroutines run on real threads. No blocking memory-safety defect found.
The actionable items below are all compile-time performance and maintainability concerns — none block correctness. Inline comments carry the specifics; a couple of non-inline notes follow.
Non-inline notes (low severity / nits):
runtime/internal/runtime/os_pthread.goheader still readsCopyright (c) 2024(carried over from thez_thread.gorename); every other new/rewritten file in this PR uses2026.runtime/internal/runtime/os_pthread.gonewosprocignores the return ofattr.Destroy(). Low risk and conventional (the create-critical returns —Init/SetDetached/SetStackSize/Create— are all checked), noting only for completeness.internal/build/main_module.godefineLibraryRuntimeInit: the doc block describes only theglobal_ctorsregistration but the function now also conditionally emits theenterLibraryLocalContextcall whenProg.NeedsLocalContext()is true. Worth extending the comment, since that call is what lets exported Go calls on a C constructor thread inherit a context.
2cd48fd to
47035fc
Compare
|
Addressed the non-inline review notes as well:
Validation on the final head:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
47035fc to
0c415b2
Compare
Depends on #2090
Problem
The compiler currently lowers every
gostatement directly to pthread setup. That couples goroutine code generation to one OS backend and provides no runtime-owned G/M/P boundary for a future scheduler.Change
ginto minimal Go-compatible G, M, and P objects;runtime.NewProc;newproc1,newm,mstart, andmexit;runtime.Goexitthrough the same cleanup path;mexitfor runtime-created goroutines and by the pthread destructor for bootstrap/external threads;This establishes the replaceable native backend boundary proposed in #2152. It does not add M:N scheduling, preemption, growable stacks, LLVM coroutines, or runtime assembly reuse.
The independent diff is
+622/-181across 15 files. It contains no #2079 locality compiler/runtime files.Validation
Tests were serialized with
GOMAXPROCS=2,GOMEMLIMIT=8GiB, and-p=1../ssa, runtime compilation, and real-LLGo G/M/P tests pass.getgisolation, and defer isolation.Reusing pinned GOROOT runtime and assembly code remains tracked separately in #2165.