Wasmtime GitHub notifications bot (Sep 01 2021 at 16:05): Wasmtime GitHub notifications bot (Sep 01 2021 at 16:05):bnjbvr opened PR #3274 from fix-m1 to main:
Three commits that make the Mac M1 pass all the tests run in CI:
- in the custom asm code for the fibers, on aarch64, the custom CFI directive that was indicating where FP (x29) lives was completely incorrect. The pushes in wasmtime_fiber_switch start with
stp lr, fp, [sp, -16]!, so FP was at CFA - 0x8, not CFA - 0x60. Unclear why it did work in the first place, and why it stopped working at some point; likely that something in Apple's libunwind has changed in the meanwhile.- the OS page size is 16K, not 4K on Mac M1: a constant is used for the code section page's size, and defined per target, and reused in a few places in the code base. Thanks to @alexcrichton for indicating me the place to look at!
- unfortunately we need to disable pointer authentication (until we implement it!) for function frames that don't have a prologue too. Preferred to split
gen_prologue_frame_setup, so there's a new functiongen_debug_frame_infothat's defaulting to doing nothing, and implemented on aarch64 and disabling pointer auth. This function is called independently from the fact that the function requires a prologue/epilogue, so we can put the unwind instruction that will generate the CFI directive in there. cc @cfallin @akirilov-arm for this changeFixes https://github.com/bytecodealliance/wasmtime/issues/3256.
Wasmtime GitHub notifications bot (Sep 01 2021 at 16:47):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2021 at 16:47):bjorn3 created PR review comment:
At least on macOS you have to query the page size at runtime. This is for example necessary for Rosetta 2 to work as it uses AArch64 sized pages. There may be other cases where it isn't possible to know this at compile time. You can use
region::page::size()to query it.
Wasmtime GitHub notifications bot (Sep 01 2021 at 16:49):bnjbvr submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2021 at 16:49):bnjbvr created PR review comment:
TIL! @alexcrichton in this case, would it be just simpler to call
region::page::size()(maybe cache it, if we expect this to be expensive) instead of having the raw constants scattered in the code?
Wasmtime GitHub notifications bot (Sep 01 2021 at 17:19):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2021 at 17:19):alexcrichton created PR review comment:
I think a better fix for this would be to inspect
self.isa.target()to accomodate cross-compiling use cases. If the section is over-aligned for some embeddings that's not an issue.
Wasmtime GitHub notifications bot (Sep 02 2021 at 07:10):bnjbvr updated PR #3274 from fix-m1 to main.
Wasmtime GitHub notifications bot (Sep 02 2021 at 07:11):bnjbvr submitted PR review.
Wasmtime GitHub notifications bot (Sep 02 2021 at 07:11):bnjbvr created PR review comment:
Good point, fixed!
Wasmtime GitHub notifications bot (Sep 02 2021 at 07:16):bnjbvr updated PR #3274 from fix-m1 to main.
Wasmtime GitHub notifications bot (Sep 02 2021 at 14:02):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 02 2021 at 14:06):bnjbvr updated PR #3274 from fix-m1 to main.
Wasmtime GitHub notifications bot (Sep 02 2021 at 14:57):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Sep 02 2021 at 14:57):bjorn3 created PR review comment:
Maybe just bump it to a newer version instead of removing it completely?
Wasmtime GitHub notifications bot (Sep 02 2021 at 15:03):bnjbvr submitted PR review.
Wasmtime GitHub notifications bot (Sep 02 2021 at 15:03):bnjbvr created PR review comment:
I was under the impression that Firefox is now following latest stable, so that cargo check step ought to be unnecessary. Plus, unclear whether Cranelift is still being updated from this particular repository or Mozilla's fork. I'll check in with the Mozilla folks, it'd be easy to add back later, if necessary.
Wasmtime GitHub notifications bot (Sep 02 2021 at 15:36):bnjbvr submitted PR review.
Wasmtime GitHub notifications bot (Sep 02 2021 at 15:36):bnjbvr created PR review comment:
Yury from Mozilla confirmed on Matrix mozilla-central is using stable, so we should be good here!
Wasmtime GitHub notifications bot (Sep 02 2021 at 15:40):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Sep 02 2021 at 15:40):bjorn3 created PR review comment:
Aren't they updating like one week after a new rust version releases?
Wasmtime GitHub notifications bot (Sep 02 2021 at 16:48):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Sep 02 2021 at 16:48):bnjbvr submitted PR review.
Wasmtime GitHub notifications bot (Sep 02 2021 at 16:48):bnjbvr created PR review comment:
It might be, but Cranelift in Firefox seems to receive updates at a lesser frequency (which makes sense for their stability guarantees, and considering the Cranelift bug has been resolved as inactive). Last update was 3 months ago.
Wasmtime GitHub notifications bot (Sep 02 2021 at 16:49):cfallin merged PR #3274.
Last updated: Dec 23 2024 at 12:05 UTC