Stream: git-wasmtime

Topic: wasmtime / Issue #2103 cranelift: arm32 codegen


view this post on Zulip Wasmtime GitHub notifications bot (Aug 06 2020 at 10:22):

github-actions[bot] commented on Issue #2103:

Subscribe to Label Action

cc @bnjbvr

<details>
This issue or pull request has been labeled: "cranelift"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Aug 06 2020 at 16:17):

cfallin commented on Issue #2103:

Wow -- this is an excellent surprise to start my day with. First of all, thanks, @jmkrauz, for putting in all of this work!

I'll review this in the next few days; I expect it will take a bit of time. One high-level comment regarding the register-overlap issue: we've thought about (and need to solve) it for other architectures too, e.g. x86-32, so I expect we'll have a better answer eventually. For now, a stopgap solution (for correctness) might be to simply allocate pairs as the fundamental unit (i.e. D regs), and just burn an extra 32-bit register for each f32, and punt on 128-bit vector support for now. Not ideal but it will get us to basic completeness at least. @julian-seward1 might have more thoughts on this?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 21 2020 at 16:38):

cfallin commented on Issue #2103:

@jmkrauz, I just wanted to comment here to say first of all, we haven't forgotten this -- we're working on some framework improvements that should make this backend a little bit nicer. Specifically:

I also don't want to hold this in limbo forever; at some point (@julian-seward1 and @bnjbvr may have opinions as well) we can consider merging and gating behind an experimental feature flag until we get this up to Wasm-MVP (i32/i64/f32/f64 support) level.

Thanks again and hopefully this will come together sooner rather than later :-)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2020 at 06:36):

jmkrauz commented on Issue #2103:

That's good news :)
I will work on this after the weekend.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 24 2020 at 08:50):

bnjbvr commented on Issue #2103:

Agreed that we could review and merge this behind a flag, or just letting know that support for arm32 is experimental; we've done this for other backends, and we can work incrementally on supporting more opcodes etc.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2020 at 00:43):

cfallin commented on Issue #2103:

Hi @jmkrauz, it looks like you've refactored to use the common abi_impl machinery (thanks!). I discussed this PR briefly today with a few others and we agree that it might be best to merge now, given that I haven't resolved the multiple-register-value question yet (which this would need for i64 values). (Sorry about that -- a lot on my plate lately!) If that's alright with you, of course; does this seem OK or do you have plans to refine the PR more first?

One thing I think we should do, however, is to put the new backend behind a feature flag until it implements Wasm MVP; otherwise someone might try to build wasmtime on arm32 and hit confusing panics. Perhaps just #[cfg(feature = "arm32")] on the main mod arm32 statement in cranelift/codegen/src/isa/mod.rs?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2020 at 11:17):

jmkrauz commented on Issue #2103:

Merging now is absolutely fine with me.
I have added additional commit in which I put ARM backend behind experimental_arm32 feature flag. In order to avoid failing arm32 filetests with experimental_arm32 disabled, I have also added small check in cranelift/filetests/src/runone.rs.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2020 at 16:07):

cfallin commented on Issue #2103:

Excellent, thanks! Looking forward to seeing this become a full backend!


Last updated: Dec 23 2024 at 12:05 UTC