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

Add ASYNCIFY+JSPI support to MINIMAL_RUNTIME build mode. #22501

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

juj
Copy link
Collaborator

@juj juj commented Sep 4, 2024

No description provided.

@@ -23,8 +23,15 @@ function assert(condition, text) {
}
#endif

#if ASYNCIFY == 1 // ASYNCIFY-mode requires checking ABORT variable to avoid operating if code has aborted during an unwind
var ABORT = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like this relates to ASYNCIFY == 1 but the PR title talks about JSPI. Does this change cover both?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Err, yes. Updated the title to clarify that this PR addresses both ASYNCIFY and JSPI.

@@ -23,8 +23,15 @@ function assert(condition, text) {
}
#endif

#if ASYNCIFY == 1 // ASYNCIFY-mode requires checking ABORT variable to avoid operating if code has aborted during an unwind
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a debug feature or do we need to add this new variable even in release builds?

I worry that this makes the feature matrix even more complicated and there could be other places in the codebase that deal with ABORT that need to be changes now that ABORT does exist undef MINIMAL_RUNTIME but only with ASYNCIFY is enabled.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added that variable to help places like this and this to compile.

There seem to be places that call to it in library_async.js: https://github.com/emscripten-core/emscripten/blob/main/src/library_async.js#L18 so looks like in ASYNCIFY==1 mode we cannot avoid adding ABORT in the build. In ASYNCIFY==2 mode, maybe we could.

Overall, I think the support would be ok to be expanded incrementally via proof by test cases.

@juj juj changed the title Add JSPI support to MINIMAL_RUNTIME build mode. Add ASYNCIFY+JSPI support to MINIMAL_RUNTIME build mode. Sep 10, 2024
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.

2 participants