-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Simplify EM_ASM handling #11972
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
Simplify EM_ASM handling #11972
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is hard for me to see that his is preserving the exact same behavior, but in general it looks like step in the right direction.
Excited to see less happening during finalize for sure.
return window.almost_PI; | ||
}); | ||
printf("almost PI: %f\n", almost_PI); | ||
assert(fabs(almost_PI - 3.14159) < 0.001); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What the changes to the test code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some added testing. We tested MAIN_THREAD_EM_ASM_INT
but not _DOUBLE
.
@@ -4186,12 +4229,6 @@ LibraryManager.library = { | |||
llvm_dbg_value: function() {}, | |||
llvm_debugtrap: function() {}, | |||
llvm_ctlz_i32: function() {}, | |||
emscripten_asm_const: function() {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function no longer exists? I guess it was never needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it may have made sense in some old approach to EM_ASM in fastcomp perhaps.
Loosk like its usage was removed in #11972.
Loosk like its usage was removed in #11972.
Emscripten counterpart to WebAssembly/binaryen#3044 , see background there,
and WebAssembly/binaryen#3043
This basically gets rid of the special handling of EM_ASMs. Instead, they are just
normal JS library methods like any other.
(The one interesting thing in them, unchanged from before this PR, is that we
need to read the varargs before doing any proxying, see the comment there for
more details.)