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

Support x86-32 in new instruction selection framework #1980

Open
tschneidereit opened this issue Jul 5, 2020 · 13 comments
Open

Support x86-32 in new instruction selection framework #1980

tschneidereit opened this issue Jul 5, 2020 · 13 comments
Labels
cranelift:area:x86 Issues related to x86 codegen cranelift:new-target Issues requesting Cranelift support for new targets. cranelift Issues related to the Cranelift code generator enhancement help wanted wasmtime:platform-support Related to supporting a new platform in Wasmtime

Comments

@tschneidereit
Copy link
Member

We don't currently have fully working x86 support, but we have a backend in-tree that's largely completely. However, it uses the old instructions selection framework, and as described in #1936, Cranelift is in the process of switching to a new one. While the old framework won't be removed until quite a bit later, that means that not much work will go into it anymore, and that any investments into backends based on it will become obsolete.

We should investigate the best approach to supporting x86 in the new framework.

There's active work going on to implement x64 support in the new framework, but no current effort around x86. In #1789 @whitequark mentions that she'd be happy to contribute to x86 support in the new framework, but only if the general support for targeting x86 exists.

@bnjbvr @julian-seward1, can you comment on what the best approach to take here would be? I understand that this isn't something that's high on your priority list, so as a first step we should just get a clearer picture of what this could look like, and the effort required.

@bnjbvr
Copy link
Member

bnjbvr commented Jul 8, 2020

Thanks for opening an issue!

Sharing code

The current encoding machinery is sufficient to support all the x86 32 and 64 bits instructions up to SSE4.2 (i.e., nothing has been done for VEX yet). Most encodings can be reused on 32 bits (of course, without the 64-bit mode); some specific encodings and addressing modes are not available in 32-bits mode (e.g. RIP-relative addressing).

Ideally, most of the encoding code could be shared between the two architectures, with an Inst enum for instructions that are common to both architectures; each of x86_64 and x86-32bits would then have their own Inst enum as well, with a particular Shared variant that would be dynamically dispatch most queries / calls to the shared Inst enum. It would mean that the 64-bits and 32-bits backends would have special code to handle the shared encodings (e.g. when printing out the instructions and registers, when handling supported operand sizes,...) so it might become a bit messy. Moreover, the code for these two architectures would be scattered in more places. Spidermonkey has tried to enforce code sharing between these two architectures, and it's made the code difficult to follow sometimes (it tends to force low-level abstractions that add unnecessary layers to the code). But we wouldn't need to duplicate work.

Alternatively, we could just duplicate all the Inst variants, so that each target has precisely what it can do. Even if this implies some code duplication, this should stay reasonably well-contained over time: code emitting encodings tends to remain stable over time, once it's been written (and maybe fixed once or twice).

Since this encoding code is still in the process of being written, it might be good to wait for stabilization before jumping into implementing a new target derived from it.

Legalizing IR

At this point, there's not a good story for transforming sequences of operations involving types that are not natively supported by the platform: be it i128 or aarch64/x64, or i64 on on x86 32 bits. We'd probably need something like lightweight legalizations to make this maintainable over time.

ABIs

32-bits ABIs would be one more work item to do, since the ABIs are quite different. This isn't quite complicated, but it requires careful testing to be sure not to make an error there.

In a nutshell, there's nothing that's too complicated. I'm not quite sure of the best way to share code yet.

@guest271314
Copy link
Contributor

Error: Sorry! Wasmtime currently only provides pre-built binaries for x86_64 architectures.

@guest271314
Copy link
Contributor

It would be helpful and informative if the README.md at https://github.com/bytecodealliance/wasmtime including an unambiguous and conspicuous header and language that states x86 32 bit architecture is not (currently) supported, so that users do not run

$ curl https://wasmtime.dev/install.sh -sSf | bash

or try https://wasi.dev/polyfill/ using instructions at https://github.com/bytecodealliance/wasmtime/blob/main/docs/WASI-tutorial.md expecting the code to work.

@bnjbvr bnjbvr added the cranelift Issues related to the Cranelift code generator label Sep 24, 2020
@github-actions
Copy link

Subscribe to Label Action

cc @bnjbvr

This issue or pull request has been labeled: "cranelift"

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

  • bnjbvr: cranelift

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

Learn more.

@bjorn3
Copy link
Contributor

bjorn3 commented Mar 20, 2021

I think this can be closed.

@whitequark
Copy link
Contributor

How can I test 32-bit x86 support? --target i686-unknown-linux-gnu doesn't seem to work for example:

thread 'main' panicked at 'host machine is not a supported target: "unsupported architecture"', crates/jit/src/native.rs:6:33

@bjorn3
Copy link
Contributor

bjorn3 commented Mar 20, 2021

The new x64 backend doesn't have 32bit support yet.

@whitequark
Copy link
Contributor

That's exactly what this issue is about, no?

@bjorn3
Copy link
Contributor

bjorn3 commented Mar 20, 2021

Oops, interpreted x86 as x86_64 instead of 32bit x86.

