WASIp3 Cooperative Threading#799
Conversation
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
An initial pass over things.
One major piece remaining in terms of synchronization is locks around the descriptor_table.h and derivative subsystems. None of that is locking-aware right now and would probably cause lots of problems if two threads tried to print to stdout for example. That's fine for an initial PR but we'll want to avoid broadcasting that threads are generally available until those issues are sorted out
| if (at) { | ||
| errno = ENOSYS; | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
Would it make sense to thread at into __waitlist_wait_on and have the error here generated by that function? That way there's sort of a centralized "this isn't supported" as opposed to having to make sure to write this down in all functions with a timeout parameter
| // TODO(wasip3) timed waits | ||
| if (ts) { | ||
| return ETIMEDOUT; | ||
| } |
There was a problem hiding this comment.
This one might be difficult to leave unsupported into the future as AFAIK waiting with timeouts on condvars is a relatively common operation. Leaving this as a TODO for now though is fine by me
| # Make the C function do the rest of work. | ||
| call __wasm_component_model_builtin_thread_index |
There was a problem hiding this comment.
Could this be deferred to the C entrypoint to avoid needing to write details like $root in asm files? That way it should be possible to use the generated bindings to get the current thread index on the C side of things
| static volatile int lock[1]; | ||
| volatile int *const __at_quick_exit_lockptr = lock; | ||
| #endif | ||
| DECLARE_STRONG_LOCK(lock, static); |
There was a problem hiding this comment.
is it intentional this is "strong" bu locked with "weak" below?
There was a problem hiding this comment.
Stray file name? (unusual to see a space)
There was a problem hiding this comment.
This I think is ok for an initial implementation, but I think we're going to want to eventually ensure that calls to __wasm_get_stack_pointer go directly to the builtin intrinsic without needing a wrapper in the middle. Otherwise there's basically no way to create a module that directly performs the call because even with LTO these won't get inlined.
| return thrd_busy; | ||
| } | ||
| } | ||
| int ret = __pthread_mutex_trylock((pthread_mutex_t *)m); |
There was a problem hiding this comment.
Would it make sense to have some sort of internal macro/header to do this cast to avoid needing to open-code it having the same representation?
| # Allocate a new stack | ||
| # Constant copied from __default_stacksize, TODO(wasip3) find a way | ||
| # to reference this constant without linking failing for shared libraries, | ||
| # i.e. a position-independent data reference. | ||
| i32.const 131072 | ||
| call malloc | ||
| call __wasm_set_stack_pointer | ||
|
|
||
| # Allocate a new TLS area | ||
| call __allocate_aligned_tls | ||
| call __copy_tls | ||
| call __wasm_set_tls_base |
There was a problem hiding this comment.
Would it be possible to delegate all of this to C instead of assembly? Or basically delegate as much of this function as possible to C instead of assembly here. I suspect that the initial instructions are required to be in assembly, but these look compatible (or at least a subset) to move over there
|
Another high level thing I'm just now remembering -- we'll need to afford a transition period from today-without-coop-threads to tomorrow-with-coop-threads-stable. Basically I think we'll want some sort of CMake flag for "ok turn on threads" which is default off for wasip3. That way the default build of wasi-sdk, for example, will have a libc.a which has pthread symbols but the symbols don't actually do anything (return errors as they do today). Once the I'd like to ideally still only have one target, Does that sound ok to you @TartanLlama? If so I was thinking |
2 participants
Currently a draft while the LLVM patches merge and I get all of this ready for review