-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[EH] Use LLVM implementation for new Wasm EH #23469
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
Changes from all commits
02891fb
2dcb7fa
d165f45
f90efe5
05b1c02
6d05b70
1ded2da
d05ec5d
2f47aff
76c7cd8
842ee68
bfeb718
21c2adf
04faed4
91204e5
06f9a9a
42ffb38
0d3119c
545402c
aa4a56d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
144652 | ||
144531 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -431,10 +431,6 @@ def check_human_readable_list(items): | |
extras = settings.BINARYEN_EXTRA_PASSES.split(',') | ||
passes += [('--' + p) if p[0] != '-' else p for p in extras if p] | ||
|
||
# Run the translator to the standardized EH instructions. | ||
if not settings.WASM_LEGACY_EXCEPTIONS: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if it would be worthwhile to keep some way to activate the translator path for a while. Just in case, e.g. someone finds a bug, they can try the other pathway? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what would be a good option for enabling this... Adding one more possible value to Also I don't have any idea how widely this feature has been adopted in the first place. I haven't received a single bug reports about this feature over the last year... So I guess this is at least not widely used, if ever used anywhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'm sure it's the case that nobody has been using it. I think Firefox is the only runtime that supports it so far, so nobody would really have a good reason to, when everything supports legacy exceptions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it's only useful for us Emscripten developers and not much for users. Having an internal option for testing for us sounds fine, but I'm a little hesitant because we already have a messy soup of options for EH/SjLj.. 🥲 Anyway can that be a followup? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, come to think of it, I don't think it will be very useful after all... We, or more likely I, will be the only one who will run that option (for debugging, in case llvm implementation has bugs or something), and I still can do that with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah that's a good point that if it's a one-command translation that would be easy enough. Is the wasm/JS interface 100% compatible? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (even if not, if we decide to do it, a followup is fine) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah I don't see why not. I checked with a simple test case, replacing only the wasm with the translated binary, and it seems to work. |
||
passes += ['--emit-exnref'] | ||
|
||
# If we are going to run metadce then that means we will be running binaryen | ||
# tools after the main invocation, whose flags are determined here | ||
# (specifically we will run metadce and possibly also wasm-opt for import/ | ||
|
Uh oh!
There was an error while loading. Please reload this page.