Flush icache on Android aarch64 too (and fix Android build)#5331
Merged
Conversation
26 tasks
bnjbvr
changed the title
afonso360
approved these changes
Nov 28, 2022
Contributor
afonso360
left a comment
afonso360
left a comment
There was a problem hiding this comment.
LGTM! And thanks for the cleanup!
cfallin
reviewed
Nov 28, 2022
Member
cfallin
left a comment
cfallin
left a comment
There was a problem hiding this comment.
Thanks -- adding Android makes sense; one comment on the refactor below.
Member
Author
|
Thanks for merging @cfallin ! Could we have a dot release 3.0.1 that would include this, thus fixing the Android build? |
Member
|
A 3.0.1 release seems reasonable to me, to kick that off would you be up for sending a PR to the |
Member
Author
|
Sure, done in #5362 ! |
afonso360
added a commit
to afonso360/wasmtime
that referenced
this pull request
Jan 1, 2023
We accidentally broke the build for FreeBSD when introducing the jit-icache-coherence crate. To avoid this happening again, add a check job just to ensure that it can build. See bytecodealliance#5323 and bytecodealliance#5331 for context.
afonso360
added a commit
to afonso360/wasmtime
that referenced
this pull request
Jan 1, 2023
We accidentally broke the build for FreeBSD when introducing the jit-icache-coherence crate. To avoid this happening again, add a check job just to ensure that it can build. See bytecodealliance#5323 and bytecodealliance#5331 for context.
afonso360
added a commit
to afonso360/wasmtime
that referenced
this pull request
Jan 1, 2023
We accidentally broke the build for FreeBSD when introducing the jit-icache-coherence crate. To avoid this happening again, add a check job just to ensure that it can build. See bytecodealliance#5323 and bytecodealliance#5331 for context.
afonso360
added a commit
to afonso360/wasmtime
that referenced
this pull request
Jan 1, 2023
We accidentally broke the build for Android when introducing the jit-icache-coherence crate. To avoid this happening again, add a check job just to ensure that it can build. See bytecodealliance#5323 and bytecodealliance#5331 for context.
afonso360
added a commit
to afonso360/wasmtime
that referenced
this pull request
Jan 3, 2023
We accidentally broke the build for FreeBSD when introducing the jit-icache-coherence crate. To avoid this happening again, add a check job just to ensure that it can build. See bytecodealliance#5323 and bytecodealliance#5331 for context.
afonso360
added a commit
to afonso360/wasmtime
that referenced
this pull request
Jan 3, 2023
We accidentally broke the build for FreeBSD when introducing the jit-icache-coherence crate. To avoid this happening again, add a check job just to ensure that it can build. See bytecodealliance#5323 and bytecodealliance#5331 for context.
afonso360
added a commit
to afonso360/wasmtime
that referenced
this pull request
Jan 3, 2023
We accidentally broke the build for Android when introducing the jit-icache-coherence crate. To avoid this happening again, add a check job just to ensure that it can build. See bytecodealliance#5323 and bytecodealliance#5331 for context.
afonso360
added a commit
to afonso360/wasmtime
that referenced
this pull request
Jan 3, 2023
We accidentally broke the build for Android when introducing the jit-icache-coherence crate. To avoid this happening again, add a check job just to ensure that it can build. See bytecodealliance#5323 and bytecodealliance#5331 for context.
afonso360
added a commit
to afonso360/wasmtime
that referenced
this pull request
Jan 3, 2023
We accidentally broke the build for Android when introducing the jit-icache-coherence crate. To avoid this happening again, add a check job just to ensure that it can build. See bytecodealliance#5323 and bytecodealliance#5331 for context.
alexcrichton
pushed a commit
that referenced
this pull request
Jan 3, 2023
afonso360
added a commit
to afonso360/wasmtime
that referenced
this pull request
Jan 3, 2023
We accidentally broke the build for Android when introducing the jit-icache-coherence crate. To avoid this happening again, add a check job just to ensure that it can build. See bytecodealliance#5323 and bytecodealliance#5331 for context.
alexcrichton
pushed a commit
that referenced
this pull request
Jan 9, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This updates the icache flushing code so it does the flushing on Android as well. From fuzzy memories working on this in the past and quick inspection of what Firefox does these days, this is required for both Android and linux, and Android is guaranteed to have access to the
membarrierfunction.I've also slightly refactored the code to use an internal module named
details, I find it slightly cleaner as it avoids all the dead code in other combinations of targets/archs, but it's mostly esthetic, so I could revert that part.In addition to correctness, this also fixes the build of wasmtime 3.0.0 on the android architecture, which we rely on. If that fix is deemed acceptable, it would be extra nice to get a dot release of wasmtime with that build fix too!
cc @afonso360 @akirilov-arm @alexcrichton @cfallin