Skip to content

Conversation

@inkeliz
Copy link
Contributor

@inkeliz inkeliz commented Jul 10, 2024

Previosly, using PURE_WASI was incompatible with
EMSCRIPTEN_MEMORY_GROWTH flag, and was not documented.

Now, using EMSCRIPTEN_MEMORY_GROWTH with PURE_WASI is possible. It will not require the host to export
emscripten_notify_memory_growth and will grow the memory as expected from EMSCRIPTEN_MEMORY_GROWTH.

Fixes #22211

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

lgtm!

I wonder if we can write a test for this?

@sbc100
Copy link
Collaborator

sbc100 commented Jul 10, 2024

Can you marge/rebase changes from the main branch? It should address the current test failures.

@inkeliz inkeliz force-pushed the main branch 2 times, most recently from c5f95fc to a486d38 Compare July 11, 2024 21:33
@sbc100 sbc100 enabled auto-merge (squash) July 11, 2024 21:35
auto-merge was automatically disabled July 15, 2024 17:04

Head branch was pushed to by a user without write access

@inkeliz inkeliz force-pushed the main branch 2 times, most recently from ed10348 to e9c9462 Compare July 15, 2024 17:05
@sbc100
Copy link
Collaborator

sbc100 commented Jul 15, 2024

It looks like this change fixes the test_memorygrowth_MAXIMUM_MEMORY_standalone test so that it is no longer impure.

Can you remove the impure=True from @also_with_standalone_wasm on line 2090 of test_core.py?

Previosly, using PURE_WASI was incompatible with
EMSCRIPTEN_MEMORY_GROWTH flag, and was not documented.

Now, using EMSCRIPTEN_MEMORY_GROWTH with PURE_WASI is
possible. It will not require the host to export
emscripten_notify_memory_growth and will grow the memory
as expected from EMSCRIPTEN_MEMORY_GROWTH.

Fixes emscripten-core#22211

Signed-off-by: inkeliz <[email protected]>
@sbc100 sbc100 enabled auto-merge (squash) July 15, 2024 23:24
@sbc100 sbc100 merged commit cb5af84 into emscripten-core:main Jul 15, 2024
verhovsky pushed a commit to verhovsky/emscripten that referenced this pull request Jul 30, 2024
…e#22217)

Previously, using PURE_WASI was incompatible with
EMSCRIPTEN_MEMORY_GROWTH flag, and was not documented.

Now, using EMSCRIPTEN_MEMORY_GROWTH with PURE_WASI is possible. It will
not require the host to export
emscripten_notify_memory_growth and will grow the memory as expected
from EMSCRIPTEN_MEMORY_GROWTH.

Fixes emscripten-core#22211
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.

ALLOW_MEMORY_GROWTH with PURE_WASI doesn't seems to work

2 participants