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

For store instructions, always load first. #1731

Merged
merged 1 commit into from
Oct 17, 2020

Conversation

nlewycky
Copy link
Contributor

Sometimes the store may be partially-successful before trapping, for instance if it is partway in valid memory and partway into the guard page.

Use a load instruction before the store to ensure that all the memory is addressable. The loaded value is discarded.

NB. We don't apply this to atomics. It's not clear whether atomic stores can be half-committed.

Fixes align.wast and memory_trap.wast on aarch64.

…instance if it is partways in valid memory and partways into the guard page.

Use a load instruction before the store to ensure that all the memory is addressable. The loaded value is discarded.

NB. We don't apply this to atomics. It's not clear whether atomic stores can be half-committed.

Fixes align.wast and memory_trap.wast on aarch64.
Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

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

Shall we re-enable this tests on aarch64?

@nlewycky
Copy link
Contributor Author

nlewycky commented Oct 17, 2020

Shall we re-enable this tests on aarch64?

They aren't disabled. They're just failing.

The cranelift ones are disabled, but this doesn't fix those. For LLVM we simply never turned on testing on arm yet.

@nlewycky
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Oct 17, 2020
1731: For store instructions, always load first. r=nlewycky a=nlewycky

Sometimes the store may be partially-successful before trapping, for instance if it is partway in valid memory and partway into the guard page.

Use a load instruction before the store to ensure that all the memory is addressable. The loaded value is discarded.

NB. We don't apply this to atomics. It's not clear whether atomic stores can be half-committed.

Fixes align.wast and memory_trap.wast on aarch64.


1732: Make wasi tests use the engine we're testing instead of always engine-jit. r=nlewycky a=nlewycky



Co-authored-by: Nick Lewycky <nick@wasmer.io>
@bors
Copy link
Contributor

bors bot commented Oct 17, 2020

Build failed (retrying...):

@bors
Copy link
Contributor

bors bot commented Oct 17, 2020

@bors bors bot merged commit f4f6a81 into master Oct 17, 2020
@bors bors bot deleted the feature/llvm-load-before-store branch October 17, 2020 02:49
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