Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wasi-sdk-pthread.cmake: add --import-memory #297

Merged
merged 1 commit into from
Apr 7, 2023

Conversation

yamt
Copy link
Contributor

@yamt yamt commented Feb 15, 2023

No description provided.

@TerrorJack
Copy link
Contributor

Shall we also add --shared-memory and a large --max-memory default value?

@yamt
Copy link
Contributor Author

yamt commented Feb 15, 2023

Shall we also add --shared-memory and a large --max-memory default value?

--shared-memory is automatically added for -pthead.

i don't think there is a good single value for --max-memory. it depends on apps.

@yamt yamt marked this pull request as draft February 15, 2023 09:39
@yamt
Copy link
Contributor Author

yamt commented Feb 15, 2023

i marked this a draft because this needs llvm >=16.

@abrown
Copy link
Collaborator

abrown commented Feb 15, 2023

I am also wondering if we should rename this file wasi-sdk-threads.cmake?

toyobayashi added a commit to toyobayashi/emnapi that referenced this pull request Feb 16, 2023
@yamt
Copy link
Contributor Author

yamt commented Feb 17, 2023

I am also wondering if we should rename this file wasi-sdk-threads.cmake?

i don't think so as what this provides is pthread.

@yamt yamt marked this pull request as ready for review April 4, 2023 04:13
@yamt
Copy link
Contributor Author

yamt commented Apr 4, 2023

this is ready as we now ship llvm 16.

@@ -9,6 +9,11 @@ set(CMAKE_SYSTEM_PROCESSOR wasm32)
set(triple wasm32-wasi-threads)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pthread")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread")
# wasi-threads requries --import-memory.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# wasi-threads requries --import-memory.
# wasi-threads requires --import-memory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. thank you.

@yamt yamt force-pushed the wasi-threads-import-memory branch from 4bd8f7e to 99a0681 Compare April 7, 2023 03:49
@abrown abrown merged commit c891cd2 into WebAssembly:main Apr 7, 2023
yamt added a commit to yamt/wasi-testsuite that referenced this pull request Aug 10, 2024
wasi requires the memory exported as "memory".
certain runtimes actually requires it. (eg. wasmtime)

cf. WebAssembly/wasi-sdk#297

also, bump the wasi-sdk version because this doesn't work
with an old wasm-ld.
loganek pushed a commit to WebAssembly/wasi-testsuite that referenced this pull request Aug 12, 2024
wasi requires the memory exported as "memory".
certain runtimes actually requires it. (eg. wasmtime)

cf. WebAssembly/wasi-sdk#297

also, bump the wasi-sdk version because this doesn't work
with an old wasm-ld.
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.

3 participants