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

Flush icache on Android aarch64 too (and fix Android build) #5331

Merged
merged 1 commit into from
Nov 30, 2022

Conversation

bnjbvr
Copy link
Member

@bnjbvr bnjbvr commented Nov 28, 2022

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 membarrier function.

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

@bnjbvr bnjbvr changed the title Flush icache on Android aarch64 too Flush icache on Android aarch64 too (and fix Android build) Nov 28, 2022
Copy link
Contributor

@afonso360 afonso360 left a comment

Choose a reason for hiding this comment

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

LGTM! And thanks for the cleanup!

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Thanks -- adding Android makes sense; one comment on the refactor below.

crates/jit-icache-coherence/src/libc.rs Show resolved Hide resolved
@bnjbvr bnjbvr requested a review from cfallin November 30, 2022 15:15
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

LGTM!

@cfallin cfallin merged commit 2bb1fb0 into main Nov 30, 2022
@cfallin cfallin deleted the flush-icache-on-android-aarch64 branch November 30, 2022 15:15
@bnjbvr
Copy link
Member Author

bnjbvr commented Dec 1, 2022

Thanks for merging @cfallin ! Could we have a dot release 3.0.1 that would include this, thus fixing the Android build?

@alexcrichton
Copy link
Member

A 3.0.1 release seems reasonable to me, to kick that off would you be up for sending a PR to the release-3.0.0 branch?

@bnjbvr
Copy link
Member Author

bnjbvr commented Dec 1, 2022

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
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 #5323 and #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 9, 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 #5323 and #5331 for context.
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.

None yet

4 participants