Skip to content

Conversation

kripken
Copy link
Member

@kripken kripken commented Apr 10, 2024

The VM limitation might be an OOM (which can change due to opts) or an
atomic wait (which can hang on proper VMs with support). We already
avoided running VMs on the optimized wasm in this case, but we still
ran them on the original wasm, which this changes, mainly to avoid that
atomic wait situation.

@kripken kripken requested a review from tlively April 10, 2024 18:53
Comment on lines -954 to -961
# if the binaryen interpreter hit a host limitation on the original
# testcase, or for some other reason we need to ignore this, then stop
# (otherwise, a host limitation on say allocations may be hit in the
# 'before' but not in the 'after' as optimizations may remove it).
if before[self.bynterpreter] == IGNORE:
# the ignoring should have been noted during run_vms()
assert(ignored_vm_runs > ignored_before)
return
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this logic as well in case the reason for the host limitation is optimized out?

Copy link
Member Author

Choose a reason for hiding this comment

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

This function calls run_vms twice, which this PR fixes. It also calls compare_before_and_after, which are passed the results we do have (which will be only the binaryen interpreter, so that should be ok).

@kripken kripken merged commit 0481320 into WebAssembly:main Apr 10, 2024
@kripken kripken deleted the ignore.host branch April 10, 2024 21:51
@gkdn gkdn mentioned this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants