-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
AUTODEBUG support for wasm files #8472
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.
Nice!
@@ -891,6 +891,90 @@ function getBinaryPromise() { | |||
// Create the wasm instance. | |||
// Receives the wasm imports, returns the exports. | |||
function createWasm(env) { | |||
#if AUTODEBUG | |||
env['setTempRet0'] = setTempRet0; | |||
env['getTempRet0'] = getTempRet0; |
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.
I thought these two functions lived in wasm and were generated by binaryen? Maybe fastcomp only?
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.
The issue is that the instrumentation passes may add a requirement for those functions, that wasn't there before (since there are the i64 logging calls, which may be the first illegal imports).
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.
Hmm.. I was confused I thought binaryen always generated these function on the wasm side.. maybe I was confusing them with stackSave/stackRestore
passes += ['--instrument-locals'] | ||
passes += ['--log-execution'] | ||
passes += ['--instrument-memory'] | ||
passes += ['--legalize-js-interface'] |
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.
I guess most of the heavy lifting to modify the code is done here in binaryen?
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.
Yeah, these binaryen passes do all the actual instrumentation. Basically this PR automates the JS side of adding the necessary stuff for that instrumentation to be useful.
tests/test_core.py
Outdated
if self.is_wasm_backend(): | ||
# autodebugging only works with bitcode objects | ||
# bitcode autodebugging only works with bitcode objects |
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.
Better to lift this out into a "@no_wasm_backend" perhaps?
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.
Sorry... maybe better to skip here if get_setting('WASM_OBJECT_FILES') == 0
(which is true for the LTO mode of testing).
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.
Thanks, changed to that.
The AUTODEBUG option previously only worked on bitcode files. It parsed them as text, adding instrumentation to log out operations for debug purposes, during the link step. But this doesn't work with wasm object files. This PR adds such support, in a different way - it runs the binaryen instrumentation passes. This has been incredibly useful in getting wasm2js working.
This reverts commit 57395a9.
This reverts commit 57395a9.
The AUTODEBUG option previously only worked on bitcode files. It parsed them as text, adding instrumentation to log out operations for debug purposes, during the link step. But this doesn't work with wasm object files. This PR adds such support, in a different way - it runs the binaryen instrumentation passes. This has been incredibly useful in getting wasm2js working.
The AUTODEBUG option previously only worked on bitcode files. It parsed them as text, adding instrumentation to log out operations for debug purposes, during the link step. But this doesn't work with wasm object files. This PR adds such support, in a different way - it runs the binaryen instrumentation passes.
This has been incredibly useful in getting wasm2js working.
Usage:
EMCC_AUTODEBUG=1 emcc [..]