@cfallin cfallin changed the title Support x86 in new instruction selection framework Support x86-32 in new instruction selection framework May 4, 2022
@cfallin cfallin added the cranelift:new-target Issues requesting Cranelift support for new targets. label May 4, 2022
@alexcrichton alexcrichton added the wasmtime:platform-support Related to supporting a new platform in Wasmtime label Jul 12, 2022
cfallin added a commit to cfallin/wasmtime that referenced this issue Nov 8, 2022
I was recently pointed to fastly/Viceroy#200 where it seems some folks
are trying to compile Wasmtime (via Viceroy) for Windows x86-32 and the
failures may not be loud enough. I've tried to reproduce this
cross-compiling to i686-pc-windows-gnu from Linux and hit build failures
(as expected) in several places.  Nevertheless, while trying to discern
what others may be attempting, I noticed some dead x86-32-specific code
in our repo, and figured it would be a good idea to clean this up.
Otherwise, it (i) sends some mixed messages -- "hey look, this codebase
does support x86-32" -- and (ii) keeps untested code around, which is
generally not great.

This PR removes x86-32-specific cases in traphandlers and unwind code,
and Cranelift's native feature detection. It adds helpful compile-error
messages in a few cases. If we ever support x86-32 (contributors
welcome! The big missing piece is Cranelift support; see bytecodealliance#1980), these
compile errors and git history should be enough to recover any knowledge
we are now encoding in the source.

I left the x86-32 support in `wasmtime-fiber` alone because that seems
like a bit of a special case -- foundation library, separate from the
rest of Wasmtime, with specific care to provide a (presumably working)
full 32-bit version.
cfallin added a commit to cfallin/wasmtime that referenced this issue Nov 8, 2022
I was recently pointed to fastly/Viceroy#200 where it seems some folks
are trying to compile Wasmtime (via Viceroy) for Windows x86-32 and the
failures may not be loud enough. I've tried to reproduce this
cross-compiling to i686-pc-windows-gnu from Linux and hit build failures
(as expected) in several places.  Nevertheless, while trying to discern
what others may be attempting, I noticed some dead x86-32-specific code
in our repo, and figured it would be a good idea to clean this up.
Otherwise, it (i) sends some mixed messages -- "hey look, this codebase
does support x86-32" -- and (ii) keeps untested code around, which is
generally not great.

This PR removes x86-32-specific cases in traphandlers and unwind code,
and Cranelift's native feature detection. It adds helpful compile-error
messages in a few cases. If we ever support x86-32 (contributors
welcome! The big missing piece is Cranelift support; see bytecodealliance#1980), these
compile errors and git history should be enough to recover any knowledge
we are now encoding in the source.

I left the x86-32 support in `wasmtime-fiber` alone because that seems
like a bit of a special case -- foundation library, separate from the
rest of Wasmtime, with specific care to provide a (presumably working)
full 32-bit version.
cfallin added a commit that referenced this issue Nov 8, 2022
* Wasmtime+Cranelift: strip out some dead x86-32 code.

I was recently pointed to fastly/Viceroy#200 where it seems some folks
are trying to compile Wasmtime (via Viceroy) for Windows x86-32 and the
failures may not be loud enough. I've tried to reproduce this
cross-compiling to i686-pc-windows-gnu from Linux and hit build failures
(as expected) in several places.  Nevertheless, while trying to discern
what others may be attempting, I noticed some dead x86-32-specific code
in our repo, and figured it would be a good idea to clean this up.
Otherwise, it (i) sends some mixed messages -- "hey look, this codebase
does support x86-32" -- and (ii) keeps untested code around, which is
generally not great.

This PR removes x86-32-specific cases in traphandlers and unwind code,
and Cranelift's native feature detection. It adds helpful compile-error
messages in a few cases. If we ever support x86-32 (contributors
welcome! The big missing piece is Cranelift support; see #1980), these
compile errors and git history should be enough to recover any knowledge
we are now encoding in the source.

I left the x86-32 support in `wasmtime-fiber` alone because that seems
like a bit of a special case -- foundation library, separate from the
rest of Wasmtime, with specific care to provide a (presumably working)
full 32-bit version.

* Remove some extraneous compile_error!s, already covered by others.
@jkshtj
Copy link

jkshtj commented Jan 15, 2023

@tschneidereit @whitequark @cfallin @bjorn3 I would be interested in picking this up. I still need to dive deeper to come up with a concrete plan, but would one of you (or someone you could connect me to) be willing to mentor me?

@jameysharp
Copy link
Contributor

Hi @Jain98! We're always happy to answer questions and help however we can. I'd encourage you to join the #cranelift stream at https://bytecodealliance.zulipchat.com/ as a good place to ask questions.

Since all our current backends are 64-bit, we haven't necessarily designed everything to support 32-bit targets well. We'd like to fix that, but in the meantime just be aware that you may find some things are harder than they should be. There's been recent discussion about that in #5572, and the advice there applies here too.

@jkshtj
Copy link

jkshtj commented Jan 22, 2023

Hi @jameysharp, thanks for replying and a link to a recent discussion. I've already joined the cranelift zulip chat, but figured I'd start here as it seemed more intentional.

It at least looks like there's already an existing backend for x86-32, no? If yes, could you could point me to it? That might be a good starting point for me.

@jameysharp
Copy link
Contributor

There used to be an x86-32 backend, but it was unmaintained and eventually got in the way of working on the rest of Cranelift, so it was deleted. If I'm reading the history correctly there have been at least two complete rewrites of how Cranelift backends are written since the last time that backend worked, so digging it up out of the git history probably won't help you much. Instead I'd suggest looking at the current x86-64 backend, which is mostly in cranelift/codegen/src/isa/x64/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:x86 Issues related to x86 codegen cranelift:new-target Issues requesting Cranelift support for new targets. cranelift Issues related to the Cranelift code generator enhancement help wanted wasmtime:platform-support Related to supporting a new platform in Wasmtime
Projects
None yet
Development

No branches or pull requests

9 participants