test: add tests for pthread API #369
Conversation
This change runs the pthread tests available in `libc-test` when
`THREAD_MODEL=posix`. This is currently a work-in-progress, since there
are custom build steps necessary to get this all to (mostly) work.
Currently this might look like:
```console
$ cd test
$ make run THREAD_MODEL=posix \
ENGINE=~/Code/wasmtime/target/debug/wasmtime \
CC=~/Code/llvm-project/build/bin/clang
```
What are the current issues?
- this requires a special build of Wasmtime that includes the
`wasi-threads` implementation; this is shown above as built from
source
- under `THREAD_MODEL=posix`, this requires Clang to understand the
`--export-memory` flag which, though merged in
https://reviews.llvm.org/D135898, is not available for download yet
(but can be built from latest source as shown above)
- not all of the tests yet pass and some do not build: this is to be
expected in several cases (PI? robust?) but there is perhaps more that
can be done here to enable more of the pthread API. Only the working
tests are included in this commit.
abrown
force-pushed
the
pthread-tests
branch
from
6cce377
to
2c12613
Compare
| # Specify the tls-model until LLVM 15 is released (which should contain | ||
| # https://reviews.llvm.org/D130053). | ||
| CFLAGS += --target=wasm32-wasi-pthread -mthread-model posix -pthread -ftls-model=local-exec | ||
| # NOTE: `--export-memory` is invalid until https://reviews.llvm.org/D135898 is |
There was a problem hiding this comment.
The change related to being able to export or import the memory with a specific name --export-memory on its own (using the default name) I think has existed for a long time.
There was a problem hiding this comment.
I was only able to see the --export-memory flag in a built-from-tree version of Clang. Should I be able to find it in some binary version of Clang?
There was a problem hiding this comment.
Oh maybe you are right.. exporting of the memory was always just the default.
Remind me why you need to both import and export the memory? .. emscripten doesn't do that today.
There was a problem hiding this comment.
Uh, well, not by choice:
- we want to import the shared memory to avoid magic: the shared memory is created outside of the instance and passed in as an import
- then, Wasmtime wants me to also export the memory because this is how the Wiggle integration figures out where buffers are at when a WASI function is called with a buffer as a parameter.
There was a problem hiding this comment.
Hmm.. that seems like unique concern of wasmtime and/or wiggle and perhaps we should at least make a note of that. Perhaps add --import-memory (which all runtimes will need) on the line above and then leave --export-memory on a line by itself with a comment? Perhaps something like "with the instnace-per-thread model we always need to import the memory, and under wasmtime we also need re-export this imported memory. The ability to both export and import a memory is only available in recent versions of wasm-ld".
There was a problem hiding this comment.
I don't think there is any need to make that assumption. no. If the runtime wants a static signal it could use the import of wasi_thread_create.
I'm not sure a runtime necessarily needs to know statically that a program is mulithreaded, though. A runtime could just implement wasi_thread_create and have it create a new thread without necessarily knowing up front couldn't it? This wouldn't be very useful without an imported shared memory though.... so we might want to specify that a program that calls wasi_thread_create needs to import at least one shared memory?
There was a problem hiding this comment.
well, "automatically create shared memory which satisfies imports" is a very wasi-threads specific behavior, isn't it?
There was a problem hiding this comment.
Reading through this chain so far, I think @sbc100 has explained things well. We need to import a shared memory and in Wasmtime we will know to do so because of configuration that turns on Wasm threads and wasi-threads. My takeaway is that we could clarify the README a bit more in this regard but I do want to be a bit cautious about overspecifying to allow different runtimes to implement the configuration as they wish.
There was a problem hiding this comment.
ok, let me see how i can amend my implementation. after that experiment, maybe i will submit a README patch.
There was a problem hiding this comment.
ok, let me see how i can amend my implementation. after that experiment, maybe i will submit a README patch.
done.
WebAssembly/wasi-threads#19 (comment)
yamt/toywasm@4d81846
WebAssembly/wasi-threads#20
|
cc: @loganek, I had forgotten about WebAssembly/wasi-threads#11 (sorry!) but perhaps some of the tests here can be adapted for use there in the spec? Also, note here how much more complex the "build and run" scripting must be to get working examples. |
| # The phony `env` target can be used to debug this. | ||
| ENV = "CC=$(CC);CFLAGS=$(CFLAGS);LDFLAGS=$(LDFLAGS);THREAD_MODEL=$(THREAD_MODEL)" | ||
| export ENV | ||
| ENV_HASH = $(OBJDIR)/env-$(shell echo $(ENV) | md5sum | cut -d ' ' -f 1) |
There was a problem hiding this comment.
macOS doesn't have md5 command.
diff --git a/test/Makefile b/test/Makefile
index 7f99f62..d52e5c2 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -153,7 +153,7 @@ endif
# The phony `env` target can be used to debug this.
ENV = "CC=$(CC);CFLAGS=$(CFLAGS);LDFLAGS=$(LDFLAGS);THREAD_MODEL=$(THREAD_MODEL)"
export ENV
-ENV_HASH = $(OBJDIR)/env-$(shell echo $(ENV) | md5sum | cut -d ' ' -f 1)
+ENV_HASH = $(OBJDIR)/env-$(shell echo $(ENV) | openssl md5 -r | cut -d ' ' -f 1 | cut -c 1-)
$(ENV_HASH): | $(OBJDIRS)
rm -f $(OBJDIR)/env-*
echo $(ENV) > $@
| # Specify the tls-model until LLVM 15 is released (which should contain | ||
| # https://reviews.llvm.org/D130053). | ||
| CFLAGS += --target=wasm32-wasi-pthread -mthread-model posix -pthread -ftls-model=local-exec | ||
| # NOTE: `--export-memory` is invalid until https://reviews.llvm.org/D135898 is |
There was a problem hiding this comment.
--import-memory here is a wasmtime-specific kludge, isn't it?
|
i needed #372 for tls tests to pass on toywasm. |
as suggested in WebAssembly/wasi-libc#369 the corresponding toywasm change: yamt/toywasm@4d81846 ``` spacetanuki% TOYWASM=~/git/toywasm/b/toywasm python3 ~/git/wasm/wasi-testsuite/test-runner/wasi_test_runner.py -t ./test/testsuite -r ~/git/toywasm/test/wasi-testsuite-adapter.py Test wasi_threads_exit_nonmain_wasi passed Test wasi_threads_exit_main_busy passed Test wasi_threads_exit_main_wasi passed Test wasi_threads_exit_nonmain_busy passed Test wasi_threads_spawn passed Test wasi_threads_exit_main_block passed Test wasi_threads_exit_nonmain_block passed ===== Test results ===== Runtime: toywasm v0.0 Suite: WASI threads proposal Total: 7 Passed: 7 Failed: 0 Test suites: 1 passed, 0 total Tests: 7 passed, 0 total spacetanuki% ```
| @@ -15,6 +15,10 @@ test: run | |||
| # directory (keeping with the `wasi-libc` conventions). | |||
| OBJDIR ?= $(CURDIR)/build | |||
| DOWNDIR ?= $(CURDIR)/download | |||
| # Like the top-level `wasi-libc` Makefile, here we can decide whether to test | |||
| # with threads (`posix`) or without (`single`). IMPORTANT: the top-level include | |||
There was a problem hiding this comment.
Feels like yet another argument in favor of cmake
as suggested in WebAssembly/wasi-libc#369 the corresponding toywasm change: yamt/toywasm@4d81846 ``` spacetanuki% TOYWASM=~/git/toywasm/b/toywasm python3 ~/git/wasm/wasi-testsuite/test-runner/wasi_test_runner.py -t ./test/testsuite -r ~/git/toywasm/test/wasi-testsuite-adapter.py Test wasi_threads_exit_nonmain_wasi passed Test wasi_threads_exit_main_busy passed Test wasi_threads_exit_main_wasi passed Test wasi_threads_exit_nonmain_busy passed Test wasi_threads_spawn passed Test wasi_threads_exit_main_block passed Test wasi_threads_exit_nonmain_block passed ===== Test results ===== Runtime: toywasm v0.0 Suite: WASI threads proposal Total: 7 Passed: 7 Failed: 0 Test suites: 1 passed, 0 total Tests: 7 passed, 0 total spacetanuki% ```
as suggested in WebAssembly/wasi-libc#369 the corresponding toywasm change: yamt/toywasm@4d81846 ``` spacetanuki% TOYWASM=~/git/toywasm/b/toywasm python3 ~/git/wasm/wasi-testsuite/test-runner/wasi_test_runner.py -t ./test/testsuite -r ~/git/toywasm/test/wasi-testsuite-adapter.py Test wasi_threads_exit_nonmain_wasi passed Test wasi_threads_exit_main_busy passed Test wasi_threads_exit_main_wasi passed Test wasi_threads_exit_nonmain_busy passed Test wasi_threads_spawn passed Test wasi_threads_exit_main_block passed Test wasi_threads_exit_nonmain_block passed ===== Test results ===== Runtime: toywasm v0.0 Suite: WASI threads proposal Total: 7 Passed: 7 Failed: 0 Test suites: 1 passed, 0 total Tests: 7 passed, 0 total spacetanuki% ```
as suggested in WebAssembly/wasi-libc#369 the corresponding toywasm change: yamt/toywasm@4d81846 ``` spacetanuki% TOYWASM=~/git/toywasm/b/toywasm python3 ~/git/wasm/wasi-testsuite/test-runner/wasi_test_runner.py -t ./test/testsuite -r ~/git/toywasm/test/wasi-testsuite-adapter.py Test wasi_threads_exit_nonmain_wasi passed Test wasi_threads_exit_main_busy passed Test wasi_threads_exit_main_wasi passed Test wasi_threads_exit_nonmain_busy passed Test wasi_threads_spawn passed Test wasi_threads_exit_main_block passed Test wasi_threads_exit_nonmain_block passed ===== Test results ===== Runtime: toywasm v0.0 Suite: WASI threads proposal Total: 7 Passed: 7 Failed: 0 Test suites: 1 passed, 0 total Tests: 7 passed, 0 total spacetanuki% ```
``` (venv) spacetanuki% TOYWASM=~/git/toywasm/b/toywasm python3 test-runner/wasi_test_runner.py -t ~/git/wasm/wasi-threads/test/testsuite -r ~/git/toywasm/test/wasi-testsuite-adapter.py Test wasi_threads_exit_nonmain_wasi passed Test wasi_threads_exit_main_busy passed Test wasi_threads_exit_main_wasi passed Test wasi_threads_exit_nonmain_busy passed Test wasi_threads_spawn passed Test wasi_threads_exit_main_block passed Test wasi_threads_exit_nonmain_block passed ===== Test results ===== Runtime: toywasm v0.0 Suite: WASI threads proposal Total: 7 Passed: 7 Failed: 0 Test suites: 1 passed, 0 total Tests: 7 passed, 0 total (venv) spacetanuki% ``` * import shared memory in wat tests as suggested in WebAssembly/wasi-libc#369 the corresponding toywasm change: yamt/toywasm@4d81846 ``` spacetanuki% TOYWASM=~/git/toywasm/b/toywasm python3 ~/git/wasm/wasi-testsuite/test-runner/wasi_test_runner.py -t ./test/testsuite -r ~/git/toywasm/test/wasi-testsuite-adapter.py Test wasi_threads_exit_nonmain_wasi passed Test wasi_threads_exit_main_busy passed Test wasi_threads_exit_main_wasi passed Test wasi_threads_exit_nonmain_busy passed Test wasi_threads_spawn passed Test wasi_threads_exit_main_block passed Test wasi_threads_exit_nonmain_block passed ===== Test results ===== Runtime: toywasm v0.0 Suite: WASI threads proposal Total: 7 Passed: 7 Failed: 0 Test suites: 1 passed, 0 total Tests: 7 passed, 0 total spacetanuki% ```
|
Note to self: this should get moved to wasi-sdk. |
This change runs the pthread tests available in
libc-testwhenTHREAD_MODEL=posix. This is currently a work-in-progress, since there are custom build steps necessary to get this all to (mostly) work. Currently this might look like:What are the current issues?
wasi-threadsimplementation; this is shown above as built from sourceTHREAD_MODEL=posix, this requires Clang to understand the--export-memoryflag which, though merged in https://reviews.llvm.org/D135898, is not available for download yet (but can be built from latest source as shown above)