Stream: git-wasmtime

Topic: wasmtime / issue #3009 [RFC] Remove the old x86 backend


view this post on Zulip Wasmtime GitHub notifications bot (Jun 21 2021 at 11:24):

bjorn3 commented on issue #3009:

The default was changed only two and a half months ago (https://github.com/bytecodealliance/wasmtime/pull/2718). It felt much longer.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 21 2021 at 15:50):

cfallin commented on issue #3009:

Thanks so much for tackling this! It has been on my to-do list for a while, but at low priority relative to a bunch of other things, so I'm happy that you had time to pick it up :-)

I think we should probably write up a proper RFC in bytecodealliance/rfcs, just to ensure we have the right folks onboard and there are no remaining surprise uses of the backend. I'll draft something soon and then we can start discussion.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 28 2021 at 20:32):

cfallin commented on issue #3009:

@bjorn3 now that the RFC is merged, would you be interested in bringing this PR up-to-date? If not, no worries, someone else can pick up the effort, but please do feel free if you're up for it, and I'm happy to review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 28 2021 at 20:47):

bjorn3 commented on issue #3009:

I will try to update it tomorrow.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2021 at 16:26):

bjorn3 commented on issue #3009:

@cfallin Rebased

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2021 at 16:42):

cfallin commented on issue #3009:

@bjorn3 there's a build failure that seems to arise from some missing DSL bits -- perhaps cut just a bit too much?

(As an aside, I'm very happy to see the -40k LoC stat; that's a lot of code we won't have to maintain!)

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2021 at 16:43):

bjorn3 commented on issue #3009:

I forgot to remove two tests it seems. They were testing parts I cut out.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2021 at 16:46):

bjorn3 commented on issue #3009:

(As an aside, I'm very happy to see the -40k LoC stat; that's a lot of code we won't have to maintain!)

~16k of deletions is tests for the old x86 backend or the old backend infrastructure. ~24k deletions are for cranelift-codegen.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2021 at 17:18):

bjorn3 commented on issue #3009:

FAIL filetests/filetests/runtests/extend.clif: run

Caused by:
    0: Cranelift codegen error
    1: Unsupported feature: Isplit: Unsupported type: types::I64

How did this test work on AArch64 in the first place? isplit from i64 -> i32, i32 shouldn't have been implemented on any new backend at all.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2021 at 17:18):

bjorn3 edited a comment on issue #3009:

FAIL filetests/filetests/runtests/extend.clif: run

Caused by:
    0: Cranelift codegen error
    1: Unsupported feature: Isplit: Unsupported type: types::I64

How did this test work on AArch64 in the first place? isplit from i64 -> i32, i32 shouldn't have been implemented on any new backend at all.

Should I remove it?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2021 at 17:30):

cfallin commented on issue #3009:

```
FAIL filetests/filetests/runtests/extend.clif: run

Caused by:
0: Cranelift codegen error
1: Unsupported feature: Isplit: Unsupported type: types::I64
```

How did this test work on AArch64 in the first place? isplit from i64 -> i32, i32 shouldn't have been implemented on any new backend at all.

Should I remove it?

Not sure, but for that particular test and its twin below, we could probably rewrite it without the isplit (maybe this is what you mean by "remove it") -- just a single 64-bit comparison rather than a split/two 32-bit compares/boolean-and.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2021 at 17:48):

bjorn3 commented on issue #3009:

Rewrote it to remove the usage of isplit.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2021 at 11:23):

bjorn3 commented on issue #3009:

CI passes

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2021 at 16:20):

cfallin commented on issue #3009:

Everything looks good -- well, here goes something! The old backend served us well, and thanks to those who worked on it; onward...

view this post on Zulip Wasmtime GitHub notifications bot (Nov 12 2021 at 11:44):

dvc94ch commented on issue #3009:

I guess x86/riscv support is canned for the moment? as in there aren't enough users for these targets?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 12 2021 at 12:25):

bjorn3 commented on issue #3009:

x86 is not a priority AFAIK, but it would be nice to have. Tracking issue: https://github.com/bytecodealliance/wasmtime/issues/1980

Riscv support has never been usable AFAICT. I believe it got prototyped with the original creation of Cranelift, but has never seen any improvements afterwards. Tracking issue: https://github.com/bytecodealliance/wasmtime/issues/2217

Nobody is currently working on either target to the best of my knowledge, but I am sure PR's for either will be accepted. @cfallin estimated that it will be 2-3 months of fulltime work to get to a reasonable state in https://github.com/bytecodealliance/wasmtime/issues/2217#issuecomment-961197939. If you do want to implement a backend, be aware of https://github.com/bytecodealliance/rfcs/pull/15.


Last updated: Nov 22 2024 at 16:03 UTC