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

Finish implementing x86 32-bits #1089

Closed
htfy96 opened this issue May 16, 2019 · 7 comments
Closed

Finish implementing x86 32-bits #1089

htfy96 opened this issue May 16, 2019 · 7 comments
Labels
cranelift Issues related to the Cranelift code generator

Comments

@htfy96
Copy link

htfy96 commented May 16, 2019

Sources

sources.zip

cb.wasm was compiled with Emscripten 1.18.31 + stock LLVM with:

EMCC_WASM_BACKEND=1 emcc -s WASM=1 -s TOTAL_MEMORY=2147418112 -s ERROR_ON_UNDEFINED_SYMBOLS=0 -s EXPORTED_FUNCTIONS=['_double_plus_cnt_plus_pid','_square','_large_struct_op'] -fPIC -DPIC -g3 -O2 -I test/lib ../test/lib/libcb.c -o cb.wasm

Behavior

Compiling this wasm with clif-util wasm --target i686 ~/code/git/wasm-sandboxing/build/cb.wasm with 42bd100 gives out verifier error: Store must have an encoding. Log is attached here

@sunfishcode
Copy link
Member

Support for i686 (32-bit x86) is currently incomplete. A lot of the major pieces are in place, since we tend to implement the 32-bit x86 parts along with the 64-bit x86 parts, however it hasn't been tested yet, and there are a few more 32-bit-specific pieces needed.

@bnjbvr bnjbvr changed the title verifier error: Store must have an encoding for i686 targets when compiling wasm Finish implementing x86 32-bits May 21, 2019
ryzokuken referenced this issue in ryzokuken/cranelift Aug 31, 2019
Add encodings for iadd carry variants (iadd_cout, iadd_cin, iadd_carry)
for x86_32, enabling the legalization for iadd.i64 to work.

Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1576675
Bug: https://github.com/CraneStation/cranelift/issues/765
ryzokuken referenced this issue in ryzokuken/cranelift Aug 31, 2019
Add encodings for isub borrow variants (isub_bout, isub_bin,
isub_borrow) for x86_32, enabling the legalization for isub.i64 to work.

Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1576675
Bug: https://github.com/CraneStation/cranelift/issues/765
ryzokuken referenced this issue in ryzokuken/cranelift Sep 4, 2019
Add encodings for isub borrow variants (isub_bout, isub_bin,
isub_borrow) for x86_32, enabling the legalization for isub.i64 to work.

Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1576675
Bug: https://github.com/CraneStation/cranelift/issues/765
ryzokuken referenced this issue in ryzokuken/cranelift Sep 5, 2019
Add encodings for isub borrow variants (isub_bout, isub_bin,
isub_borrow) for x86_32, enabling the legalization for isub.i64 to work.

Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1576675
Bug: https://github.com/CraneStation/cranelift/issues/765
ryzokuken referenced this issue in ryzokuken/cranelift Sep 5, 2019
Add encodings for isub borrow variants (isub_bout, isub_bin,
isub_borrow) for x86_32, enabling the legalization for isub.i64 to work.

Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1576675
Bug: https://github.com/CraneStation/cranelift/issues/765
bnjbvr referenced this issue in bytecodealliance/cranelift Sep 5, 2019
Add encodings for isub borrow variants (isub_bout, isub_bin,
isub_borrow) for x86_32, enabling the legalization for isub.i64 to work.

Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1576675
Bug: https://github.com/CraneStation/cranelift/issues/765
@alexcrichton alexcrichton transferred this issue from bytecodealliance/cranelift Feb 28, 2020
@alexcrichton alexcrichton added the cranelift Issues related to the Cranelift code generator label Feb 28, 2020
@whitequark
Copy link
Contributor

whitequark commented Apr 27, 2020

I'm very interested in seeing this issue fixed, since it has a significant impact on the usefulness of wasmtime-py; if you navigate to python.org on Windows, either 32-bit or 64-bit, it always suggests you a 32-bit Python build, so most people certainly end up with 32-bit Python that cannot run wasmtime.pyd. Not only that, but the link to the 64-bit installer is somewhat hard to find, and there AFAIK isn't a redirect I can send to people to ask them to install 64-bit Python that is compatible with wasmtime-py.

Could one of the developers more familiar with the codebase provide some guidance? I've tried running a few WASI applications with wasmtime targeting i686 Linux but they all crash with a constraint issue that only ever mentions x86_64 registers (rax, etc) so I'm not sure if I'm doing something seriously wrong, see below.

                                       block0(v0: i32 [ss0], v1: i32 [ss1], v94: i32 [%rbp], v95: i32 [%rdi]):
       [Op1ld#8b,%rax]                     v90 = load.i32 notrap aligned v0
       ;~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       ; error: inst112: Op1ld#8b constraints not satisfied in: v90 = load.i32 notrap aligned v0
       function u0:15(i32 vmctx [0], i32 [4], i32 fp [%rbp], i32 csr [%rdi]) -> i32 fp [%rbp], i32 csr [%rdi] system_v {

(I do understand that a Windows port is more complex--I've heard that SEH is going to be involved--but even an i686 Linux port would be useful, both on its own and as a starting point.)

@tschneidereit
Copy link
Member

Thank you for bringing this up, and for starting to work on it!

I at least wasn't aware of the unfortunate situation with 64-bit Python on Windows, which I agree makes this more important.

One thing to be aware of is that we're in the middle of a pretty big change to Cranelift, involving a rewrite of the instruction selector (and register allocator, but I don't think that's too important here.) I'm not sure how that factors in, but @cfallin, @bnjbvr, or @julian-seward1 would.

@bnjbvr
Copy link
Member

bnjbvr commented Apr 28, 2020

Hi @whitequark and thanks for working on it!

As Till pointed out, we're in the middle of switching the instruction selection and codegen backends in Cranelift. As a matter of fact, we're rewriting the codegen bits, which include recipes/encodings and all of this, to a simpler and more straightforward system (see for example the new aarch64 directory under codegen/src/isa).

At this point, the current meta system should be considered in maintenance-only mode. Note that since the two codegen backend frameworks are very different, we won't be able to port x86 32 bits changes to the old backend onto the new backend, so this work will likely be thrown away.

Since there might be some time until we get to the final switch to the new system (and plain removal of the previous one), I don't think we should get in the way of good will. Meaning that if people want to submit patches, and other people want to review them, this is totally fine; this is useful up to the point where we switch to the new system.

Note there is a work-in-progress PR for a new x64 backend. This code could be a great base for a new x86 32-bits backend too, duplicating code for a new codegen backend, or sharing and reusing most of the x64 code there.

@whitequark
Copy link
Contributor

Ah, I see. In that case I would probably not put any effort towards a Windows port until the x64 backend is ready.

@bjorn3
Copy link
Contributor

bjorn3 commented Oct 1, 2021

I think this should be closed in favor of #1980.

@cfallin
Copy link
Member

cfallin commented Oct 1, 2021

Sure, they both seem to be tracking the same thing; will close this one.

@cfallin cfallin closed this as completed Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

No branches or pull requests

8 participants